Cleanups before splitting java_bytecode_parsert #5105
Conversation
Moved pool_entryt typedef earlier to use it in two more places Use existing functions is_java_array_tag and java_array_element_type Changed control flow to remove redundant check
NathanJPhillips
requested review from
cesaro,
cristina-david,
kroening,
peterschrammel,
pkesseli,
smowton,
tautschnig and
thk123
as code owners
| result <<= 8; | ||
| result |= in->get(); | ||
| result <<= 8u; | ||
| result |= static_cast<u1>(in->get()); |
There was a problem hiding this comment.
I think we try to use narrow_cast in this situation
There was a problem hiding this comment.
I'm just making it unsigned.
| #define ACC_STRICT 0x0800u | ||
| #define ACC_SYNTHETIC 0x1000u | ||
| #define ACC_ANNOTATION 0x2000u | ||
| #define ACC_ENUM 0x4000u |
There was a problem hiding this comment.
I think this block was better before
There was a problem hiding this comment.
I agree. I ran it through clang-format because I added the u unsigned postfix. Should I undo that? 🔔
| #define ACC_ABSTRACT 0x0400u | ||
| #define ACC_SYNTHETIC 0x1000u | ||
| #define ACC_ANNOTATION 0x2000u | ||
| #define ACC_ENUM 0x4000u |
There was a problem hiding this comment.
I think this block looked better before
| @@ -1565,8 +1565,13 @@ class reference_typet:public pointer_typet | |||
| template <> | |||
| inline bool can_cast_type<reference_typet>(const typet &type) | |||
| { | |||
| return can_cast_type<pointer_typet>(type) && type.get_bool(ID_C_reference) && | |||
| !type.get(ID_width).empty(); | |||
There was a problem hiding this comment.
Why do you think it's better to move this check to validation? You could add a reason to the commit message.
A reference type without a width is still a reference type, it's just an invalid one. Rather than the cast failing, a DATA_INVARIANT should be put on this property.
Mostly relate to signed/unsigned mismatch and passing as const reference
NathanJPhillips
force-pushed
the
cleanup/pre-split-parser
branch
from
495b2e1 to
7a9e0ce
Compare
|
@NathanJPhillips You didn't address @smowton 's comment that you should use clang-format off to avoid reformatting those two big blocks. Was that an oversight or did you choose not to take the suggestion? |
There was a problem hiding this comment.
✔️
Passed Diffblue compatibility checks (cbmc commit: 7a9e0ce).
Build URL: https://travis-ci.com/diffblue/test-gen/builds/127366064
Non-behavioural changes in preparation for major refactoring of
java_bytecode_parsertinto separate load and parse stages.