On Thu, 3 Aug 2023 09:05:12 GMT, Dean Long <dl...@openjdk.org> wrote:

>> The recent change in 
>> [JDK-8301996](https://bugs.openjdk.org/browse/JDK-8301996) added more 
>> -Wconversion warnings that are addressed in this patch. The aforementioned 
>> change has overlooked inconsistencies with the types used by 
>> `ResolvedFieldEntry` and the method `fill_in()`. Verified with tier 1-4 
>> tests.
>
> src/hotspot/share/interpreter/rewriter.cpp line 192:
> 
>> 190:     int field_entry_index = Bytes::get_native_u2(p);
>> 191:     int pool_index = 
>> _initialized_field_entries.at(field_entry_index).constant_pool_index();
>> 192:     Bytes::put_Java_u2(p, (u2)pool_index);
> 
> I guess this isn't using checked_cast because similar code elsewhere in this 
> file uses this style, but it means we can silently do the wrong thing.  Is 
> there a separate RFE to remove as many of these unchecked casts as possible?

We could rewrite _initialized_field_entries to be an array of u2 and not need 
the cast, but that changed felt out of scope for this fix.

> src/hotspot/share/oops/resolvedFieldEntry.hpp line 106:
> 
>> 104:   void set_flags(bool is_final, bool is_volatile) {
>> 105:     int new_flags = (is_final << is_final_shift) | 
>> static_cast<int>(is_volatile);
>> 106:     _flags = (u1)new_flags;
> 
> Why isn't this using checked_cast?

I still don't see the point of using checked_cast if we know that this is only 
two bits.

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

PR Review Comment: https://git.openjdk.org/jdk/pull/15126#discussion_r1283090793
PR Review Comment: https://git.openjdk.org/jdk/pull/15126#discussion_r1283084054

Reply via email to