On Tue, 7 Jan 2025 20:59:18 GMT, Dean Long <dl...@openjdk.org> wrote:

>> Coleen Phillimore has updated the pull request incrementally with one 
>> additional commit since the last revision:
>> 
>>   Make more AccessFlags fetches more specific and remove an assert and 
>> remove this->s.
>
> src/hotspot/share/classfile/vmIntrinsics.cpp line 39:
> 
>> 37: 
>> 38: // These are flag-matching functions:
>> 39: inline bool match_F_R(u2 flags) {
> 
> I wish more code could be size-agnostic.  So instead of using `u2` here, 
> there could be a typedef in accessFlags.hpp that we could use that hides the 
> size.  However, it's not a big deal, because it seems unlikely this type will 
> change much in the future without a JVM spec change.  I'm tempted to 
> suggesting using AccessFlags here, but it's a class.  Since this is an 
> end-point "consumer" of the type that doesn't store it or pass it along, we 
> could even use something like `uint` here.

That seems like a good idea if this weren't so limited.  I did chat with DanH. 
to see if access flags will ever grow in size from u2 and he was doubtful that 
would ever happen. uint would work here too, but this small use might prevent 
some future -Wconversion warnings and doesn't hurt anything.
Thank you for reviewing.

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

PR Review Comment: https://git.openjdk.org/jdk/pull/22246#discussion_r1906090983

Reply via email to