-
-
Notifications
You must be signed in to change notification settings - Fork 352
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
feat: add method CtMethod#getTopDefinitions #1844
Conversation
e2050c2
to
62cf33c
Compare
Detected changes by Revapi: 1. Old API: fr.inria.gforge.spoon:spoon-core:jar:6.2.0-20180209.130552-78 New API: fr.inria.gforge.spoon:spoon-core:jar:6.2.0-SNAPSHOT
|
62cf33c
to
5da6f84
Compare
5da6f84
to
a108db2
Compare
// now removing the intermediate methods for which there exists a definition upper in the hierarchy | ||
List<CtMethod<?>> finalMeths = new ArrayList<>(s); | ||
for (CtMethod m1 : s) { | ||
ClassTypingContext context2 = new ClassTypingContext(m1.getDeclaringType()); |
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.
Did you try to use existing context
? I guess the existing one already contains all the necessary information, so there is no need to create new one for each method.
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.
done
|
||
// first collect potential declarations of this method in the type hierarchy | ||
ClassTypingContext context = new ClassTypingContext(this.getDeclaringType()); | ||
for (Object m : getDeclaringType().map(new AllTypeMembersFunction(CtMethod.class)).list()) { |
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.
What about CtQuery#forEach instead of list() and then for(m : list){}
? It will be shorter code, faster run and less memory needed.
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.
not possible since we need two passes on the list
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 created a PR on your PR here monperrus#7
I'll try.
|
done the changes |
Detected changes by Revapi: 1. Old API: fr.inria.gforge.spoon:spoon-core:jar:6.2.0-20180213.235035-85 New API: fr.inria.gforge.spoon:spoon-core:jar:6.2.0-SNAPSHOT
|
replace for(m:list()) by forEach(m->)
merged your PRPR, note that you can push to my branch directly as well. |
Detected changes by Revapi: 1. Old API: fr.inria.gforge.spoon:spoon-core:jar:6.2.0-20180215.200548-88 New API: fr.inria.gforge.spoon:spoon-core:jar:6.2.0-SNAPSHOT
|
fix #1821