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