RFR: 8313435: Clean up unused default methods code [v2]

David Holmes dholmes at openjdk.org
Wed Aug 2 07:39:55 UTC 2023


On Tue, 1 Aug 2023 15:43:48 GMT, Coleen Phillimore <coleenp at openjdk.org> wrote:

>> Default methods processing code has unused code (that gets -Wconversion warnings) from when it was used to create bridge (called overpass) method for an early implementation of generic reification in Hotspot.
>> 
>> This change removes unused bytecodeAssembler code and adds a check for methods and constant pool overflow.
>> Tested with tier1 and runtime/lambda-features tests.
>
> Coleen Phillimore has updated the pull request incrementally with one additional commit since the last revision:
> 
>   Align parameters.

Generally looks good but a couple of nits/queries.

Thanks

src/hotspot/share/classfile/bytecodeAssembler.cpp line 55:

> 53: 
> 54:   int new_size = _orig->length() + _entries.length();
> 55:   if (new_size > std::numeric_limits<u2>::max()) {

Should there be an include header for this in this file?

src/hotspot/share/classfile/defaultMethods.cpp line 1060:

> 1058: 
> 1059:   int new_methods_length = klass->methods()->length() + new_methods->length();
> 1060:   if (new_methods_length > std::numeric_limits<u2>::max()) {

Again do we need a header file include for this?

src/hotspot/share/classfile/defaultMethods.cpp line 1061:

> 1059:   int new_methods_length = klass->methods()->length() + new_methods->length();
> 1060:   if (new_methods_length > std::numeric_limits<u2>::max()) {
> 1061:     THROW_MSG(vmSymbols::java_lang_InternalError(), "default methods error methods overflow");

That message doesn't quite read right to me - not clear exactly what has overflowed here.

src/hotspot/share/classfile/defaultMethods.cpp line 1081:

> 1079:   int new_idx = 0;
> 1080: 
> 1081:   for (u2 i = 0; i < new_size; ++i) {

Is this change for the loop index necessary? Even as an int it is constrained to be <= max(u2) - and `at_put` expects an int index doesn't it?

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

PR Review: https://git.openjdk.org/jdk/pull/15095#pullrequestreview-1558373999
PR Review Comment: https://git.openjdk.org/jdk/pull/15095#discussion_r1281506048
PR Review Comment: https://git.openjdk.org/jdk/pull/15095#discussion_r1281508760
PR Review Comment: https://git.openjdk.org/jdk/pull/15095#discussion_r1281510854
PR Review Comment: https://git.openjdk.org/jdk/pull/15095#discussion_r1281513914


More information about the hotspot-runtime-dev mailing list