trailing comment

[Alex Buckley]

Hi Mike,

On 1/7/2013 7:03 PM, michael keith wrote:
> - p. 4 - Is there a use case for allowing @FooContainer to be
> @Documented/@Inherited when @Foo is not? In most cases seemingly
> arbitrary limitations are bad, but in this case I don't see a need for
> allowing it, and it might actually be detrimental to do so. For example,
> if I make a mistake as the annotation developer and during the course of
> development I no longer want @Foo to be @Inherited so I remove the
> meta-annotation from @Foo but forget to remove it from @FooContainer.
> When a user goes to use @Foo and annotates their UserClass with it their
> UserSubClass won't see it, as expected. However, if they add another
> @Foo then both will be wrapped in a @FooContainer and now UserSubClass
> will be able to see them using getAllAnnotations(Foo.class) because the
> container is inherited. I am just wondering if the feature was motivated
> by a case that someone thought would be useful, or if it was a case of
> trying not to impose unnecessary limitations. In general I am concerned
> about the maintenance issue of keeping the two annotations in sync. If
> they are allowed to be out of sync then the IDE won't be able to help me.

Simplicity a.k.a. "both are @Inherited, or neither is" is good, but 
unnatural because there's no solid link from a containing annotation 
type to a repeatable annotation type. Consider these types:

@Inherited
@Repeatable(FooContainer.class)
@interface Foo {}

@Inherited
@interface FooContainer { Foo[] value; }

@Inherited
@interface RandomContainer { Foo[] value; }

When compiling Foo, you can't fail to notice @Repeatable, so it's 
reasonable to load up the named FooContainer and enforce 
"Foo-is- at Inherited --> FooContainer-is- at Inherited".

When compiling FooContainer and RandomContainer, you don't check 
anything about Foo. The presence of Foo in a return type is not 
significant enough for a compiler to load Foo and ponder a two-way 
relationship. It would be a wasted effort for RandomContainer, a 
perfectly legal type which has nothing to do with repeatability. For 
FooContainer, the compiler would be happy that it and Foo are 
@Inherited, but that's not authoritative as I can swap in a 
non- at Inherited FooContainer.class at runtime. (That's why core 
reflection has the AnnotationFormatError check on page 10.)

In summary, I think the compiler should not be expected to "save" an 
annotation type owner who removes @Inherited from Foo but leaves it on 
FooContainer.

> - p. 6 - The note after the first para in 9.7 states:
>      "This rule implements the policy that an annotation may repeat at
> only some of the locations where the annotation may appear. See §9.6 for
> more details."
> I don't see how the first para of 9.7 defines that behavior. Am I
> missing something? (It is defined in section 9.6 by the allowance of
> different Targets, in any case.)

We have a policy that an annotation may repeat at only some of the 
locations where the annotation may appear. This is encoded in 9.6's 
rules about targets, but it's not enforced until someone actually writes 
@Foo @Foo at a given location - then 9.7 comes into play by requiring 
the containing type to be legal there.

The reason I also require the repeatable type to be legal there is 
because I plan in JLS8 to centralize rules about annotation legality 
which are scattered throughout JLS7.

> - p. 9 - Feels like a typo, but the para that begins:  "An annotation A
> is directly present on an element E if ... " should finish by saying
> "... contains exactly one annotation C whose type is the containing
> annotation type of A's type (§9.6) and whose value element contains *at
> least one annotation of type* A."

That would allow @A(1) to be directly present on an element if there's a 
container annotation with value={@A(2)}.

> - p. 18 - Cut/paste typo in para that begins: "Now suppose Foo is made
> repeatable ...". The bracketed part should be removed since it was
> stated above that FooContainer was inheritable (Foo may or may not be
> inheritable).

Thanks, corrected.

> - p. 18 - Missing closing bracket in
> "B.class.getAllDeclaredAnnotations(FooContainer.class = [ ]"

Thanks, corrected.

Alex