[Checkers] Compiler annotations output

[Mahmood Ali]

On Jul 2, 2008, at 10:01 AM, Michael Ernst wrote:

>> For some reason, now section 4.1 makes a bit [more] sense for me ...
>
> Great.  If you have suggestions regarding how it could be further  
> improved
> (or whether some commentary elsewhere in the document would be  
> useful),
> please let me know.

The document is written in the specification format, and not in the  
instructive/tutorial/guide style.  I assume that through me off a  
bit.  The major difficulty for me the first times I read the proposals  
was understanding why we needed those fields.  Now that I worked with  
the compiler for a bit and been thinking about the problem, I see  
what's going on.

Here are my general comments about section 4:

4  Class file format extensions

- the sections intermixed the proposed changes along with what the  
current specification is.  I found it to be a bit confusing.  I don't  
think that separating them is necessary better.
    * I might prefer a link to the current specifications with  
attributions rather than linking to Blo04 and LY.  LY is not available  
yet, and Blo04 link does seem to contain the technical information  
necessary to understand the new changes.
    * Example of the undesired intermix is the sentance 'Class files  
already store annotations in the form of “attributes”' in the second  
paragraph.  Attributes are defined only a paragraph later.  The  
attribution point doesn't serve much purpose here in my opinion.

- I assume is evident that Runtime[In]visibleTypeAnnotations for  
annotation is associated with the most enclosing program element  
(class, field, or method).  I didn't get piece of information the  
first few times I read it.

- second paragraph mentions local fields but not method parameters.   
Are these stored?  If so, Are these kept along with the method symbols  
and how the target of the annotation is identified if are stored with  
the method symbol?  Why aren't using them to store annotations for  
method parameter generic types?  If not, maybe we can say something  
about that.

- The second to last paragraph (right before the TODO item) explains  
the new needed structures (e.g. field_info) without explaining their  
purpose. A possible suggestion is having the second sentence saying  
something like "

- References to JVMS3 just threw me off.  I couldn't find any online  
document regarding it and it took me awhile later to see that it's the  
LY 'to appear' reference.  It might be nice to reference JVMS2 when  
referred sections are already there.

- In the section as whole, it's not clear what an annotation is  
targetting.  In instances we say that an annotation is target a  
program element.  In my understanding program elements are symbols  
(like class, method, variables, fields) not expressions or types.  Are  
generic/arrays targets program elements in your definition?  What  
about typecasts and expressions?

4.1  The extended_annotation structure

- target_type and reference_info comments weren't helpful.  Both said  
'new in JSR 308: where the annotation appears'.  I would rather have  
extended_annotation declared as follows:

extended_annotation {
     // original fields from 'annotation'
     u2 type_index;
     u2 num_element_value_pairs;
     {
         u2 element_name_index;
         element_value value;
     } element_value_pairs[num_element_value_pairs];
     // new fields in JSR 308
     u1 target_type;    // the type of the targeted program element
     {
         ...
     } reference_info;  // field to uniquely identify the target  
bytecode
}

4.1.1  The target_type field
- the same issue about the target for the annotation.

4.1.2  The reference_info field

- It might be nice for it to say something along the lines of:

reference_info is a structure contains enough information to uniquely  
identify the target of the annotations.  Each value of target_type may  
require different set of fields.  The describtion is as follows:


- Mahmood