[lworld] RFR: 8374016: [lworld] Too late checked_cast in ResolvedFieldEntry::set_flags

[Stefan Karlsson]

In the lworld branch the `new_flags` type has been changed to u1 instead of int. We now perform a bunch of bit operations, which results in an int, and then we narrow that down into a u1. After the u1 has been created we check that this doesn't overflow a u1 with the checked_cast<u1> check. That's too late.

Mainline code looks like this:

int new_flags = ...
_flags = checked_cast<u1>(new_flags);


and Valhalla code looks like this:

u1 new_flags = ...
_flags = checked_cast<u1>(new_flags);


This voids the benefit of having a checked_cast here. I propose that we revert back to using `int new_flags` and keep the checked_cast<u1>.

While reading this code there were a few things that made the code harder to follow and I've tweaked it.

1) The `bool` `is_volatile_shift` is special-treated and instead of shifting it is directly casted to int. That hard codes a knowledge/assumption that the flag is always going to be 1 (and maybe also what value bools have).

That made me have to look at the actual values of these and at one point I thought that is_volatile_shift was `1` and not `0` and that the above was a bug. The main reason I thought that was this odd order here:

   u1 new_flags = ((is_final_flag ? 1 : 0) << is_final_shift) | static_cast<int>(is_volatile_flag) |
      ((is_flat_flag ? 1 : 0) << is_flat_shift) |
      ((is_null_free_inline_type_flag ? 1 : 0) << is_null_free_inline_type_shift) |
      ((has_null_marker_flag ? 1 : 0) << has_null_marker_shift);

Here the order of usage is

is_final_flag
is_volatile_flag
is_flat_flag
is_null_free_inline_type_flag
has_null_marker_flag


but the enum order has final and volatile swapped:

is_volatile_shift
is_final_shift
is_flat_shift
is_null_free_inline_type_shift
has_null_marker_shift


I'd prefer if we stayed consistent with the order, to lower the risk that some bugs sneak in here. So, I've replaced the cast with a shift (just like the other values are handled) and I  ordered functions, parameters, and operations in the same order as the enums.

2) Tweaked some alignments to make these kind of issues clearer.

Please tell me if I went to far with the style changes.

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

Commit messages:
 - Merge remote-tracking branch 'valhalla/lworld' into lworld_8374016_ResolvedFieldEntry
 - 8374016: [lworld] Too late checked_cast in ResolvedFieldEntry::set_flags
 - 8374007: [lworld] Remove dead code in Symbol

Changes: https://git.openjdk.org/valhalla/pull/1844/files
  Webrev: https://webrevs.openjdk.org/?repo=valhalla&pr=1844&range=00
  Issue: https://bugs.openjdk.org/browse/JDK-8374016
  Stats: 28 lines in 2 files changed: 8 ins; 2 del; 18 mod
  Patch: https://git.openjdk.org/valhalla/pull/1844.diff
  Fetch: git fetch https://git.openjdk.org/valhalla.git pull/1844/head:pull/1844

PR: https://git.openjdk.org/valhalla/pull/1844