[PATCH] D74819: [DebugInfo] Report unsupported maximum_operations_per_instruction values

[James Henderson via Phabricator via llvm-commits]

jhenderson marked 2 inline comments as done.
jhenderson added inline comments.


================
Comment at: llvm/lib/DebugInfo/DWARF/DWARFDebugLine.cpp:581
+                                                   uint64_t OpcodeOffset) {
+  StringRef OpcodeName = getOpcodeName(Opcode, LineTable->Prologue.OpcodeBase);
+  // For versions less than 4, the MaxOpsPerInst member is set to 0, as the
----------------
probinson wrote:
> Sink this under the `if` as you don't need the name unless you're reporting the warning.
It's used in the second error added in D75189, so I don't think sinking it inside the `if` makes much sense.


================
Comment at: llvm/lib/DebugInfo/DWARF/DWARFDebugLine.cpp:595
+        LineTable->Prologue.MaxOpsPerInst));
+  ReportAdvanceAddrProblem = false;
   uint64_t AddrOffset = OperationAdvance * LineTable->Prologue.MinInstLength;
----------------
probinson wrote:
> After sinking the getOpcodeName call, you have a block under the `if` instead of just a single statement, in which case you might as well move `ReportAdvanceAddrProblem = false` into that block as well.
> These moves help distinguish the error-reporting stuff from the functional stuff as well.
Same response as above. Given the additional error message in D75189, I can't put it inside the `if`.


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D74819/new/

https://reviews.llvm.org/D74819