fixathon marked an inline comment as done.
fixathon added inline comments.

================
Comment at: lldb/source/Plugins/Process/Utility/ARMUtils.h:49
   shift_t = SRType_Invalid;
   return UINT32_MAX;
 }
----------------
DavidSpickett wrote:
> This is now dead code?
This will execute for the default case after its break.


================
Comment at: lldb/source/Plugins/Process/Utility/ARMUtils.h:29
+    // assert(0 && "Invalid shift type");
+    break;
   case 0:
----------------
DavidSpickett wrote:
> 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)
Added the assert back


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

Reply via email to