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

[Werner Dietl]

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