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 typed parameters in access path #1007

Merged
merged 6 commits into from
Nov 28, 2016

Conversation

pvojtechovsky
Copy link
Collaborator

fixes #1000

@@ -324,6 +324,9 @@ public TypeFactory() {
}

ref.setSimpleName(type.getSimpleName());
for (CtTypeParameter typeParam : type.getFormalCtTypeParameters()) {
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

why is this change required?

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I have already found, that this change is wrong. I will rollback it and do it different.

@pvojtechovsky
Copy link
Collaborator Author

This implementation is still not 100% correct, but

  • it is fitting more to origin concept of @tdurieux, because CtTypeReference.toString() now returns access path and not qualified name - same like in his origin concept! I restored some test cases to the state before my accessPath implementation.
  • all tests pass and guava seems to compile now too.

The remaining problem is how to correctly map actual type arguments. See the comment in CtTypeReferenceImpl.applyActualTypeArguments.
I do not want to implement that algorithm now. I vote for merging it as it is and to fix it after somebody writes code which will really need that. ... the origin problem was very rare and the case when this implementation fails should be even more rare :-)

@monperrus
Copy link
Collaborator

it looks good to me, thanks. @tdurieux OK for you?

CtTypeReference.toString() now returns access path and not qualified name - same like in his origin concept!

this is fine, in general toString on references is not as important as toString on code elements, it's more for debug purposes, so the specification is more malleable.

@tdurieux
Copy link
Collaborator

@monperrus it is good for me

@monperrus monperrus merged commit 9947d77 into INRIA:master Nov 28, 2016
@pvojtechovsky pvojtechovsky deleted the fixTypedParametersInAccessPath branch November 28, 2016 20:10
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.

regression on fully-qualified names
3 participants