-
Notifications
You must be signed in to change notification settings - Fork 45
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Fix wrapper concrete types #964
base: main
Are you sure you want to change the base?
Conversation
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I have many questions about types choice, I think we need additional documentation or explanations in the code, why it is done this way
utbot-framework/src/main/kotlin/org/utbot/engine/ArrayObjectWrappers.kt
Outdated
Show resolved
Hide resolved
utbot-framework/src/main/kotlin/org/utbot/engine/ArrayObjectWrappers.kt
Outdated
Show resolved
Hide resolved
utbot-framework/src/main/kotlin/org/utbot/engine/CollectionWrappers.kt
Outdated
Show resolved
Hide resolved
utbot-framework/src/main/kotlin/org/utbot/engine/StreamWrappers.kt
Outdated
Show resolved
Hide resolved
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I don't fully understand the logic, and have several concerns about the selection of types. It would be nice to discuss your PR in more details.
utbot-framework/src/main/kotlin/org/utbot/engine/ObjectWrappers.kt
Outdated
Show resolved
Hide resolved
utbot-framework-test/src/test/kotlin/org/utbot/examples/casts/InstanceOfExampleTest.kt
Show resolved
Hide resolved
utbot-framework/src/main/kotlin/org/utbot/engine/ArrayObjectWrappers.kt
Outdated
Show resolved
Hide resolved
@@ -137,6 +139,9 @@ abstract class BaseContainerWrapper(containerClassName: String) : BaseOverridden | |||
} | |||
} | |||
|
|||
override fun getPotentialPossibleTypes(type: Type): Set<Type> = | |||
setOf(Scene.v().getSootClass(chooseClassIdWithConstructor(type.classId).canonicalName).type) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I suppose that this code can return not only concrete classes but interfaces as well. It seems that it may be a bit dangerous. What do you think?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Are you sure? chooseClassIdWithConstructor must return class with callable constructor (I suppose) that we use for creation assemble model.
1f3de41
to
c390666
Compare
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
TBH, I don't have an opinion about this request. Probably, we should discuss it in person before merging
val possibleObjectTypes = Scene.v().classes.map { it.type } | ||
return possibleObjectTypes.mapTo(mutableSetOf()) { | ||
it.arrayType | ||
} |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Now it contains all the types, including interfaces, abstract classes, artificial entities, our own classes, etc.
Description
The problem was in wrappers instantiation. Initialized concrete types were incorrect. I've provided method for wrappers witch returns set of possible types in resolving. Now all of set's sizes equals 1.
Fixes #957
Type of Change
How Has This Been Tested?
Automated Testing
org.utbot.examples.casts.InstanceOfExampleTest
Manual Scenario
Described in issue #957
Checklist: