trailing comment

[michael keith]

On 08/01/2013 8:27 PM, Alex Buckley wrote:
> 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.

I'm not convinced the compiler needs to verify both directions; I think 
it only needs to verify Foo. Foo is the annotation of interest, the one 
that will get modified during development. FooContainer is the 
uninteresting "support annotation" needed for repeatability and will 
likely be completely ignored once created. The only way for it to be 
"out of sync" with Foo is if somebody goes back and modifies 
FooContainer after having compiled Foo. In that unlikely case it is no 
different than if somebody removes a method from a class and recompiles 
the class, even though another class may be invoking the removed method. 
Either the IDE "saves" or figures out the referencer and an error shows 
up on the referencing class, or an exception occurs at runtime. In our 
annotation case, the IDE could do the same, i.e. remember the 
annotation/container pairs, or the AnnotationFormatError will be eagerly 
thrown when Foo is first loaded. That makes sense to me, and seems a 
more reasonable behavior than the incorrect and unexpected outcome in 
the scenario I described earlier.

In any case, it's not a showstopper issue, I just can't think of a good 
use case for differing @Inherited specs (can you?) but I do see some 
reasons for not allowing it.

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

Ok, thanks. Following the train of thought that led you to include that 
statement was the key.

>> - 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)}.

Sorry, yes, I had a brain fart on the first phrase.

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