[lworld] [Rev 01] RFR: rename verifier related value-types, rename JVM_SIGNATURE_VALUETYPE, …

Frederic Parain fparain at openjdk.java.net
Tue Jun 16 13:25:09 UTC 2020


On Tue, 16 Jun 2020 12:55:14 GMT, Harold Seigel <hseigel at openjdk.org> wrote:

>> This pull requiest renames JVM_SIGNATURE_VALUETYPE to JVM_SIGNATURE_INLINETYPE, renames verifier related value_type
>> items to inline_type, and has a few other small changes.
>
> Harold Seigel has updated the pull request incrementally with one additional commit since the last revision:
> 
>   change JVM_SIGNATURE_VALUETYPE to JVM_SIGNATURE_INLINE_TYPE

Harold,

Overall changes look good.
I've commented a few places where naming could be made more consistent (putting an underscore between "inline" and
"type"). They are just suggestions, I'll let you decide to change names or not.

Thank you,

Fred

src/hotspot/share/classfile/stackMapFrame.cpp line 115:

> 114:       if (ss.type() == T_VALUETYPE) {
> 115:         return VerificationType::inlinetype_type(sig);
> 116:       }

VerificationType::inlinetype_type looks weird.
Could we use VerificationType::inline_type instead, which is consistent with VerificationType::reference_type?

src/hotspot/share/classfile/stackMapTable.cpp line 197:

> 196:       }
> 197:       return VerificationType::inlinetype_type(fund_name);
> 198:     } else {

VerificationType::inlinetype_type -> VerificationType::inline_type ?

src/hotspot/share/classfile/verificationType.cpp line 163:

> 162:
> 163: bool VerificationType::is_inlinetype_assignable_from(const VerificationType& from) const {
> 164:   // Check that 'from' is not null, is an inline type, and is the same inline type.

is_inlinetype_assignable_from() -> is_inline_type_assignable_from()
to be more consistent with other names?

src/hotspot/share/classfile/verificationType.cpp line 165:

> 164:   // Check that 'from' is not null, is an inline type, and is the same inline type.
> 165:   assert(is_inlinetype(), "called with a non-inlinetype type");
> 166:   assert(!is_null(), "inlinetype is not null");

is_inlinetype() -> is_inline_type() ?

src/hotspot/share/classfile/verificationType.hpp line 156:

> 155:     { return VerificationType(ReferenceQuery); }
> 156:   static VerificationType inlinetype_check()
> 157:     { return VerificationType(InlineTypeQuery); }

inlinetype_check() -> inline_type_check() ?

src/hotspot/share/classfile/verificationType.hpp line 182:

> 181:   // Provides a way for an inline type to be distinguished from a reference type.
> 182:   static VerificationType inlinetype_type(Symbol* sh) {
> 183:       assert(((uintptr_t)sh & TypeMask) == 0, "Symbols must be aligned");

inlinetype_type() -> inline_type() ?

src/hotspot/share/classfile/verificationType.hpp line 225:

> 224:   bool is_reference_check() const { return _u._data == ReferenceQuery; }
> 225:   bool is_inlinetype_check() const { return _u._data == InlineTypeQuery; }
> 226:   bool is_nonscalar_check() const { return _u._data == NonScalarQuery; }

is_inlinetype_check() -> is_inline_type_check() ?

src/hotspot/share/classfile/verificationType.hpp line 245:

> 244:   bool is_array_array() const { return is_x_array(JVM_SIGNATURE_ARRAY); }
> 245:   bool is_inline_array() const { return is_x_array(JVM_SIGNATURE_INLINE_TYPE); }
> 246:   bool is_reference_array() const

The term "inline" alone can be misleading (is it about the element being an inline type, is it about the elements of
the array being inlined in the array, is it about the array being inlined in something else, etc). Here, the check is
about the kind of elements stored in the array, so I would suggest "is_inline_type_array()".

src/hotspot/share/classfile/verificationType.hpp line 266:

> 265:
> 266:   static VerificationType change_ref_to_inlinetype(VerificationType ref) {
> 267:     assert(ref.is_reference(), "Bad arg");

change_ref_to_inlinetype() -> change_ref_to_inline_type() ?

src/hotspot/share/classfile/verifier.cpp line 591:

> 590:
> 591: VerificationType reference_or_inlinetype(InstanceKlass* klass) {
> 592:   if (klass->is_value()) {

reference_or_inlinetype() -> reference_or_inline_type() ?

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

Marked as reviewed by fparain (Committer).

PR: https://git.openjdk.java.net/valhalla/pull/83


More information about the valhalla-dev mailing list