[GitHub] [groovy] danielsun1106 edited a comment on issue #1171: GROOVY-9416: Avoid unnecessary looking up non default import classes …

[GitBox <gi...@apache.org>]

danielsun1106 edited a comment on issue #1171: GROOVY-9416: Avoid unnecessary looking up non default import classes …
URL: https://github.com/apache/groovy/pull/1171#issuecomment-589982874
 
 
   > is it really right to return false in case of the deterministic default imports fallback? You having had to change so many imports in tests hints, that this issue changes the resolving process in an undesirable way... or you forgot to remove that code, which you then should do ;)
   
   "the deterministic default imports" means the default imported classes found by `ClassFinder` are complete, in other words, groovy compiler knows all the default imported classes, so it does not have to gurantee some unknown default imported classes via looking up against all the default imported packages. Let's talk about the topic via some key code:
   
   In the old resolving logic for default imports, we resolve the default imported classes by 2 steps:
   1) Try to find the deterministic package name from the cache, if found, we do not need more looking up
   https://github.com/apache/groovy/blob/GROOVY_3_0_1/src/main/java/org/codehaus/groovy/control/ResolveVisitor.java#L618-L620
   
   2) If the step 1 failed, we look up the type name against all the default imported packages
   https://github.com/apache/groovy/blob/GROOVY_3_0_1/src/main/java/org/codehaus/groovy/control/ResolveVisitor.java#L623-L625
   
   In the new resolving logic for default imports, we eleminate the step 2 by default because the cache contains all default imported classes and step 1 will them find successfuly. If step 1 does not find, the type must not be default imported class, so step 2 is not necessary to process, i.e. we replace the step 2 by Java9 vmplugin's `return false` directly. The new logic of step 2 is shown as follows:
   https://github.com/apache/groovy/blob/9d36f6dfcec2d165dd3dea9b97a2381600a2c0ca/src/main/java/org/codehaus/groovy/vmplugin/v9/Java9.java#L145-L151
   
   

----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
 
For queries about this service, please contact Infrastructure at:
users@infra.apache.org


With regards,
Apache Git Services