Maurizio,

> On 3 Dec 2019, at 12:18, Maurizio Cimadamore <maurizio.cimadam...@oracle.com> 
> wrote:
> 
> Hi Chris,
> I like your proposal; first, it makes isRecord/getRecordComponents in sync 
> with other similar pair of methods like isArray/getComponentType. Secondly, 
> it allows tighter tests to be written e.g. by the JCK team, which is, I 
> think, where the value really is.

Right. The proposal tightly couples the pair of isRecord and
getRecordComponents - which as you subsequently point out, is just one
half of the problem. But an important part, which can be resolved
independently of the other part.

> In your updated sped we say:
> 
> "Returns {@code true} if and only if this class is a record class."
> 
> Which is fine, but it doesn't define precisely how to establish as to whether 
> a class _is_ a record class.
> 
> That is fine, I think, from a spec perspective, but I think that, moving 
> forward, the implementation should be more deeply tested to make sure that 
> the definition of what's a record and what's not remain stable over time. 
> More specifically, while establishing as to whether a class extends 
> j.l.Record is a straightforward binary check, ensuring that the record 
> attribute is present leads to problems - because the attribute could be 
> there, but parts of it might not be well-formed (e.g. one of the signature 
> attribute point to an invalid CP entry). In such cases we need (at least) an 
> internal agreement of what should happen - both at JVM classfile parse time 
> _and_ at javac classfile parse time (although I realize these might 
> implementation details).
> 
> In other words, I'm a bit concerned because, e.g. in enums we could piggy 
> back on ACC_ENUM, which is another straightforward binary check. Here we're 
> relying on a classfile attribute and, more specifically, on what happens when 
> wrong bits of such attributes are discovered by the JVM; this in principle 
> opens up issues where different VM implementations could have different 
> recovery strategy, which might in turn lead to different answer to 
> Class::isRecord.
> 
> The only way I see to tighten this up a bit is for the VM to reject a class 
> whose record attribute contains wrong bits, rather than throw the attribute 
> on the floor (like it happens with annotations).

At least from the JVM side of things, one could lean on the JVMS,
Section 4.1 “The ClassFile Structure”. While the draft records JVMS does
amend various sections and subsections of Chapter 4, it does not touch
section 4.1.  Reading between the lines, I think one way of ensuring the
well-formedness of the Record attribute would be to add a reference to
it from the top-level `attributes[]` format, e.g.

  "If a Java Virtual Machine implementation recognizes class files
   whose version number is XX.xx or above, it must recognize and
   correctly read the Record (§4.7.xx) attribute found in the attributes
   table of a ClassFile structure of a class file whose version number
   is XX.xx or above."

This is similar to the Signature, and a few other, attributes.

If we had this, then the implementation could rely on simply the
presence of a Record attribute, and no further checking would be
necessary.

-Chris.



Reply via email to