JDK 12 RFR of JDK-8190886: package-info handling in RoundEnvironment.getElementsAnnotatedWith

[Vicente Romero]

looks good,
Vicente

On 07/25/2018 02:32 AM, joe darcy wrote:
> Hello,
>
> I had hoped that writing a few more test to cover the modules cases 
> would be a simple exercise, but that is not how things worked out!
>
> The resulting set of changes is now:
>
>     http://cr.openjdk.java.net/~darcy/8190886.6/
>
> Major differences between the prior version sent for review are in the 
> files
>
> src/jdk.compiler/share/classes/com/sun/tools/javac/processing/JavacRoundEnvironment.java 
>
> test/langtools/tools/javac/processing/environment/round/TestElementsAnnotatedWith.java 
>
>
> The earlier iteration of the fix was adequate for package-info files; 
> however, there are several complications related to modules the 
> previous fix did not account for.
>
> The getElementsAnnotatedWith methods with a java.lang.Class parameter 
> need some way to bridge between the core reflection world and the 
> compile-time reflection world of javax.lang.model. The way to do this 
> is to construct a TypeElement for an annotation type corresponding to 
> the Class object for the annotation type argument by mapping from the 
> name of the annotation type to a type element via 
> Elements.getTypeElement(name). A wrinkle is that the behavior of 
> Elements.getTypeElement(name) was redefined in JDK 9 to account for 
> modules since a name can correspond to multiple types post-modules. 
> The revised fix first tries Elements.getTypeElement(name) and if that 
> fails, including failing due to multiple results being found, it fails 
> over to the two-argument version of Elements.getTypeElement which has 
> a module parameter. The module parameter is set to a module whose name 
> matches the name of the module of the Class, if such a Class is in a 
> named module.
>
> On the testing sides, as discussed in JEP 200 when the compiler is 
> running in single-module mode (which includes when compiling a 
> module-info file), all the files being compiled are assumed to be in 
> that module even if they would not be in the module by virtue of their 
> file system locations. This necessitates some of the testing 
> conditions being conditionally excluded.
>
> I'll leave any extension of this test to multi-module compilation mode 
> for future work.
>
> Thanks,
>
> -Joe
>
>
> On 7/24/2018 10:00 AM, Vicente Romero wrote:
>> looks good, just minimal copyright for test:
>>
>> test/langtools/tools/javac/processing/environment/round/pkg/package-info.java 
>>
>>
>> should be 2018
>>
>> Vicente
>>
>> On 07/24/2018 02:50 AM, joe darcy wrote:
>>> Hello,
>>>
>>> Please review the changes to address
>>>
>>>     JDK-8190886: package-info handling in 
>>> RoundEnvironment.getElementsAnnotatedWith
>>>     http://cr.openjdk.java.net/~darcy/8190886.1/
>>>
>>> Contrary to the spec, the RoundEnvironment.getElementsAnnotatedWith 
>>> implementation was traversing into packages (and modules). Given the 
>>> phase of JDK 11 and at least the potential for behavioral 
>>> compatibility impact, I'm proposing to fix this in 12 rather than 
>>> 11. I'll file a CSR for the behavioral change in due course.
>>>
>>> The implementation change is small, just adding overrides of visitor 
>>> methods which skip scanning packages and modules. To allow use of 
>>> AnnotatedElementInfo in compilation units in a named package, the 
>>> AnnotatedElementInfo type has to itself be in a named package rather 
>>> than an unnamed one; therefore, a number of supporting changes are 
>>> needed to other test files in the directory to support that type 
>>> rename.
>>>
>>> The tests have not been updated to verify the behavior on modules, 
>>> but that could be done too.
>>>
>>> I've verified the updated test fails with a promoted build of JDK 12 
>>> but passes with a build with these javac modifications. All other 
>>> langtools tests pass on a modified build too.
>>>
>>> I'll perform a copyright pass before pushing once the rest of the 
>>> contents are reviewed.
>>>
>>> Thanks,
>>>
>>> -Joe
>>>
>>
>