RFR: 8231436: Fix the applicability of a no-@Target annotation type

classic Classic list List threaded Threaded
23 messages Options
12
Reply | Threaded
Open this post in threaded view
|

RFR: 8231436: Fix the applicability of a no-@Target annotation type

Liam Miller-Cushon-2
Please review this fix to add `ElementType.MODULE` to the default list of annotation targets, to allow annotations without an explicit `@Target` to be used on module declarations.

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

Commit messages:
 - 8231436: Fix the applicability of a no-@Target annotation type

Changes: https://git.openjdk.java.net/jdk/pull/2303/files
 Webrev: https://webrevs.openjdk.java.net/?repo=jdk&pr=2303&range=00
  Issue: https://bugs.openjdk.java.net/browse/JDK-8231436
  Stats: 12 lines in 3 files changed: 10 ins; 0 del; 2 mod
  Patch: https://git.openjdk.java.net/jdk/pull/2303.diff
  Fetch: git fetch https://git.openjdk.java.net/jdk pull/2303/head:pull/2303

PR: https://git.openjdk.java.net/jdk/pull/2303
Reply | Threaded
Open this post in threaded view
|

Re: RFR: 8231436: Fix the applicability of a no-@Target annotation type

Joel Borggrén-Franck-3
On Thu, 28 Jan 2021 22:33:53 GMT, Liam Miller-Cushon <[hidden email]> wrote:

> Please review this fix to add `ElementType.MODULE` to the default list of annotation targets, to allow annotations without an explicit `@Target` to be used on module declarations.

Hi Liam,

Code looks good, but I'd prefer if you file a more specific bug. Maybe also add a small test that doesn't compile before the fix?

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

PR: https://git.openjdk.java.net/jdk/pull/2303
Reply | Threaded
Open this post in threaded view
|

Re: RFR: 8231436: Fix the applicability of a no-@Target annotation type

Joe Darcy-2
On Mon, 1 Feb 2021 20:18:09 GMT, Joel Borggrén-Franck <[hidden email]> wrote:

>> Please review this fix to add `ElementType.MODULE` to the default list of annotation targets, to allow annotations without an explicit `@Target` to be used on module declarations.
>
> Hi Liam,
>
> Code looks good, but I'd prefer if you file a more specific bug. Maybe also add a small test that doesn't compile before the fix?

Please file a CSR so that the behavioral compatibly change can be assessed.

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

PR: https://git.openjdk.java.net/jdk/pull/2303
Reply | Threaded
Open this post in threaded view
|

Re: RFR: 8231436: Fix the applicability of a no-@Target annotation type

Liam Miller-Cushon-2
On Mon, 1 Feb 2021 20:40:54 GMT, Joe Darcy <[hidden email]> wrote:

>> Hi Liam,
>>
>> Code looks good, but I'd prefer if you file a more specific bug. Maybe also add a small test that doesn't compile before the fix?
>
> Please file a CSR so that the behavioral compatibly change can be assessed.

Thanks for the comments,

@jbf I'll add a more focused test. For the bug, the original discussion in https://mail.openjdk.java.net/pipermail/compiler-dev/2019-September/013705.html is specifically about annotations without an explicit `@Target` applying to `MODULE`, I don't think there's anything that needs to be done for [JDK-8231436](https://bugs.openjdk.java.net/browse/JDK-8231436) besides supporting `MODULE`, would you still prefer a separate bug?

@jddarcy note that there was already a spec change related to this in [JDK-8231435](https://bugs.openjdk.java.net/browse/JDK-8231435) and the spec bug mentions "this expansion is source, binary, and behaviorally compatible", should I still file a CSR?

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

PR: https://git.openjdk.java.net/jdk/pull/2303
Reply | Threaded
Open this post in threaded view
|

Re: RFR: 8231436: Fix the applicability of a no-@Target annotation type

Joel Borggrén-Franck-3
On Mon, 1 Feb 2021 20:50:41 GMT, Liam Miller-Cushon <[hidden email]> wrote:

>> Please file a CSR so that the behavioral compatibly change can be assessed.
>
> Thanks for the comments,
>
> @jbf I'll add a more focused test. For the bug, the original discussion in https://mail.openjdk.java.net/pipermail/compiler-dev/2019-September/013705.html is specifically about annotations without an explicit `@Target` applying to `MODULE`, I don't think there's anything that needs to be done for [JDK-8231436](https://bugs.openjdk.java.net/browse/JDK-8231436) besides supporting `MODULE`, would you still prefer a separate bug?
>
> @jddarcy note that there was already a spec change related to this in [JDK-8231435](https://bugs.openjdk.java.net/browse/JDK-8231435) and the spec bug mentions "this expansion is source, binary, and behaviorally compatible", should I still file a CSR?

From this follow up here: https://mail.openjdk.java.net/pipermail/compiler-dev/2019-September/013715.html my interpretation is that this bug was intended to widen it to all contexts, including type use. This has been changed in JLS but not in the normative javadoc and also not in the compiler. I believe we should keep declarations only, and back out the change to JLS.

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

PR: https://git.openjdk.java.net/jdk/pull/2303
Reply | Threaded
Open this post in threaded view
|

Re: RFR: 8231436: Fix the applicability of a no-@Target annotation type

Guoxiong Li-2
On Tue, 2 Feb 2021 09:28:59 GMT, Joel Borggrén-Franck <[hidden email]> wrote:

>> Thanks for the comments,
>>
>> @jbf I'll add a more focused test. For the bug, the original discussion in https://mail.openjdk.java.net/pipermail/compiler-dev/2019-September/013705.html is specifically about annotations without an explicit `@Target` applying to `MODULE`, I don't think there's anything that needs to be done for [JDK-8231436](https://bugs.openjdk.java.net/browse/JDK-8231436) besides supporting `MODULE`, would you still prefer a separate bug?
>>
>> @jddarcy note that there was already a spec change related to this in [JDK-8231435](https://bugs.openjdk.java.net/browse/JDK-8231435) and the spec bug mentions "this expansion is source, binary, and behaviorally compatible", should I still file a CSR?
>
> From this follow up here: https://mail.openjdk.java.net/pipermail/compiler-dev/2019-September/013715.html my interpretation is that this bug was intended to widen it to all contexts, including type use. This has been changed in JLS but not in the normative javadoc and also not in the compiler. I believe we should keep declarations only, and back out the change to JLS.

Maybe we should collect the concrete information about this problem. Here are some materials that I know.

Order by time:
- 2019.08 **Werner Dietl** launched the discussion. [1]
- 2019.09 After the discussion between **Alex Buckley** and **Michael Ernst**, an initiative result came out.[2]
- 2019.09 Two JBS issues were filed. [3][4]
- 2019.12 **Alex Buckley** fixed the JLS(fix version: 14). But the compiler code and javadoc were not fixed. [5][6]
- 2020.10 **Christian Stein** created another issue about it. [7]
- 2020.10 **Guoxiong Li**(me) submitted a PR about it in Github. [8]
- 2020.10 **Joel Borggrén-Franck** restarted the discussion about the specification. [9]
- 2020.12 Because the JDK16 was nearly at RDP1, we decided to only fix JDK-8254023. And other work left to JDk17.  [7][10][11]
- 2020.12 The issue JDK-8254023 was fixed at JDK16. [7][8][12]
- **Now, we need to discuss the problem(unify the specification, implement and javadoc) that JDK16 has left.**[11]

[1] https://mail.openjdk.java.net/pipermail/compiler-dev/2019-August/013666.html
[2] https://mail.openjdk.java.net/pipermail/compiler-dev/2019-September/013705.html
[3] https://bugs.openjdk.java.net/browse/JDK-8231435
[4] https://bugs.openjdk.java.net/browse/JDK-8231436
[5] https://docs.oracle.com/javase/specs/jls/se14/html/jls-9.html#jls-9.6.4.1
[6] https://docs.oracle.com/en/java/javase/14/docs/api/java.base/java/lang/annotation/Target.html
[7] https://bugs.openjdk.java.net/browse/JDK-8254023
[8] https://github.com/openjdk/jdk/pull/622
[9] https://mail.openjdk.java.net/pipermail/compiler-dev/2020-October/015197.html
[10] http://openjdk.java.net/projects/jdk/16/
[11] https://mail.openjdk.java.net/pipermail/compiler-dev/2020-December/015503.html
[12] https://github.com/openjdk/jdk16/pull/34

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

PR: https://git.openjdk.java.net/jdk/pull/2303
Reply | Threaded
Open this post in threaded view
|

Re: RFR: 8231436: Fix the applicability of a no-@Target annotation type

Alex Buckley-3
I initially proposed "all declaration contexts" as the smallest possible
streamlining of the SE 7-oriented rule defined in SE 8. It would have
ensured that legacy annotation types without @Target were locked out of
the new world of type uses enabled in SE 8 by JSR 308. As it turned out,
my conservative approach was unnecessary because Mike was fine with
admitting those legacy annotation types for type uses via the "all
contexts" rule. In the real world, "all declaration contexts" versus
"all declaration contexts + all type contexts" is a distinction without
a difference (that is, an insignificant distinction) -- we're best off
adopting the latter rule, phrased simply as "all contexts".

Alex

On 2/2/2021 3:44 AM, Guoxiong Li wrote:

> On Tue, 2 Feb 2021 09:28:59 GMT, Joel Borggrén-Franck <[hidden email]> wrote:
>
>>> Thanks for the comments,
>>>
>>> @jbf I'll add a more focused test. For the bug, the original discussion in https://mail.openjdk.java.net/pipermail/compiler-dev/2019-September/013705.html is specifically about annotations without an explicit `@Target` applying to `MODULE`, I don't think there's anything that needs to be done for [JDK-8231436](https://bugs.openjdk.java.net/browse/JDK-8231436) besides supporting `MODULE`, would you still prefer a separate bug?
>>>
>>> @jddarcy note that there was already a spec change related to this in [JDK-8231435](https://bugs.openjdk.java.net/browse/JDK-8231435) and the spec bug mentions "this expansion is source, binary, and behaviorally compatible", should I still file a CSR?
>>
>>  From this follow up here: https://mail.openjdk.java.net/pipermail/compiler-dev/2019-September/013715.html my interpretation is that this bug was intended to widen it to all contexts, including type use. This has been changed in JLS but not in the normative javadoc and also not in the compiler. I believe we should keep declarations only, and back out the change to JLS.
>
> Maybe we should collect the concrete information about this problem. Here are some materials that I know.
>
> Order by time:
> - 2019.08 **Werner Dietl** launched the discussion. [1]
> - 2019.09 After the discussion between **Alex Buckley** and **Michael Ernst**, an initiative result came out.[2]
> - 2019.09 Two JBS issues were filed. [3][4]
> - 2019.12 **Alex Buckley** fixed the JLS(fix version: 14). But the compiler code and javadoc were not fixed. [5][6]
> - 2020.10 **Christian Stein** created another issue about it. [7]
> - 2020.10 **Guoxiong Li**(me) submitted a PR about it in Github. [8]
> - 2020.10 **Joel Borggrén-Franck** restarted the discussion about the specification. [9]
> - 2020.12 Because the JDK16 was nearly at RDP1, we decided to only fix JDK-8254023. And other work left to JDk17.  [7][10][11]
> - 2020.12 The issue JDK-8254023 was fixed at JDK16. [7][8][12]
> - **Now, we need to discuss the problem(unify the specification, implement and javadoc) that JDK16 has left.**[11]
>
> [1] https://mail.openjdk.java.net/pipermail/compiler-dev/2019-August/013666.html
> [2] https://mail.openjdk.java.net/pipermail/compiler-dev/2019-September/013705.html
> [3] https://bugs.openjdk.java.net/browse/JDK-8231435
> [4] https://bugs.openjdk.java.net/browse/JDK-8231436
> [5] https://docs.oracle.com/javase/specs/jls/se14/html/jls-9.html#jls-9.6.4.1
> [6] https://docs.oracle.com/en/java/javase/14/docs/api/java.base/java/lang/annotation/Target.html
> [7] https://bugs.openjdk.java.net/browse/JDK-8254023
> [8] https://github.com/openjdk/jdk/pull/622
> [9] https://mail.openjdk.java.net/pipermail/compiler-dev/2020-October/015197.html
> [10] http://openjdk.java.net/projects/jdk/16/
> [11] https://mail.openjdk.java.net/pipermail/compiler-dev/2020-December/015503.html
> [12] https://github.com/openjdk/jdk16/pull/34
>
> -------------
>
> PR: https://git.openjdk.java.net/jdk/pull/2303
>
Reply | Threaded
Open this post in threaded view
|

Re: RFR: 8231436: Fix the applicability of a no-@Target annotation type

Joel Borggren-Franck
Hi Alex,

Looking at  the class file, and therefore reflection, there is a difference between “all declaration contexts” and “all contexts, declaration + type”. For the 5 ambiguous locations we would have to produce type annotation attributes if there is an annotation present whose declaration lack @Target. While this may or may not be desirable, and I’m leaning towards not, it is surely significant as it would change the semantics of reflective annotation processors in use.

cheers
/Joel

> On 2 Feb 2021, at 19:50, Alex Buckley <[hidden email]> wrote:
>
> I initially proposed "all declaration contexts" as the smallest possible streamlining of the SE 7-oriented rule defined in SE 8. It would have ensured that legacy annotation types without @Target were locked out of the new world of type uses enabled in SE 8 by JSR 308. As it turned out, my conservative approach was unnecessary because Mike was fine with admitting those legacy annotation types for type uses via the "all contexts" rule. In the real world, "all declaration contexts" versus "all declaration contexts + all type contexts" is a distinction without a difference (that is, an insignificant distinction) -- we're best off adopting the latter rule, phrased simply as "all contexts".
>
> Alex
>
> On 2/2/2021 3:44 AM, Guoxiong Li wrote:
>> On Tue, 2 Feb 2021 09:28:59 GMT, Joel Borggrén-Franck <[hidden email]> wrote:
>>>> Thanks for the comments,
>>>>
>>>> @jbf I'll add a more focused test. For the bug, the original discussion in https://mail.openjdk.java.net/pipermail/compiler-dev/2019-September/013705.html is specifically about annotations without an explicit `@Target` applying to `MODULE`, I don't think there's anything that needs to be done for [JDK-8231436](https://bugs.openjdk.java.net/browse/JDK-8231436) besides supporting `MODULE`, would you still prefer a separate bug?
>>>>
>>>> @jddarcy note that there was already a spec change related to this in [JDK-8231435](https://bugs.openjdk.java.net/browse/JDK-8231435) and the spec bug mentions "this expansion is source, binary, and behaviorally compatible", should I still file a CSR?
>>>
>>> From this follow up here: https://mail.openjdk.java.net/pipermail/compiler-dev/2019-September/013715.html my interpretation is that this bug was intended to widen it to all contexts, including type use. This has been changed in JLS but not in the normative javadoc and also not in the compiler. I believe we should keep declarations only, and back out the change to JLS.
>> Maybe we should collect the concrete information about this problem. Here are some materials that I know.
>> Order by time:
>> - 2019.08 **Werner Dietl** launched the discussion. [1]
>> - 2019.09 After the discussion between **Alex Buckley** and **Michael Ernst**, an initiative result came out.[2]
>> - 2019.09 Two JBS issues were filed. [3][4]
>> - 2019.12 **Alex Buckley** fixed the JLS(fix version: 14). But the compiler code and javadoc were not fixed. [5][6]
>> - 2020.10 **Christian Stein** created another issue about it. [7]
>> - 2020.10 **Guoxiong Li**(me) submitted a PR about it in Github. [8]
>> - 2020.10 **Joel Borggrén-Franck** restarted the discussion about the specification. [9]
>> - 2020.12 Because the JDK16 was nearly at RDP1, we decided to only fix JDK-8254023. And other work left to JDk17.  [7][10][11]
>> - 2020.12 The issue JDK-8254023 was fixed at JDK16. [7][8][12]
>> - **Now, we need to discuss the problem(unify the specification, implement and javadoc) that JDK16 has left.**[11]
>> [1] https://mail.openjdk.java.net/pipermail/compiler-dev/2019-August/013666.html
>> [2] https://mail.openjdk.java.net/pipermail/compiler-dev/2019-September/013705.html
>> [3] https://bugs.openjdk.java.net/browse/JDK-8231435
>> [4] https://bugs.openjdk.java.net/browse/JDK-8231436
>> [5] https://docs.oracle.com/javase/specs/jls/se14/html/jls-9.html#jls-9.6.4.1
>> [6] https://docs.oracle.com/en/java/javase/14/docs/api/java.base/java/lang/annotation/Target.html
>> [7] https://bugs.openjdk.java.net/browse/JDK-8254023
>> [8] https://github.com/openjdk/jdk/pull/622
>> [9] https://mail.openjdk.java.net/pipermail/compiler-dev/2020-October/015197.html
>> [10] http://openjdk.java.net/projects/jdk/16/
>> [11] https://mail.openjdk.java.net/pipermail/compiler-dev/2020-December/015503.html
>> [12] https://github.com/openjdk/jdk16/pull/34
>> -------------
>> PR: https://git.openjdk.java.net/jdk/pull/2303

Reply | Threaded
Open this post in threaded view
|

Re: RFR: 8231436: Fix the applicability of a no-@Target annotation type

Joe Darcy
Hello,

To me, it seems that intending an annotation to be used on declarations
or on types is a basic design decision of the annotation type. I think
it can be reasonable to interpret lack of @Target to mean "usable in any
declaration context," but ill-advised to silently reinterpret absence of
a @Target to indicate types and declarations.

Cheers,

-Joe

On 2/3/2021 5:29 AM, Joel Borggren-Franck wrote:

> Hi Alex,
>
> Looking at  the class file, and therefore reflection, there is a difference between “all declaration contexts” and “all contexts, declaration + type”. For the 5 ambiguous locations we would have to produce type annotation attributes if there is an annotation present whose declaration lack @Target. While this may or may not be desirable, and I’m leaning towards not, it is surely significant as it would change the semantics of reflective annotation processors in use.
>
> cheers
> /Joel
>
>> On 2 Feb 2021, at 19:50, Alex Buckley <[hidden email]> wrote:
>>
>> I initially proposed "all declaration contexts" as the smallest possible streamlining of the SE 7-oriented rule defined in SE 8. It would have ensured that legacy annotation types without @Target were locked out of the new world of type uses enabled in SE 8 by JSR 308. As it turned out, my conservative approach was unnecessary because Mike was fine with admitting those legacy annotation types for type uses via the "all contexts" rule. In the real world, "all declaration contexts" versus "all declaration contexts + all type contexts" is a distinction without a difference (that is, an insignificant distinction) -- we're best off adopting the latter rule, phrased simply as "all contexts".
>>
>> Alex
>>
>> On 2/2/2021 3:44 AM, Guoxiong Li wrote:
>>> On Tue, 2 Feb 2021 09:28:59 GMT, Joel Borggrén-Franck <[hidden email]> wrote:
>>>>> Thanks for the comments,
>>>>>
>>>>> @jbf I'll add a more focused test. For the bug, the original discussion in https://mail.openjdk.java.net/pipermail/compiler-dev/2019-September/013705.html is specifically about annotations without an explicit `@Target` applying to `MODULE`, I don't think there's anything that needs to be done for [JDK-8231436](https://bugs.openjdk.java.net/browse/JDK-8231436) besides supporting `MODULE`, would you still prefer a separate bug?
>>>>>
>>>>> @jddarcy note that there was already a spec change related to this in [JDK-8231435](https://bugs.openjdk.java.net/browse/JDK-8231435) and the spec bug mentions "this expansion is source, binary, and behaviorally compatible", should I still file a CSR?
>>>>  From this follow up here: https://mail.openjdk.java.net/pipermail/compiler-dev/2019-September/013715.html my interpretation is that this bug was intended to widen it to all contexts, including type use. This has been changed in JLS but not in the normative javadoc and also not in the compiler. I believe we should keep declarations only, and back out the change to JLS.
>>> Maybe we should collect the concrete information about this problem. Here are some materials that I know.
>>> Order by time:
>>> - 2019.08 **Werner Dietl** launched the discussion. [1]
>>> - 2019.09 After the discussion between **Alex Buckley** and **Michael Ernst**, an initiative result came out.[2]
>>> - 2019.09 Two JBS issues were filed. [3][4]
>>> - 2019.12 **Alex Buckley** fixed the JLS(fix version: 14). But the compiler code and javadoc were not fixed. [5][6]
>>> - 2020.10 **Christian Stein** created another issue about it. [7]
>>> - 2020.10 **Guoxiong Li**(me) submitted a PR about it in Github. [8]
>>> - 2020.10 **Joel Borggrén-Franck** restarted the discussion about the specification. [9]
>>> - 2020.12 Because the JDK16 was nearly at RDP1, we decided to only fix JDK-8254023. And other work left to JDk17.  [7][10][11]
>>> - 2020.12 The issue JDK-8254023 was fixed at JDK16. [7][8][12]
>>> - **Now, we need to discuss the problem(unify the specification, implement and javadoc) that JDK16 has left.**[11]
>>> [1] https://mail.openjdk.java.net/pipermail/compiler-dev/2019-August/013666.html
>>> [2] https://mail.openjdk.java.net/pipermail/compiler-dev/2019-September/013705.html
>>> [3] https://bugs.openjdk.java.net/browse/JDK-8231435
>>> [4] https://bugs.openjdk.java.net/browse/JDK-8231436
>>> [5] https://docs.oracle.com/javase/specs/jls/se14/html/jls-9.html#jls-9.6.4.1
>>> [6] https://docs.oracle.com/en/java/javase/14/docs/api/java.base/java/lang/annotation/Target.html
>>> [7] https://bugs.openjdk.java.net/browse/JDK-8254023
>>> [8] https://github.com/openjdk/jdk/pull/622
>>> [9] https://mail.openjdk.java.net/pipermail/compiler-dev/2020-October/015197.html
>>> [10] http://openjdk.java.net/projects/jdk/16/
>>> [11] https://mail.openjdk.java.net/pipermail/compiler-dev/2020-December/015503.html
>>> [12] https://github.com/openjdk/jdk16/pull/34
>>> -------------
>>> PR: https://git.openjdk.java.net/jdk/pull/2303
Reply | Threaded
Open this post in threaded view
|

Re: RFR: 8231436: Fix the applicability of a no-@Target annotation type

Liam Miller-Cushon-2
In reply to this post by Guoxiong Li-2
On Tue, 2 Feb 2021 11:03:17 GMT, Guoxiong Li <[hidden email]> wrote:

>> From this follow up here: https://mail.openjdk.java.net/pipermail/compiler-dev/2019-September/013715.html my interpretation is that this bug was intended to widen it to all contexts, including type use. This has been changed in JLS but not in the normative javadoc and also not in the compiler. I believe we should keep declarations only, and back out the change to JLS.
>
> Maybe we should collect the concrete information about this problem. Here are some materials that I know.
>
> Order by time:
> - 2019.08 **Werner Dietl** launched the discussion. [1]
> - 2019.09 After the discussion between **Alex Buckley** and **Michael Ernst**, an initiative result came out.[2]
> - 2019.09 Two JBS issues were filed. [3][4]
> - 2019.12 **Alex Buckley** fixed the JLS(fix version: 14). But the compiler code and javadoc were not fixed. [5][6]
> - 2020.10 **Christian Stein** created another issue about it. [7]
> - 2020.10 **Guoxiong Li**(me) submitted a PR about it in Github. [8]
> - 2020.10 **Joel Borggrén-Franck** restarted the discussion about the specification. [9]
> - 2020.12 Because the JDK16 was nearly at RDP1, we decided to only fix JDK-8254023. And other work left to JDk17.  [7][10][11]
> - 2020.12 The issue JDK-8254023 was fixed at JDK16. [7][8][12]
> - **Now, we need to discuss the problem(unify the specification, implement and javadoc) that JDK16 has left.**[11]
>
> [1] https://mail.openjdk.java.net/pipermail/compiler-dev/2019-August/013666.html
> [2] https://mail.openjdk.java.net/pipermail/compiler-dev/2019-September/013705.html
> [3] https://bugs.openjdk.java.net/browse/JDK-8231435
> [4] https://bugs.openjdk.java.net/browse/JDK-8231436
> [5] https://docs.oracle.com/javase/specs/jls/se14/html/jls-9.html#jls-9.6.4.1
> [6] https://docs.oracle.com/en/java/javase/14/docs/api/java.base/java/lang/annotation/Target.html
> [7] https://bugs.openjdk.java.net/browse/JDK-8254023
> [8] https://github.com/openjdk/jdk/pull/622
> [9] https://mail.openjdk.java.net/pipermail/compiler-dev/2020-October/015197.html
> [10] http://openjdk.java.net/projects/jdk/16/
> [11] https://mail.openjdk.java.net/pipermail/compiler-dev/2020-December/015503.html
> [12] https://github.com/openjdk/jdk16/pull/34

Thanks, I was missing context here. I now realize this patch is fixing a bug related to [JDK-8254023](https://bugs.openjdk.java.net/browse/JDK-8254023)--when the handling of annotations without `@Target` was updated to allow them to be applied to module declarations, the handling of repeatable annotations without `@Target` should also have been updated. I filed a new bug for the sub-issue that this patch is fixing, and will update it accordingly in a minute: https://bugs.openjdk.java.net/browse/JDK-8261088

I'm also happy to help with any work to be done on the larger question of interpreting `@Target`-less annotations as applicable to type contexts, if there's agreement about what to do there.

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

PR: https://git.openjdk.java.net/jdk/pull/2303
Reply | Threaded
Open this post in threaded view
|

Re: RFR: 8231436: Fix the applicability of a no-@Target annotation type

Alex Buckley-3
In reply to this post by Joel Borggren-Franck
Let me focus first on the design question of what a no-@Target
annotation type "means". There's no particular reason why it _shouldn't_
mean "all contexts". Mike gave good reasons why it should mean that. For
the corner case of an annotation appearing in one of the ambiguous
locations in source code, the annotation will appear in the class
file/via reflection exactly as if someone had taken the perfectly
legitimate course of spelling out @Target({METHOD, TYPE_USE}) -- the
no-@Target case is not a secret door to behavior that's otherwise
impossible or undesirable.

(We could have said "all contexts" for a no-@Target annotation type but
then added a rule in 9.7.4 to special-case a no-@Target annotation that
appears in an ambiguous location, e.g., to compile the annotation as if
it was applicable only in declaration contexts. More compatibility with
SE 8, but more complexity, and I don't think anyone really wants that.)

Turning to the compatibility concern: yes, JDK-8231435 shouldn't have
claimed behavioral compatibility. It was my error to not recognize what
Mike was saying in "It is a behavior change from the current
specification, but a small one that affects poorly-written programs that
have no @Target meta-annotation." Still, at the end of the day, it's no
more than a small behavioral incompatibility for annotation processors
(they may see more type annotations than they did before), and one which
was within the power of the annotation type's owner to induce anyway
(annotation processors that look for type annotations can reasonably be
expected to ignore type annotations in places they don't care about).

I have cc'd Mike in case he wants to add anything further, but the
design intent of "all contexts" still looks reasonable to me.

Alex

On 2/3/2021 5:29 AM, Joel Borggren-Franck wrote:

> Hi Alex,
>
> Looking at  the class file, and therefore reflection, there is a difference between “all declaration contexts” and “all contexts, declaration + type”. For the 5 ambiguous locations we would have to produce type annotation attributes if there is an annotation present whose declaration lack @Target. While this may or may not be desirable, and I’m leaning towards not, it is surely significant as it would change the semantics of reflective annotation processors in use.
>
> cheers
> /Joel
>
>> On 2 Feb 2021, at 19:50, Alex Buckley <[hidden email]> wrote:
>>
>> I initially proposed "all declaration contexts" as the smallest possible streamlining of the SE 7-oriented rule defined in SE 8. It would have ensured that legacy annotation types without @Target were locked out of the new world of type uses enabled in SE 8 by JSR 308. As it turned out, my conservative approach was unnecessary because Mike was fine with admitting those legacy annotation types for type uses via the "all contexts" rule. In the real world, "all declaration contexts" versus "all declaration contexts + all type contexts" is a distinction without a difference (that is, an insignificant distinction) -- we're best off adopting the latter rule, phrased simply as "all contexts".
>>
>> Alex
>>
>> On 2/2/2021 3:44 AM, Guoxiong Li wrote:
>>> On Tue, 2 Feb 2021 09:28:59 GMT, Joel Borggrén-Franck <[hidden email]> wrote:
>>>>> Thanks for the comments,
>>>>>
>>>>> @jbf I'll add a more focused test. For the bug, the original discussion in https://mail.openjdk.java.net/pipermail/compiler-dev/2019-September/013705.html is specifically about annotations without an explicit `@Target` applying to `MODULE`, I don't think there's anything that needs to be done for [JDK-8231436](https://bugs.openjdk.java.net/browse/JDK-8231436) besides supporting `MODULE`, would you still prefer a separate bug?
>>>>>
>>>>> @jddarcy note that there was already a spec change related to this in [JDK-8231435](https://bugs.openjdk.java.net/browse/JDK-8231435) and the spec bug mentions "this expansion is source, binary, and behaviorally compatible", should I still file a CSR?
>>>>
>>>>  From this follow up here: https://mail.openjdk.java.net/pipermail/compiler-dev/2019-September/013715.html my interpretation is that this bug was intended to widen it to all contexts, including type use. This has been changed in JLS but not in the normative javadoc and also not in the compiler. I believe we should keep declarations only, and back out the change to JLS.
>>> Maybe we should collect the concrete information about this problem. Here are some materials that I know.
>>> Order by time:
>>> - 2019.08 **Werner Dietl** launched the discussion. [1]
>>> - 2019.09 After the discussion between **Alex Buckley** and **Michael Ernst**, an initiative result came out.[2]
>>> - 2019.09 Two JBS issues were filed. [3][4]
>>> - 2019.12 **Alex Buckley** fixed the JLS(fix version: 14). But the compiler code and javadoc were not fixed. [5][6]
>>> - 2020.10 **Christian Stein** created another issue about it. [7]
>>> - 2020.10 **Guoxiong Li**(me) submitted a PR about it in Github. [8]
>>> - 2020.10 **Joel Borggrén-Franck** restarted the discussion about the specification. [9]
>>> - 2020.12 Because the JDK16 was nearly at RDP1, we decided to only fix JDK-8254023. And other work left to JDk17.  [7][10][11]
>>> - 2020.12 The issue JDK-8254023 was fixed at JDK16. [7][8][12]
>>> - **Now, we need to discuss the problem(unify the specification, implement and javadoc) that JDK16 has left.**[11]
>>> [1] https://mail.openjdk.java.net/pipermail/compiler-dev/2019-August/013666.html
>>> [2] https://mail.openjdk.java.net/pipermail/compiler-dev/2019-September/013705.html
>>> [3] https://bugs.openjdk.java.net/browse/JDK-8231435
>>> [4] https://bugs.openjdk.java.net/browse/JDK-8231436
>>> [5] https://docs.oracle.com/javase/specs/jls/se14/html/jls-9.html#jls-9.6.4.1
>>> [6] https://docs.oracle.com/en/java/javase/14/docs/api/java.base/java/lang/annotation/Target.html
>>> [7] https://bugs.openjdk.java.net/browse/JDK-8254023
>>> [8] https://github.com/openjdk/jdk/pull/622
>>> [9] https://mail.openjdk.java.net/pipermail/compiler-dev/2020-October/015197.html
>>> [10] http://openjdk.java.net/projects/jdk/16/
>>> [11] https://mail.openjdk.java.net/pipermail/compiler-dev/2020-December/015503.html
>>> [12] https://github.com/openjdk/jdk16/pull/34
>>> -------------
>>> PR: https://git.openjdk.java.net/jdk/pull/2303
>
Reply | Threaded
Open this post in threaded view
|

Re: RFR: 8231436: Fix the applicability of a no-@Target annotation type

Liam Miller-Cushon-2
In reply to this post by Liam Miller-Cushon-2
On Wed, 3 Feb 2021 18:20:15 GMT, Liam Miller-Cushon <[hidden email]> wrote:

>> Maybe we should collect the concrete information about this problem. Here are some materials that I know.
>>
>> Order by time:
>> - 2019.08 **Werner Dietl** launched the discussion. [1]
>> - 2019.09 After the discussion between **Alex Buckley** and **Michael Ernst**, an initiative result came out.[2]
>> - 2019.09 Two JBS issues were filed. [3][4]
>> - 2019.12 **Alex Buckley** fixed the JLS(fix version: 14). But the compiler code and javadoc were not fixed. [5][6]
>> - 2020.10 **Christian Stein** created another issue about it. [7]
>> - 2020.10 **Guoxiong Li**(me) submitted a PR about it in Github. [8]
>> - 2020.10 **Joel Borggrén-Franck** restarted the discussion about the specification. [9]
>> - 2020.12 Because the JDK16 was nearly at RDP1, we decided to only fix JDK-8254023. And other work left to JDk17.  [7][10][11]
>> - 2020.12 The issue JDK-8254023 was fixed at JDK16. [7][8][12]
>> - **Now, we need to discuss the problem(unify the specification, implement and javadoc) that JDK16 has left.**[11]
>>
>> [1] https://mail.openjdk.java.net/pipermail/compiler-dev/2019-August/013666.html
>> [2] https://mail.openjdk.java.net/pipermail/compiler-dev/2019-September/013705.html
>> [3] https://bugs.openjdk.java.net/browse/JDK-8231435
>> [4] https://bugs.openjdk.java.net/browse/JDK-8231436
>> [5] https://docs.oracle.com/javase/specs/jls/se14/html/jls-9.html#jls-9.6.4.1
>> [6] https://docs.oracle.com/en/java/javase/14/docs/api/java.base/java/lang/annotation/Target.html
>> [7] https://bugs.openjdk.java.net/browse/JDK-8254023
>> [8] https://github.com/openjdk/jdk/pull/622
>> [9] https://mail.openjdk.java.net/pipermail/compiler-dev/2020-October/015197.html
>> [10] http://openjdk.java.net/projects/jdk/16/
>> [11] https://mail.openjdk.java.net/pipermail/compiler-dev/2020-December/015503.html
>> [12] https://github.com/openjdk/jdk16/pull/34
>
> Thanks, I was missing context here. I now realize this patch is fixing a bug related to [JDK-8254023](https://bugs.openjdk.java.net/browse/JDK-8254023)--when the handling of annotations without `@Target` was updated to allow them to be applied to module declarations, the handling of repeatable annotations without `@Target` should also have been updated. I filed a new bug for the sub-issue that this patch is fixing, and will update it accordingly in a minute: https://bugs.openjdk.java.net/browse/JDK-8261088
>
> I'm also happy to help with any work to be done on the larger question of interpreting `@Target`-less annotations as applicable to type contexts, if there's agreement about what to do there.

I spun the original change that related specifically to repeatable annotations and `@Target(MODULE)` out into https://github.com/openjdk/jdk/pull/2412 and filed corresponding CSR [JDK-8261181](https://bugs.openjdk.java.net/browse/JDK-8261181).

I will look at updating this review to actually implement the change to make no-`@Target` annotations applicable to all contexts next.

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

PR: https://git.openjdk.java.net/jdk/pull/2303
Reply | Threaded
Open this post in threaded view
|

Re: RFR: 8231436: Fix the applicability of a no-@Target annotation type

Joel Borggren-Franck
In reply to this post by Alex Buckley-3
Hi,

Some comments inline,

> On 3 Feb 2021, at 19:39, Alex Buckley <[hidden email]> wrote:
>
> Let me focus first on the design question of what a no-@Target annotation type "means". There's no particular reason why it _shouldn't_ mean "all contexts". Mike gave good reasons why it should mean that. For the corner case of an annotation appearing in one of the ambiguous locations in source code, the annotation will appear in the class file/via reflection exactly as if someone had taken the perfectly legitimate course of spelling out @Target({METHOD, TYPE_USE}) -- the no-@Target case is not a secret door to behavior that's otherwise impossible or undesirable.
>

I agree that had we done this in one go from scratch, it seems likely that the “all contexts” default for no-@Target annotation would have been chosen.


> (We could have said "all contexts" for a no-@Target annotation type but then added a rule in 9.7.4 to special-case a no-@Target annotation that appears in an ambiguous location, e.g., to compile the annotation as if it was applicable only in declaration contexts. More compatibility with SE 8, but more complexity, and I don't think anyone really wants that.)

FYI this is what ecj does. They allow all contexts but don’t generate type annotation attributes from the ambiguous contexts.

> Turning to the compatibility concern: yes, JDK-8231435 shouldn't have claimed behavioral compatibility. It was my error to not recognize what Mike was saying in "It is a behavior change from the current specification, but a small one that affects poorly-written programs that have no @Target meta-annotation." Still, at the end of the day, it's no more than a small behavioral incompatibility for annotation processors (they may see more type annotations than they did before), and one which was within the power of the annotation type's owner to induce anyway (annotation processors that look for type annotations can reasonably be expected to ignore type annotations in places they don't care about).
>
> I have cc'd Mike in case he wants to add anything further, but the design intent of "all contexts" still looks reasonable to me.
>

In isolation yes, “all contexts” makes sense, but so does “all declaration contexts”. What worries me here is that we change the semantics of programs written as far back as 2004. While I don’t think this is an absolute no, we shouldn’t do this lightly, the gain should be substantial, and the risk well quantified. One aspect here is that while annotation interface declarations without @Target might very well be poorly written, it is/was still the only way to specify "all current and future declaration targets”, it is/was also the only alternative with some brevity to specify all current declaration contexts, the alternative being to type out 9 ElementTypes.

An alternative would be to adopt your original proposal, “all current and future declaration contexts”, and adding an ElementType corresponding to this default. This way the question “where are annotations applicable?” would be solved, the user can easily and with brevity opt in to all contexts by using something like “@Target({DECLARATIONS, TYPE_USE})" and the backward compatibility risk is lower. The downside is It would mean more work for us in implementing this.

cheers
/Joel
Reply | Threaded
Open this post in threaded view
|

Re: RFR: 8231436: Fix the applicability of a no-@Target annotation type

Alex Buckley-3
On 2/5/2021 6:44 AM, Joel Borggren-Franck wrote:

> In isolation yes, “all contexts” makes sense, but so does “all
> declaration contexts”. What worries me here is that we change the
> semantics of programs written as far back as 2004. While I don’t
> think this is an absolute no, we shouldn’t do this lightly, the gain
> should be substantial, and the risk well quantified. One aspect here
> is that while annotation interface declarations without @Target might
> very well be poorly written, it is/was still the only way to specify
> "all current and future declaration targets”, it is/was also the only
> alternative with some brevity to specify all current declaration
> contexts, the alternative being to type out 9 ElementTypes.

The original meaning of no-@Target in 2004 was open-ended, the "all
contexts" of its day -- "If a Target meta-annotation is not present on
an annotation type declaration, the declared type may be used on any
program element."

When Java 8 added annotations on declarations of type parameters (a
declaration context overlooked by Java 5, and addressed by JSR 308), we
should arguably have thrown that new declaration context into the "all
contexts" pot from Java 5 and leveraged Java 8's richer terminology to
state the meaning of no-@Target as "all declaration contexts". We didn't
do that out of an abundance of caution, not wanting to change the
semantics of programs written back to 2004. Instead, we gave what I now
regard as undue weight to what was important in 2012: Java 7's
locations, a.k.a. Java 5's locations. This led to a weird
backward-looking policy that does poorly when new declaration contexts
(MODULE, RECORD_COMPONENT) are added.

I think your dislike of no-@Target meaning "all contexts" is not that it
includes type contexts as such, but rather that when it meets the corner
case of ambiguous locations, it causes some annotations to be written
into the class file twice. If the corner case somehow didn't exist, I
think you would appreciate the brevity of being able to get all
declaration contexts without spelling out nine targets, and you just
wouldn't think about the type contexts coming along for the ride.

The corner case is a fact of Java life (and I might even concede it's
larger than a corner case, given the prominence of the ambiguous
locations), but I don't want it to dominate the decision about the
meaning of no-@Target. The simplicity of "all contexts" was appealing in
2004 and it's appealing in 2021. The cost of carelessly omitting @Target
was _always_ that an annotation processor might see @Foo in locations it
didn't expect as Java evolved, and now there may be some class file
overhead too. The power to avoid surprises is entirely in the hands of
Foo's owner -- they're declaring what looks like an API but is really a
new modifier for Java programs, so they ought to be concerned with how
their annotations affect user code.

Encouraging the owner to specify @Target by offering an
ElementType.DECLARATIONS constant that means "all [current and future]
declaration contexts" is a good idea, balancing how ElementType.TYPE_USE
means "all [current and future] type contexts".

(In Java 15, TYPE_USE implied 16 type contexts; in Java 16, TYPE_USE
implies 17 type contexts, the addition being the type in a record
component declaration. Open-ended rules work! They embody "lumping, not
splitting".)

Alex

> An alternative would be to adopt your original proposal, “all current
> and future declaration contexts”, and adding an ElementType
> corresponding to this default. This way the question “where are
> annotations applicable?” would be solved, the user can easily and
> with brevity opt in to all contexts by using something like
> “@Target({DECLARATIONS, TYPE_USE})" and the backward compatibility
> risk is lower. The downside is It would mean more work for us in
> implementing this.
>
> cheers /Joel
Reply | Threaded
Open this post in threaded view
|

Re: RFR: 8231436: Fix the applicability of a no-@Target annotation type

Liam Miller-Cushon
On Fri, Feb 5, 2021 at 6:45 AM Joel Borggren-Franck <[hidden email]> wrote:
An alternative would be to adopt your original proposal, “all current and future declaration contexts”, and adding an ElementType corresponding to this default. This way the question “where are annotations applicable?” would be solved, the user can easily and with brevity opt in to all contexts by using something like “@Target({DECLARATIONS, TYPE_USE})" and the backward compatibility risk is lower. The downside is It would mean more work for us in implementing this.

On Fri, Feb 5, 2021 at 12:15 PM Alex Buckley <[hidden email]> wrote:
Encouraging the owner to specify @Target by offering an
ElementType.DECLARATIONS constant that means "all [current and future]
declaration contexts" is a good idea, balancing how ElementType.TYPE_USE
means "all [current and future] type contexts".

I had a similar thought while updating the affected tests: `@Target(DECLARATIONS)` would have been useful for a number of them.

I don't have a strong opinion either way about whether this change is worth pursuing, or worth pursuing now. The simplicity of @Target-less annotations applying to everything is appealing. I also think there's going to be some noticeable compatibility impact, due to the number of annotations that will start appearing in type contexts, and the number of tools that process annotations that will need to become more robust in their handling of type annotations.

If there's a tentative agreement that making @Target-less annotations apply to everything together with introducing ElementType.DECLARATIONS is a reasonable path forward, I can work on a proposal for that.
Reply | Threaded
Open this post in threaded view
|

Re: [External] : Re: RFR: 8231436: Fix the applicability of a no-@Target annotation type

Alex Buckley-3
On 2/5/2021 4:18 PM, Liam Miller-Cushon wrote:
> I don't have a strong opinion either way about whether this change is
> worth pursuing, or worth pursuing now. The simplicity of @Target-less
> annotations applying to everything is appealing. I also think there's
> going to be some noticeable compatibility impact, due to the number of
> annotations that will start appearing in type contexts, and the number
> of tools that process annotations that will need to become more robust
> in their handling of type annotations.
Suppose that the no-@Target annotation @Foo is today a declaration
annotation on a method declaration, but tomorrow (due to recompiling Foo
and the aforementioned method declaration) @Foo becomes a declaration
annotation _and a type annotation that applies to the return type of the
method_. Hardly any annotation processor will care, because it'll be
calling Method::getAnnotations that only exposes declaration
annotations. Type annotations are exposed by a different API,
Method::getAnnotatedReturnType & friends.

Of course a type-checking annotation processor might be calling
Method::getAnnotatedReturnType to look for, say, @NonNull, but I have a
hard time believing that such a processor will be unable to cope with
@Foo's presence. And the owner of Foo can choose at any time to add an
@Target that makes @Foo appear as both declaration and type annotation.

Is there a compatibility impact I am overlooking?

Alex
Reply | Threaded
Open this post in threaded view
|

Re: [External] : Re: RFR: 8231436: Fix the applicability of a no-@Target annotation type

Liam Miller-Cushon
On Fri, Feb 5, 2021 at 4:52 PM Alex Buckley <[hidden email]> wrote:
Is there a compatibility impact I am overlooking?

I don't think there's anything major, it's just the usual concern that this is changing observable behaviour, and all observable behaviour of a system will be depended on by somebody [1].

The least contrived example I can think of is that an annotation processor might rely on TypeMirror#toString, and the string representation of types will more often include type annotations, which could affect logic in the processor or code that gets generated. (I think the javadoc bug I encountered in JDK-8261203 might be something like that, where it's converting an annotated type to a string and not escaping a string literal in the annotation element).

The additional attributes in the class file will also make them slightly larger, which I think will increase metaspace usage ever so slightly, which may be observable for some applications.

I agree that being able to add an explicitly @Target to annotations where this is undesired is a relatively good and inexpensive workaround for the cases that are affected.

If I was writing a CSR for this I'd probably estimate the compatibility impact as 'low'--I think it's more than 'minimal', and it may deserve a release note, but it's likely not prohibitively high. Does that sound right?
Reply | Threaded
Open this post in threaded view
|

Re: [External] : Re: RFR: 8231436: Fix the applicability of a no-@Target annotation type

Alex Buckley-3
On 2/5/2021 5:13 PM, Liam Miller-Cushon wrote:
> If I was writing a CSR for this I'd probably estimate the compatibility
> impact as 'low'--I think it's more than 'minimal', and it may deserve a
> release note, but it's likely not prohibitively high. Does that sound right?

Yes. The real issue is that _any_ open-ended definition, even the
middle-ground "all declaration contexts" one, can lead to
incompatibilities in processors when annotations are applied in places
newer than the processor. (Consider a no-@Target @Foo annotation being
applied to a record component declaration, and thus showing up as a
declaration annotation on both the corresponding component field and
accessor method, versus an annotation processor that assumed @Foo would
only ever appear on class declarations.)  We accept the low risk of
incompatibilities because open-ended definitions are simple and because
programmers can easily opt out by giving their desired applicability.

> [1] https://www.hyrumslaw.com/ 

a.k.a. Martin Buchholz's "Every change is an incompatible change."
Reply | Threaded
Open this post in threaded view
|

Re: RFR: 8231436: Fix the applicability of a no-@Target annotation type

Brian Goetz-2
In reply to this post by Alex Buckley-3
We appear to have painted ourselves into a corner quite nicely.

In Java 5, the interpretation of "a no-@Target annotation (NTA) is
applicable in all contexts" made perfect sense; you could use the
@Target meta-annotation to narrow the set of targets if you were fussy. 
Then along came type annotations, and we picked the sensible rule that
the default for an NTA would be all _declaration_ annotations.  This
avoided short-term incompatibility, but what it did was set type
annotations apart from all the other places where annotations can go.

I agree with Joel that:
> I agree that had we done this in one go from scratch, it seems likely that the “all contexts” default for no-@Target annotation would have been chosen.
but, we didn't do this in one go from scratch.  I also agree with Joe
that the reality seems to be that "one of these things is not like the
others", and that this is a decision best made by the author of the
annotation:

> To me, it seems that intending an annotation to be used on
> declarations or on types is a basic design decision of the annotation
> type.

The concern now seems mostly to be about "but, there are old moldy
annotations (most of them some spelling of "non null"), that we'd like
to be applicable in all places, but were written without a @Target, but
which we'd like to interpret as type annotations." Understandable, but
it seems that the fix for that is to convince the maintainers of those
annotations of this fact, not change the language to absolve them from
responsibility for updating their design intent in their code.

So I agree with Joe that:

> I think it can be reasonable to interpret lack of @Target to mean
> "usable in any declaration context," but ill-advised to silently
> reinterpret absence of a @Target to indicate types and declarations.

and prefer that we leave the existing interpretation of "all declaration
contexts" for NTAs.  Like raw types, which we convinced ourselves we
could get rid of eventually, sometimes this turns out to be wishful
thinking.






On 2/3/2021 1:39 PM, Alex Buckley wrote:

> Let me focus first on the design question of what a no-@Target
> annotation type "means". There's no particular reason why it
> _shouldn't_ mean "all contexts". Mike gave good reasons why it should
> mean that. For the corner case of an annotation appearing in one of
> the ambiguous locations in source code, the annotation will appear in
> the class file/via reflection exactly as if someone had taken the
> perfectly legitimate course of spelling out @Target({METHOD,
> TYPE_USE}) -- the no-@Target case is not a secret door to behavior
> that's otherwise impossible or undesirable.
>
> (We could have said "all contexts" for a no-@Target annotation type
> but then added a rule in 9.7.4 to special-case a no-@Target annotation
> that appears in an ambiguous location, e.g., to compile the annotation
> as if it was applicable only in declaration contexts. More
> compatibility with SE 8, but more complexity, and I don't think anyone
> really wants that.)
>
> Turning to the compatibility concern: yes, JDK-8231435 shouldn't have
> claimed behavioral compatibility. It was my error to not recognize
> what Mike was saying in "It is a behavior change from the current
> specification, but a small one that affects poorly-written programs
> that have no @Target meta-annotation." Still, at the end of the day,
> it's no more than a small behavioral incompatibility for annotation
> processors (they may see more type annotations than they did before),
> and one which was within the power of the annotation type's owner to
> induce anyway (annotation processors that look for type annotations
> can reasonably be expected to ignore type annotations in places they
> don't care about).
>
> I have cc'd Mike in case he wants to add anything further, but the
> design intent of "all contexts" still looks reasonable to me.
>
> Alex
>
> On 2/3/2021 5:29 AM, Joel Borggren-Franck wrote:
>> Hi Alex,
>>
>> Looking at  the class file, and therefore reflection, there is a
>> difference between “all declaration contexts” and “all contexts,
>> declaration + type”. For the 5 ambiguous locations we would have to
>> produce type annotation attributes if there is an annotation present
>> whose declaration lack @Target. While this may or may not be
>> desirable, and I’m leaning towards not, it is surely significant as
>> it would change the semantics of reflective annotation processors in
>> use.
>>
>> cheers
>> /Joel
>>
>>> On 2 Feb 2021, at 19:50, Alex Buckley <[hidden email]> wrote:
>>>
>>> I initially proposed "all declaration contexts" as the smallest
>>> possible streamlining of the SE 7-oriented rule defined in SE 8. It
>>> would have ensured that legacy annotation types without @Target were
>>> locked out of the new world of type uses enabled in SE 8 by JSR 308.
>>> As it turned out, my conservative approach was unnecessary because
>>> Mike was fine with admitting those legacy annotation types for type
>>> uses via the "all contexts" rule. In the real world, "all
>>> declaration contexts" versus "all declaration contexts + all type
>>> contexts" is a distinction without a difference (that is, an
>>> insignificant distinction) -- we're best off adopting the latter
>>> rule, phrased simply as "all contexts".
>>>
>>> Alex
>>>
>>> On 2/2/2021 3:44 AM, Guoxiong Li wrote:
>>>> On Tue, 2 Feb 2021 09:28:59 GMT, Joel Borggrén-Franck
>>>> <[hidden email]> wrote:
>>>>>> Thanks for the comments,
>>>>>>
>>>>>> @jbf I'll add a more focused test. For the bug, the original
>>>>>> discussion in
>>>>>> https://mail.openjdk.java.net/pipermail/compiler-dev/2019-September/013705.html 
>>>>>> is specifically about annotations without an explicit `@Target`
>>>>>> applying to `MODULE`, I don't think there's anything that needs
>>>>>> to be done for
>>>>>> [JDK-8231436](https://bugs.openjdk.java.net/browse/JDK-8231436)
>>>>>> besides supporting `MODULE`, would you still prefer a separate bug?
>>>>>>
>>>>>> @jddarcy note that there was already a spec change related to
>>>>>> this in
>>>>>> [JDK-8231435](https://bugs.openjdk.java.net/browse/JDK-8231435)
>>>>>> and the spec bug mentions "this expansion is source, binary, and
>>>>>> behaviorally compatible", should I still file a CSR?
>>>>>
>>>>>  From this follow up here:
>>>>> https://mail.openjdk.java.net/pipermail/compiler-dev/2019-September/013715.html 
>>>>> my interpretation is that this bug was intended to widen it to all
>>>>> contexts, including type use. This has been changed in JLS but not
>>>>> in the normative javadoc and also not in the compiler. I believe
>>>>> we should keep declarations only, and back out the change to JLS.
>>>> Maybe we should collect the concrete information about this
>>>> problem. Here are some materials that I know.
>>>> Order by time:
>>>> - 2019.08 **Werner Dietl** launched the discussion. [1]
>>>> - 2019.09 After the discussion between **Alex Buckley** and
>>>> **Michael Ernst**, an initiative result came out.[2]
>>>> - 2019.09 Two JBS issues were filed. [3][4]
>>>> - 2019.12 **Alex Buckley** fixed the JLS(fix version: 14). But the
>>>> compiler code and javadoc were not fixed. [5][6]
>>>> - 2020.10 **Christian Stein** created another issue about it. [7]
>>>> - 2020.10 **Guoxiong Li**(me) submitted a PR about it in Github. [8]
>>>> - 2020.10 **Joel Borggrén-Franck** restarted the discussion about
>>>> the specification. [9]
>>>> - 2020.12 Because the JDK16 was nearly at RDP1, we decided to only
>>>> fix JDK-8254023. And other work left to JDk17. [7][10][11]
>>>> - 2020.12 The issue JDK-8254023 was fixed at JDK16. [7][8][12]
>>>> - **Now, we need to discuss the problem(unify the specification,
>>>> implement and javadoc) that JDK16 has left.**[11]
>>>> [1]
>>>> https://mail.openjdk.java.net/pipermail/compiler-dev/2019-August/013666.html
>>>> [2]
>>>> https://mail.openjdk.java.net/pipermail/compiler-dev/2019-September/013705.html
>>>> [3] https://bugs.openjdk.java.net/browse/JDK-8231435
>>>> [4] https://bugs.openjdk.java.net/browse/JDK-8231436
>>>> [5]
>>>> https://docs.oracle.com/javase/specs/jls/se14/html/jls-9.html#jls-9.6.4.1
>>>> [6]
>>>> https://docs.oracle.com/en/java/javase/14/docs/api/java.base/java/lang/annotation/Target.html
>>>> [7] https://bugs.openjdk.java.net/browse/JDK-8254023
>>>> [8] https://github.com/openjdk/jdk/pull/622
>>>> [9]
>>>> https://mail.openjdk.java.net/pipermail/compiler-dev/2020-October/015197.html
>>>> [10] http://openjdk.java.net/projects/jdk/16/
>>>> [11]
>>>> https://mail.openjdk.java.net/pipermail/compiler-dev/2020-December/015503.html
>>>> [12] https://github.com/openjdk/jdk16/pull/34
>>>> -------------
>>>> PR: https://git.openjdk.java.net/jdk/pull/2303
>>

Reply | Threaded
Open this post in threaded view
|

Re: RFR: 8231436: Fix the applicability of a no-@Target annotation type

Alex Buckley-3
On 2/9/2021 6:34 AM, Brian Goetz wrote:
> So I agree with Joe that:
>
>> I think it can be reasonable to interpret lack of @Target to mean
>> "usable in any declaration context," but ill-advised to silently
>> reinterpret absence of a @Target to indicate types and declarations.
>
> and prefer that we leave the existing interpretation of "all declaration
> contexts" for NTAs.

Thanks Brian. I will proceed with that policy by filing a new spec bug
to overrule JDK-8231435.

A coarse DECLARATIONS target that explicitly indicates "all declaration
contexts" (and that serves as a dual to the coarse TYPE_USE target) can
be left for another day/thread/RFE.

Alex
12