RFR: 8254023: A module declaration is not allowed to be a target of an annotation that lacks an @Target meta-annotation

Guoxiong Li github.com+13688759+lgxbslgx at openjdk.java.net
Fri Dec 11 16:35:01 UTC 2020


On Thu, 10 Dec 2020 19:34:01 GMT, Vicente Romero <vromero at openjdk.org> wrote:

>> Hi,
>> 
>> I don't think TargetAnnoCombo should be updated, at least not for this fix. It is an old hard to understand test that predates modules.
>> 
>> It would be good to do further testing as Vicente writes. I suggest:
>> 
>> 1) A test that the module info compiles, you already have this.
>> 2) A test that the class file contains the bytes for the annotation might make sense. As Jon has pointed out in different review threads there is the toolbox, perhaps that could be of use? There are some examples of toolbox tests in  `jdk/test/langtools/tools/javac/modules` though I don't have a good example of one that verifies the contents of the class file. Perhaps @vicente-romero-oracle can point to an example? 
>> 3) There should already be tests that annotation processing can pick up annotations on module declarations, it looks like `jdk/test/langtools/tools/javac/modules/AnnotationProcessing.java` contains some of this. See if you can add a case with an annotation without @Target.
>
>> Hi,
>> 
>> I don't think TargetAnnoCombo should be updated, at least not for this fix. It is an old hard to understand test that predates modules.
>> 
>> It would be good to do further testing as Vicente writes. I suggest:
>> 
>>     1. A test that the module info compiles, you already have this.
>> 
>>     2. A test that the class file contains the bytes for the annotation might make sense. As Jon has pointed out in different review threads there is the toolbox, perhaps that could be of use? There are some examples of toolbox tests in  `jdk/test/langtools/tools/javac/modules` though I don't have a good example of one that verifies the contents of the class file. Perhaps @vicente-romero-oracle can point to an example?
> 
> it is probably an overkill but method: `testAnnos` in test: `test/langtools/tools/javac/records/RecordCompilationTests.java` is an example applied to records that could be adapted to what we want here

I added two test cases. Thank you for taking the time to review.

-------------

PR: https://git.openjdk.java.net/jdk/pull/622


More information about the compiler-dev mailing list