[type-annos-observers] Comments on the Nov 7 specification

Tue Nov 20 14:31:25 PST 2012

Werner-

We will go with this proposal.  I'll write it up in the specification soon.

		    -Mike

> Subject: Re: [type-annos-observers] Comments on the Nov 7 specification
> From: Werner Dietl <wdietl at gmail.com>
> To: Alex Buckley <alex.buckley at oracle.com>,
>         type-annotations-spec-comments at openjdk.java.net
> Date: Tue, 13 Nov 2012 18:49:33 -0800
>
> Alex, experts,
>
> how about:
>
> struct type_path_entry {
>   u1 type_path_kind;
>     // 0: annotation is deeper in this array type
>     // 1: annotation is deeper in this nested type
>     // 2: annotation is on the bound of this wildcard type arg
>     // 3: annotation is on the i'th type arg of this parameterized type
>   u1 additional_index;
>     // 0: ignore me
>     // non-0: the 1st, 2nd, etc array index, nested type, or type arg
> of this parameterized type
> }
>
> That is, we use the additional_index (better name?) as argument for
> arrays, nested types, and type arguments.
>
> Instead of
>
> array_type_path { array_type_path {array_type_path {}}}
>
> one can then write:
>
> length: 1
> elements: array_type_path, 3.
>
> This applies in addition to the comment I already had about nested
> types, where I also think it would be easier to use a flag and an
> argument instead of repeating a flag multiple times.
> This would make handling of arrays, nested types, and type arguments
> nicely uniform.
>
> I like re-using one field instead of having a context-dependent union.
>
> About encoding everything within the flag: this will prevent any
> future extension of the type_path_kind.
> Also, how many type parameters can a class/method have? Are 253
> possible values enough? (Similarly for the nesting-depth of arrays and
> types.)
>
> cu, WMD.
>
>
> On Tue, Nov 13, 2012 at 6:20 PM, Alex Buckley <alex.buckley at oracle.com> wrote:
> > Experts,
> >
> > Werner Dietl sent the comments below about the spec in Mike's mail
> > "Improving the format of type annotation attributes" of 11/7/12.
> >
> > Most are useful clarifications to the spec, but one comment proposes an
> > array rather than a tree to represent the hierarchical location of a type
> > annotation in a compound type. Each level in the tree becomes the succeeding
> > array entry.
> >
> > The proposal has a union with a context-sensitive single member that would
> > break pack200, so here is a slightly modified version:
> >
> > struct type_path {
> >   u1              path_length;
> >   type_path_entry path[path_length];
> > }
> >
> > struct type_path_entry {
> >   u1 type_path_kind;
> >     // 0: annotation is deeper in this array type
> >     // 1: annotation is deeper in this nested type
> >     // 2: annotation is on the bound of this wildcard type arg
> >     // 3: annotation is on the i'th type arg of this parameterized type
> >   u1 type_argument_index;
> >     // 0: ignore me
> >     // non-0: the 1st, 2nd, etc type arg of this parameterized type
> > }
> >
> > I think this is fine. A further improvement would be to drop the
> > type_argument_index item and encode the i'th type arg into the
> > type_path_kind item. Namely, type_path_kind values >=3 represent the
> > type_path_kind-2'th type arg of the current parameterized type. This trick
> > is used in the stack_map_frame structure (JVMS 4.7.4).
> >
> > Alex
> >
> > **********
> >
> > page 16:
> > - The values for "path_type" are not specified. Something like Fig. 1
> > might be overkill, but I don't just want to assume that they are 0 - 4.
> >
> > - I'm not quite sure I see the advantage of the nested structure.
> > Couldn't we use an array instead? We are simply specifying a sequence
> > of steps to follow, not a complicated tree.
> >
> > struct type_path_entry {
> >   u1 path_type;
> >   union {
> >     // empty for most, only type_argument_path has info
> >     u1 type_argument_index;
> >   }
> > }
> >
> > struct type_path {
> >   u1 path_length;
> >   type_path_entry [ path_length ];
> > }
> >
> > The path_length could be used instead of or in addition to ending each
> > path with empty_path - like we can have either C-style null-terminated
> > strings or Pascal-style strings with length
> > I would prefer using path_length without empty_path.
> >
> > Then instead of:
> >
> > type_argument_path { 1, type_argument_path { 0, empty path } }
> >
> > we would have:
> >
> > length: 2;
> > elements: type_argument_path, 1, type_argument_path, 0;
> >
> > I think the resulting sequence of bytes would be the same (if
> > path_length is left off), but I would find the presentation as array a
> > lot simpler.
> > Am I missing what the advantage of this nested presentation is? If so,
> > I might be implementing it wrong
> >
> >
> > - On a related note, I find the representation of nested types weird. It
> > basically is "go one step up in the nesting". Similarly,
> > type_argument_path could be a "go one step right in the type
> > arguments" operation.
> > I find the solution for type arguments nicer and would suggest that we use:
> >
> >   union {
> >     // empty for most
> >     u1 type_argument_index;
> >     u1 outer_index;
> >   }
> >
> > The location for @M in the first example was:
> >
> > inner_type_path { inner_type_path { inner_type_path { empty_path } } }
> >
> > and would now simply be:
> >
> > length: 1;
> > elements: inner_type_path, 3;
> >
> > This is similar to the old way of counting, just ignoring any
> > array/type argument confusion and just counting the nesting. 0 is the
> > main modifier and left off, 1 is the first enclosing, etc. This way
> > the index corresponds to the number of "inner_type_path" elements in
> > the complicated/current way. We could use 0-based instead.
> >
> >
> > page 14:
> > - Section 3.3.6: I'm a bit amazed that the "throws_type_index" is a
> > u2, but the "method_parameter_index" is a u1. Can there really be so
> > many more exceptions than parameters?
> >
> > - I note that annotations in the signature are stored in the
> > "method_info structure", whereas annotations in method bodies are
> > stored in "a Code attribute".
> > Is this an intentional difference? Or is this something that wasn't
> > updated yet? If it's intentional, I would find a heads-up useful,
> > maybe together with Fig. 1 that discusses the different categories.
> >
> >
> > page 3:
> > - I would put "for casts" and "for type tests" together. I would
> > mention that there are no runtime checks for these.
> >
> > - In "for constructor invocation results":
> > In
> >   myVar . new @Tainted NestedClass
> > add "()" at the end.
> >
> > - Point 4 already talks about receivers, before point 5 introduced the
> > concept and syntax for that. Maybe the order should be switched?
> >
> > page 5:
> > - The last sentence of point 5 is the first to mention TYPE_USE. Would
> > it make sense to introduce the new ElementType constants earlier?
> >
> > - Point 6 could mention that it uses ElementType.TYPE_PARAMETER
> >
> > page 10:
> > - "Annotations that target instructions are _are_ those..."
> >
> > - In "How Java SE 7 stores annotations" we are reminded that there are
> > both Runtime[In]VisibleParameterAnnotations and
> > Runtime[In]VisibleAnnotations.
> > Would it help to highlight that in JSR 308 we do not add
> > Runtime[In]VisibleParameterTypeAnnotations and instead store such
> > annotations with the method?
> >
> > page 11:
> > - The last paragraph of Section 3.1 should also have a reference to
> > Section 3.4 and give a similar overview as for the other sections.
> >
> > page 12:
> > - Last sentence of Section 3.3 refers to generic type arguments and
> > arrays only. It should have the complete list with nested types, etc.
> >
> > - Copy & paste mistake: Section 3.3.1 mentions
> > "type_parameter_bound_target" and "bound".
> >
> > page 13:
> > - Section 3.3.2 could also mention that the index is 0-based, like
> > earlier and later subsections do (redundantly, I agree).
> >
> > **********
>
>
>
> --
> http://www.google.com/profiles/wdietl