Skip to content
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

Draft
wants to merge 3 commits into
base: main
Choose a base branch
from

Conversation

SBOne-Kenobi
Copy link
Collaborator

@SBOne-Kenobi SBOne-Kenobi commented Sep 19, 2022

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

  • Minor bug fix (non-breaking small changes)
  • Refactoring (typos and non-functional changes)

How Has This Been Tested?

Automated Testing

org.utbot.examples.casts.InstanceOfExampleTest

Manual Scenario

Described in issue #957

Checklist:

  • The change followed the style guidelines of the UTBot project
  • Self-review of the code is passed
  • The change contains enough commentaries, particularly in hard-to-understand areas
  • New documentation is provided or existed one is altered
  • No new warnings
  • New tests have been added
  • All tests pass locally with my changes

Copy link
Member

@CaelmBleidd CaelmBleidd left a 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

Copy link
Collaborator

@dtim dtim left a 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.

@@ -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)
Copy link
Collaborator

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?

Copy link
Collaborator Author

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.

Copy link
Member

@CaelmBleidd CaelmBleidd left a 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

Comment on lines +268 to +271
val possibleObjectTypes = Scene.v().classes.map { it.type }
return possibleObjectTypes.mapTo(mutableSetOf()) {
it.arrayType
}
Copy link
Member

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.

@SBOne-Kenobi SBOne-Kenobi marked this pull request as draft December 5, 2022 09:25
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Engine doesn't cover branch instanceof String
3 participants