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

[Jan Lahoda]

Looks good to me.

Jan

On 25.7.2018 08:32, 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
>>>
>>
>