<html><body><div style="font-family: arial, helvetica, sans-serif; font-size: 12pt; color: #000000"><div><br></div><div><br></div><hr id="zwchr" data-marker="__DIVIDER__"><div data-marker="__HEADERS__"><blockquote style="border-left:2px solid #1010FF;margin-left:5px;padding-left:5px;color:#000;font-weight:normal;font-style:normal;text-decoration:none;font-family:Helvetica,Arial,sans-serif;font-size:12pt;"><b>De: </b>"Maurizio Cimadamore" <maurizio.cimadamore@oracle.com><br><b>À: </b>"Remi Forax" <forax@univ-mlv.fr>, "Chris Hegarty" <chris.hegarty@oracle.com><br><b>Cc: </b>"amber-spec-experts" <amber-spec-experts@openjdk.java.net>, "joe darcy" <joe.darcy@oracle.com><br><b>Envoyé: </b>Mardi 3 Décembre 2019 22:40:11<br><b>Objet: </b>Re: Clarifying record reflective support<br></blockquote></div><div data-marker="__QUOTED_TEXT__"><blockquote style="border-left:2px solid #1010FF;margin-left:5px;padding-left:5px;color:#000;font-weight:normal;font-style:normal;text-decoration:none;font-family:Helvetica,Arial,sans-serif;font-size:12pt;"><p>I feel torn here - on the one hand, flash-forward in time and
      we'll have a way to transform isRecord/getRecordComponents and
      isArray/getComponentType pairs into deconstruction patterns. If we
      go down that route, it is actually beneficial if the set of return
      values for getRecordComponents and getComponentType is disjoint in
      the two cases (record/non-record, array/non-array). Because then
      you can turn "returns null" into "does not match" and everything
      else follows pretty nicely from that.</p></blockquote><div><br></div><div>I don't follow you, conceptually a pattern matching is an optional of tuples (i don't want to go more deep than that given that the real implementation is still in flux), you can construct an optional if getReturnComponents() returns null or not.<br data-mce-bogus="1"></div><div><br data-mce-bogus="1"></div><blockquote style="border-left:2px solid #1010FF;margin-left:5px;padding-left:5px;color:#000;font-weight:normal;font-style:normal;text-decoration:none;font-family:Helvetica,Arial,sans-serif;font-size:12pt;">
    <p>That said, it is true that all methods in j.l.Class which return
      arrays return 0-length arrays; for instance, getClasses doc says:</p>
    <p>
      </p><blockquote>
        <div class="block">Returns an array containing <code>Class</code>
          objects representing all the public classes and interfaces
          that are members of the class represented by this <code>Class</code>
          object. This includes public class and interface members
          inherited from superclasses and public class and interface
          members declared by the class. This method returns an array of
          length 0 if this <code>Class</code> object has no public
          member classes or interfaces. This method also returns an
          array of length 0 if this <code>Class</code> object
          represents a primitive type, an array class, or void.</div>
      </blockquote>
      <br>
      On the other hand, there something off about "getRecordComponents"
      because the very name of the method _screams_ that it should only
      really be called on record classes. In fact, if we were writing an
      API form scratch, it wouldn't be unreasonable to throw
      UnsupportedOperationException if this method was called when
      isRecord() == false. So, the very biased nature of the
      "getRecordComponents" method is what makes me uncomfortable in
      just saying "return a zero-length array". If you are calling a
      method that's only really supposed to be called on a record on
      something that's not a record, wouldn't you want to know about it?</blockquote><div><br></div><div>If we start from scratch we will use an optional which delay the choice in between null, an empty array or throwing an exception and let the user choose.<br data-mce-bogus="1"></div><div>I think it's not reasonable to have an Optional that pop into the java.lang.reflect API because it will not be consistent with the rest of the API.<br data-mce-bogus="1"></div><div>I pretty sure there is an item in Effective Java saying that It's bad practice to have a method that return null instead of an empty array.</div><div><br data-mce-bogus="1"></div><blockquote style="border-left:2px solid #1010FF;margin-left:5px;padding-left:5px;color:#000;font-weight:normal;font-style:normal;text-decoration:none;font-family:Helvetica,Arial,sans-serif;font-size:12pt;"><br>
    
    <p>Maurizio</p></blockquote><div><br></div><div>Rémi<br data-mce-bogus="1"></div><div><br data-mce-bogus="1"></div><blockquote style="border-left:2px solid #1010FF;margin-left:5px;padding-left:5px;color:#000;font-weight:normal;font-style:normal;text-decoration:none;font-family:Helvetica,Arial,sans-serif;font-size:12pt;"><p><br>
    </p>
    <div class="moz-cite-prefix">On 03/12/2019 21:10, Remi Forax wrote:<br>
    </div>
    <blockquote cite="mid:768716347.1000827.1575407454834.JavaMail.zimbra@u-pem.fr">
      <pre class="moz-quote-pre">Hi Chris,

Please do not add another java.io.File.listFiles() !

People will test a code that use Class.getRecordComponents() only with records forgetting as usual to have negative tests and it will blow in production.

A method that returns an array should never return null.
I repeat
A method that returns an array should never return null.

If someone want to disambiguate between i've no record component because i'm a record or i've no record component because i'm not a record, isRecord is exactly the kind of method you want to call.

regards,
Rémi


----- Mail original -----
</pre>
      <blockquote>
        <pre class="moz-quote-pre">De: "Chris Hegarty" <a class="moz-txt-link-rfc2396E" href="mailto:chris.hegarty@oracle.com" target="_blank"><chris.hegarty@oracle.com></a>
À: "amber-spec-experts" <a class="moz-txt-link-rfc2396E" href="mailto:amber-spec-experts@openjdk.java.net" target="_blank"><amber-spec-experts@openjdk.java.net></a>, "joe darcy" <a class="moz-txt-link-rfc2396E" href="mailto:joe.darcy@oracle.com" target="_blank"><joe.darcy@oracle.com></a>, "Maurizio Cimadamore"
<a class="moz-txt-link-rfc2396E" href="mailto:maurizio.cimadamore@oracle.com" target="_blank"><maurizio.cimadamore@oracle.com></a>
Envoyé: Mardi 3 Décembre 2019 12:26:53
Objet: Clarifying record reflective support
</pre>
      </blockquote>
      
      <blockquote>
        <pre class="moz-quote-pre">A number of issues and/or concerns have been come up recently relating
to the reflective support for records. These arose when finalizing and
completing the runtime CSR. Taken together they seem to lead back to a
few small, but significant, omissions in the spec that would be good to
tighten up. It is important that it be possible to reflectively reason
about record classes in a way that is unambiguous and provides
certainty that the record class is well-formed.

For a class to be a record class, then:

 1) It's direct superclass must be java.lang.Record, and
 2) It must have a Record attribute.

The entry point to the reflective API for records is the two methods:
Class::isRecord and Class::getRecordComponents.

The isRecord method should, in it's specification, guaranteed both of
point 1 and point 2 above. That is to say, for a class to be considered
a record class, then its direct superclass must be java.lang.Record, and
it must have a Record attribute. The implementation already behaves
this way, but the specification should require it.

The getRecordComponents method currently returns an empty array for both
a record class with no components and a non-record class. We thought it
kinda nice to be able to avoid returning null, but with hindsight I
think that it would be better to remove this potential ambiguity. The
getRecordComponents method should only return a non-null value if both
point 1 ( the class is a record class ) and point 2 above are true.
There are many other null returning methods in Class, so this is not
unusual or out of place. The implementation only requires a minor change
to support this ( return null for non-record classes ).

The most significant part of the changes proposed are to the
specification, so I've included that here inline. The proposed changes
tightly couple the pair of methods as part of their specification,
something that will hopefully be cleaner to do (or even unnecessary)
when we have full pattern matching.


/src/java.base/share/classes/java/lang/Class.java

    /**
     * {@preview Associated with records, a preview feature of the Java language.
     *
     *           This method is associated with <i>records</i>, a preview
     *           feature of the Java language. Preview features
     *           may be removed in a future release, or upgraded to permanent
     *           features of the Java language.}
     *
     * Returns {@code true} if and only if this class is a record class.
-     * It returns {@code false} otherwise. Note that class {@link Record} is
not a
-     * record type and thus invoking this method on class {@link
java.lang.Record}
-     * returns {@code false}.
-     *
-     * @return true if and only if this class is a record class
+     *
+     * <p> The {@linkplain #getSuperclass() direct superclass} of a record
+     * class is {@code java.lang.Record}. A record class has (possibly empty)
+     * record components, that is, {@link #getRecordComponents()} returns a
+     * non-null value.
+     *
+     * <p> Note that class {@link Record} is not a record type and thus
invoking
+     * this method on class {@code Record} returns {@code false}.
+     *
+     * @return true if and only if this class is a record class, otherwise
false
     * @jls 8.10 Record Types
     * @since 14
     */
    public boolean isRecord() { ... }

    /**
     * {@preview Associated with records, a preview feature of the Java language.
     *
     *           This method is associated with <i>records</i>, a preview
     *           feature of the Java language. Preview features
     *           may be removed in a future release, or upgraded to permanent
     *           features of the Java language.}
     *
-     * Returns an array containing {@code RecordComponent} objects reflecting
all the
-     * declared record components of the record represented by this {@code
Class} object.
-     * The components are returned in the same order that they are declared in
the
-     * record header.
-     *
-     * @return  The array of {@code RecordComponent} objects representing all
the
-     *          record components of this record. The array is empty if this
class
-     *          is not a record, or if this class is a record with no
components.
+     * Returns an array of {@code RecordComponent} objects representing all the
+     * record components of this record class, or {@code null} if this class is
+     * not a record class.
+     *
+     * <p> The components are returned in the same order that they are declared
+     * in the record header. The array is empty if this record class has no
+     * components. If the class is not a record class, that is {@link
+     * #isRecord()} returns false, then this method returns null. Conversely,
if
+     * {@link #isRecord()} returns true, then this method returns a non-null
+     * value.
+     *
+     * @return  An array of {@code RecordComponent} objects representing all
the
+     *          record components of this record class, or {@code null} if this
+     *          class is not a record class
     * @throws  SecurityException
     *          ...
     *
     * @jls 8.10 Record Types
     * @since 14
     */
    public RecordComponent[] getRecordComponents() { … }


-Chris.
</pre>
      </blockquote>
    </blockquote><br></blockquote></div></div></body></html>