On Mon, 17 Jun 2024 15:56:56 GMT, Adam Sotona <asot...@openjdk.org> wrote:
>> Chen Liang has updated the pull request incrementally with two additional
>> commits since the last revision:
>>
>> - Improve tests to check unmatched bit position and failure for
>> non-inner-classes
>> - Report error for flag problems
>
> src/jdk.jdeps/share/classes/com/sun/tools/javap/BasicWriter.java line 84:
>
>> 82: } catch (IllegalArgumentException ex) {
>> 83: mask &= LOCATION_MASKS.get(location);
>> 84: report(ex);
>
> Unfortunately the original exception message is missing any info that it is
> related to access flags and it is not very clear what "Error: Unmatched bit
> position 0x2 for location CLASS" means.
> I would add at least a message prefix, for example "Error: Access Flags:
> Unmatched bit position 0x2 for location CLASS".
Agreed, even though this message is already printed right by the access
modifiers.
> test/langtools/tools/javap/UndefinedAccessFlagTest.java line 105:
>
>> 103: .writeAll()
>> 104: .getOutputLines(Task.OutputKind.DIRECT);
>> 105: assertTrue(lines.stream().anyMatch(l -> l.contains("Unmatched
>> bit position")), () -> String.join("\n", lines));
>
> I would add a check the "fatal" error does not occur or any other way to
> confirm correct recovery.
Great suggestion! I will filter the lines starting with `Error:` and assert
they `allMatch(st -> st.contains("Access Flags:"))`
-------------
PR Review Comment: https://git.openjdk.org/jdk/pull/19708#discussion_r1643072918
PR Review Comment: https://git.openjdk.org/jdk/pull/19708#discussion_r1643073763
