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

[cushon]

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.

---

Progress

• Change must not contain extraneous whitespace
• Commit message must refer to an issue
• Change must be properly reviewed

Integration blocker

 ⚠️ The change requires a CSR request to be approved.

Issue

• JDK-8231436: Fix the applicability of a no-@target annotation type

Download

$ git fetch https://git.openjdk.java.net/jdk pull/2303/head:pull/2303
$ git checkout pull/2303

[bridgekeeper]

👋 Welcome back cushon! A progress list of the required criteria for merging this PR into master will be added to the body of your pull request. There are additional pull request commands available for use with this pull request.

[openjdk]

@cushon The following label will be automatically applied to this pull request:

• compiler

When this pull request is ready to be reviewed, an "RFR" email will be sent to the corresponding mailing list. If you would like to change these labels, use the /label pull request command.

[mlbridge]

Webrevs

• 01: Full - Incremental (7e1c404)
• 00: Full (b0d6abbc2f156b72ec2e64fe5e91a35521643229)

[jbf]

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?

[jddarcy]

/csr

[openjdk]

@jddarcy has indicated that a compatibility and specification (CSR) request is needed for this pull request.
@cushon please create a CSR request and add link to it in JDK-8231436. This pull request cannot be integrated until the CSR request is approved.

[jddarcy]

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

[cushon]

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 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 and the spec bug mentions "this expansion is source, binary, and behaviorally compatible", should I still file a CSR?

[jbf]

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.

[lgxbslgx]

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] #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] openjdk/jdk16#34

[mlbridge]

Mailing list message from Alex Buckley on compiler-dev:

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:

[mlbridge]

Mailing list message from Joel Borggren-Franck on compiler-dev:

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 <alex.buckley at oracle.com> 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:

[mlbridge]

Mailing list message from Joe Darcy on compiler-dev:

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 <alex.buckley at oracle.com> 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:

[cushon]

Thanks, I was missing context here. I now realize this patch is fixing a bug related to 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.

[mlbridge]

Mailing list message from Alex Buckley on compiler-dev:

Let me focus first on the design question of what a no- at 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- at Target case is not a secret door to behavior that's otherwise
impossible or undesirable.

(We could have said "all contexts" for a no- at Target annotation type but
then added a rule in 9.7.4 to special-case a no- at 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 <alex.buckley at oracle.com> 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:

[cushon]

I spun the original change that related specifically to repeatable annotations and @Target(MODULE) out into #2412 and filed corresponding CSR JDK-8261181.

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

[cushon]

I pushed an initial implementation of a fix to make no-@Target annotation applicable in all contexts, including type contexts.

I have updated affected tier 1 langtools tests, which provides some signal about the compatibility impact. That also uncovered a couple of bugs: one issue where javadoc isn't escaping type annotations in html (JDK-8261203), and a crash with intersection types and LVTI (JDK-8261205).

[mlbridge]

Mailing list message from Joel Borggren-Franck on compiler-dev:

Hi,

Some comments inline,

On 3 Feb 2021, at 19:39, Alex Buckley <alex.buckley at oracle.com> wrote:

Let me focus first on the design question of what a no- at 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- at 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- at Target annotation would have been chosen.

(We could have said "all contexts" for a no- at Target annotation type but then added a rule in 9.7.4 to special-case a no- at 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

[mlbridge]

Mailing list message from Alex Buckley on compiler-dev:

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- at 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- at 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- at 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- at 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

[mlbridge]

Mailing list message from Liam Miller-Cushon on compiler-dev:

On Fri, Feb 5, 2021 at 6:45 AM Joel Borggren-Franck <
joel.p.borggren-franck at oracle.com> 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 <alex.buckley at oracle.com>
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.
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <https://mail.openjdk.java.net/pipermail/compiler-dev/attachments/20210205/251fb622/attachment.htm>

[mlbridge]

Mailing list message from Alex Buckley on compiler-dev:

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- at 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

[mlbridge]

Mailing list message from Liam Miller-Cushon on compiler-dev:

On Fri, Feb 5, 2021 at 4:52 PM Alex Buckley <alex.buckley at oracle.com> 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
<https://bugs.openjdk.java.net/browse/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?

[1] https://www.hyrumslaw.com/ <https://www.hyrumslaw.com/>
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <https://mail.openjdk.java.net/pipermail/compiler-dev/attachments/20210205/46ebc845/attachment-0001.htm>

[mlbridge]

Mailing list message from Alex Buckley on compiler-dev:

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- at 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."

[mlbridge]

Mailing list message from Brian Goetz on compiler-dev:

We appear to have painted ourselves into a corner quite nicely.

In Java 5, the interpretation of "a no- at 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- at 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- at 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- at Target case is not a secret door to behavior
that's otherwise impossible or undesirable.

(We could have said "all contexts" for a no- at Target annotation type
but then added a rule in 9.7.4 to special-case a no- at 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 <alex.buckley at oracle.com> 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:

[mlbridge]

Mailing list message from Alex Buckley on compiler-dev:

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

[mlbridge]

Mailing list message from Liam Miller-Cushon on compiler-dev:

On Thu, Feb 11, 2021 at 9:52 AM Alex Buckley <alex.buckley at oracle.com>
wrote:

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.

Thanks all. I will close this review.

To confirm, is there still consensus that "all declaration contexts" for
NTAs includes declaration contexts introduced in later language levels,
i.e. NTAs should be applicable to module declarations in Java >= 9, and
that it's OK to proceed with JDK-8261088?
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <https://mail.openjdk.java.net/pipermail/compiler-dev/attachments/20210211/cfad4665/attachment.htm>

[mlbridge]

Mailing list message from Alex Buckley on compiler-dev:

On 2/11/2021 11:28 AM, Liam Miller-Cushon wrote:

On Thu, Feb 11, 2021 at 9:52 AM Alex Buckley <alex.buckley at oracle.com
<mailto:alex.buckley at oracle.com>> wrote:

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\.

Thanks all. I will close this review.

I have filed spec bug https://bugs.openjdk.java.net/browse/JDK-8261610

Since the javac bug JDK-8231436 never got beyond New status, I think it
would be reasonable to use it as the carrier for javac changes connected
with the new spec bug.

To confirm, is there still consensus?that "all declaration contexts" for
NTAs includes declaration contexts introduced in later language levels,
i.e. NTAs should be applicable to module declarations in Java >= 9, and
that it's OK to proceed with?JDK-8261088?

Yes. The spec bug discusses the open-ended nature of "all declaration
contexts", including that it captures module declarations added in 9.
For JDK-8261088, the anno.interface T (applicable in all declaration
contexts, including modules) should be able to nominate a containing
anno.interface that's applicable in only the module declaration context.

Alex

[mlbridge]

Mailing list message from Michael Ernst on compiler-dev:

I concur with Alex: an annotation defined without a @Target annotation
should be applicable to all contexts.
Some wording goofs have caused confusion about the intent and the effect,
but we have an opportunity now to clarify that.

Mike

On Wed, Feb 3, 2021 at 10:39 AM Alex Buckley <alex.buckley at oracle.com>
wrote:

Let me focus first on the design question of what a no- at 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- at Target case is not a secret door to behavior that's otherwise
impossible or undesirable.

(We could have said "all contexts" for a no- at Target annotation type but
then added a rule in 9.7.4 to special-case a no- at 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 <alex.buckley at oracle.com> 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 <
jfranck at openjdk.org> 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//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

-------------- next part --------------
An HTML attachment was scrubbed...
URL: <https://mail.openjdk.java.net/pipermail/compiler-dev/attachments/20210515/fde049d8/attachment.htm>

[mlbridge]

Mailing list message from Michael Ernst on compiler-dev:

I concur with Alex: an annotation defined without a @Target annotation
should be applicable to all contexts.
Some wording goofs have caused confusion about the intent and the effect,
but we have an opportunity now to clarify that.

Mike

On Wed, Feb 3, 2021 at 10:39 AM Alex Buckley <alex.buckley at oracle.com>
wrote:

Let me focus first on the design question of what a no- at 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- at Target case is not a secret door to behavior that's otherwise
impossible or undesirable.

(We could have said "all contexts" for a no- at Target annotation type but
then added a rule in 9.7.4 to special-case a no- at 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 <alex.buckley at oracle.com> 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 <
jfranck at openjdk.org> 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//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

-------------- next part --------------
An HTML attachment was scrubbed...
URL: <https://mail.openjdk.java.net/pipermail/compiler-dev/attachments/20210515/fde049d8/attachment.htm>