RFR: JEP 359-Records: reflection code

[Joe Darcy]

Hello,

A few comments on the 01 iteration:

For Class.isRecord, I suggest explicitly stating that java.lang.Record 
is *not* a record.

It would be reasonable for ElementType to make some reference to the 
special annotation-handling rules for records.

For the equals method operating on floating-point values, 
Double.compare/Float.compare comparing the result against 0 are likely 
to do what one wants more so than ==. I see the ObjectMethods 
implementation already does this.

For ObjectMethods, note that void.class is regarded by core reflection 
as being a primitive class and should likely be treated specially.

It seems odd their isn't a better return type than Object for the 
bootstrap method. Should the invokedynamic and dynamic constant cases be 
handled by the same method?

I recommend some more descriptive parameter names:

     @param theClass     the class hosting the components =>

     @param hostClass

At least in isolation, I don't think it is clear how the lookup object 
is used.

Cheers,

-Joe

On 10/27/2019 4:41 PM, Vicente Romero wrote:
> Hi Brian,
>
> Thanks for your comments. I have published another review iteration 
> [1]. Some comments inlined below.
>
> On 10/25/19 5:04 PM, Brian Goetz wrote:
>> Spec in Class.java:
>>
>>  - Various {@code RecordComponent} should be {@link RecordComponent}.
>
> we could do that but the current approach is consistent with what we 
> are doing in similar APIs like Class::getDeclaredFields()
>
>>  - Rather than say "if this class was declared as a record in the 
>> source code", instead say "is a record class", since record classes 
>> are a term defined by the JLS.
> fixed
>>  - For isRecord/getRecordComponents, should include "@jls 8.10" link
> done
>>
>> RecordComponent.java:
>>
>>  - Add @jls 8.10 link
>>  - "Returns the name of the component represented by this record 
>> component." -> "Returns the name of this record component." and 
>> similar in other methods.  Since RC models a record component, its 
>> clear enough what is meant.
>
> done, the javadoc was following the same approach as in 
> java.lang.reflect.Field
>
>>  - For getGenericSignature, there should be a link back to @jls or 
>> @jvms that describes the format of the signature string.
> done, added @jvms 4.7.9.1 Signatures
>
>>  - Doesn't getAnnotation() need some spec, or an {@inheritDoc}?
>
> yep done
>
> Thanks,
> Vicente
>
> [1] 
> http://cr.openjdk.java.net/~vromero/records.review/reflection/webrev.01/