DavidSpickett added inline comments.
================
Comment at: lldb/source/Plugins/Process/Utility/ARMUtils.h:29
+ // assert(0 && "Invalid shift type");
+ break;
case 0:
----------------
Looking at the codepaths and the reason this function exists, it's either
called with a 2 bit number which is the "type" field of an instruction, or it's
not called because the type is known to be SRType_RRX already.
In the enum ARM_ShifterType SRType_RRX would be 4, which wouldn't fit the 2 bit
expectation.
So I think we can safely re-enable this assert and move the two unreachable
lines up here. I didn't get any test failures doing so and I don't see a
codepath that could hit it.
(but it would be fairly easy for future changes to make this mistake since it's
not very obvious from the types, so an assert is good here)
================
Comment at: lldb/source/Plugins/Process/Utility/ARMUtils.h:314-315
switch (bits(imm12, 9, 8)) {
default: // Keep static analyzer happy with a default case
+ break;
+
----------------
fixathon wrote:
> Same here. Past code history suggests the break was missing by accident, i.e
> the **default** case was added to satisfy a static code checker without the
> intent to modify the execution flow.
This part LGTM. This "bits" function is returning uint32 so the compiler can't
know it'll only be in a 2 bit range.
Repository:
rG LLVM Github Monorepo
CHANGES SINCE LAST ACTION
https://reviews.llvm.org/D131244/new/
https://reviews.llvm.org/D131244
_______________________________________________
lldb-commits mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
