RFR: 8320396: Class-File API ClassModel::verify should include checks from hotspot/share/classfile/classFileParser.cpp [v5]

[Adam Sotona]

On Tue, 14 May 2024 08:30:56 GMT, Aggelos Biboudis <abimpoudis at openjdk.org> wrote:

>> Adam Sotona has updated the pull request incrementally with two additional commits since the last revision:
>> 
>>  - fixed error thrown by VerifierImpl
>>  - applied suggested changes
>
> src/java.base/share/classes/jdk/internal/classfile/impl/verifier/ParserVerifier.java line 223:
> 
>> 221:                     errors.add(new VerifyError("Arguments can't fit into locals in %s".formatted(toString(ae))));
>> 222:                 }
>> 223:                 int l = 12 + ca.codeLength() + 8 * ca.exceptionHandlers().size();
> 
> nit (or not): it would be amazing if all these constants have a name. They are almost intuitive, I admit, but it would improve readability a lot.

I think about the readability exactly opposite. When deciding between:

static final int CODE_ATTRIBUTE_HEADER_SIZE = 12;
static final int CODE_ATTRIBUTE_EXCEPTION_HANDLER_ENTRY_SIZE = 8;

...

int l = CODE_ATTRIBUTE_HEADER_SIZE + ca.codeLength() + CODE_ATTRIBUTE_EXCEPTION_HANDLER_ENTRY_SIZE * ca.exceptionHandlers().size();

or

int l = 12 + ca.codeLength() + 8 * ca.exceptionHandlers().size();

For this specific case ( implementation transcription of `classFileParser.cpp`) I prefer the shorter.

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

PR Review Comment: https://git.openjdk.org/jdk/pull/16809#discussion_r1599666623