RFR: 8004823: Add VM support for type annotation reflection

[John Rose]

This is good work.  You can count me as a reviewer.

One comment:  There are a couple of places where a field index is used as an annotation array index (one old one new).  I think it would be best to put an explicit range check like this:

-  if (md == NULL)
+  if (md == NULL || index() >= md->length())
     return NULL;
   return md->at(index());

There are two reasons:  First, although it is likely this is a needless check, the code which ensures length equality for field arrays and annotation arrays is buried in another file (classFileParser) and hard to check.  Second, recent changes to classFileParser actually decouple the field array length from the value declared in the class file, by injecting extra fields into certain system classes.  Although it is (again) unlikely that this will cause a problem, putting a local range check in the code above guarantees an adequate level of safety.

The "at" method includes an assertion check, which is also good, but assertions are (a) not enabled in product mode and (b) can crash the VM if they fail.

— John

On Dec 18, 2012, at 9:05 AM, Joel Borggrén-Franck wrote:

> New webrev up:
> 
> http://cr.openjdk.java.net/~jfranck/8004823/webrev.v7/
> 
> - I had to rename the field name for the type annotation byte[] in Field, Constructor and Method.
> 
> Also fixed:
> 
> - I fixed the mapfile that got reindented.
> - Added back a comment that I accidentally removed.