RFR: 8298065: Provide more information in message of NoSuchFieldError [v7]

Ioi Lam iklam at openjdk.org
Tue Jan 10 16:18:53 UTC 2023


On Tue, 10 Jan 2023 07:15:57 GMT, David Holmes <dholmes at openjdk.org> wrote:

>> Sorry I am a bit obsessed with names.
>> 
>> The two existing methods are used only for method signatures. They each print *a portion* of the signature. For example, when given a signature like "(ZI)J":
>> 
>> - print_as_signature_external_parameters() prints `boolean, int`. Note that the parentheses aren't printed
>> - print_as_signature_external_return_type() prints `long`
>> 
>> Matias's new function is used only for field signatures. David's suggestion of "print_as_signature_external_field_type" is not consistent with the existing names (which do not say "method").
>> 
>> While "print_as_signature_external_parameters()" can be read as "print the parameters of a (method) signature in external format", I think it's clumsy and incomplete. How about:
>> 
>> - print_method_signature_parameters()
>> - print_method_signature_return_type()
>> - print_field_signature_type()
>> 
>> I think the word "external" is confusing and can be skipped. For method signatures, we don't need methods that just print the parameters in their internal format (i.e., `ZI` or `L`), so there's no need to distinguish these functions from "internal" ones.
>> 
>> Also, the first two methods should assert `Signature::is_method(this)` and the last one should assert `!Signature::is_method(this)` .
>
> I also get somewhat obsessed with names :)
> 
> We are dealing with a VMSymbol which is really just a character string that could represent many things. One of those things is a method signature. So `as_signature` is telling us that we are interpreting/expecting the string to be a method signature. We then want either the parameters or return type of the method signature, and those could be in internal or external format. So IMO all the existing naming is perfectly good and desirable.
> 
> The issue with the new method is that fields don't have signatures** in the general sense that methods do. A field just has a name and a type. So to me the correct naming here would be `as_field_external_type`.
> 
> ** In the VM the classfile Signature attribute does exist for a FieldInfo but it is only used for fields declared with type variables or parameterized types. Within the VM all fields have their erased type.

`as_field_external_type` sounds good to me.

I still think the asserts for Signature::is_method should be added to these 3 functions.

-------------

PR: https://git.openjdk.org/jdk/pull/11745


More information about the hotspot-runtime-dev mailing list