Comments on the JSR 308 specification

[Werner Dietl]

Dear JSR 308 expert group and fellow implementers,

please find some comments about the specification (as of November 9th) below.

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

Best regards,
cu, WMD.

-- 
http://www.google.com/profiles/wdietl