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

Wed Mar 4 09:32:54 PST 2020

probinson added inline comments.

================
Comment at: llvm/include/llvm/DebugInfo/DWARF/DWARFDebugLine.h:374
+    uint64_t advanceAddr(uint64_t OperationAdvance, uint64_t OpcodeOffset,
+                         StringRef OpcodeName);

----------------
As a future API simplification, think about using `dwarf::LNStandardName()` to compute the names as needed, instead of passing around strings.  LNStandardName() would need an opcode_base parameter to know which ones are special for a particular line-number program, but it's used in only one place that I can find, so that should be an easy update.
I know there's the weird case of DW_LNS_const_add_pc where you're basing the advance on 255 but want to use the real opcode for reporting; but the weird cases are why we pay you the big bucks.

================
Comment at: llvm/unittests/DebugInfo/DWARF/DWARFDebugLineTest.cpp:9

+#include "llvm/DebugInfo/DWARF/DWARFDebugLine.h"
 #include "DwarfGenerator.h"
----------------
jhenderson wrote:
> probinson wrote:
> > Should not have moved this; it's not the header for this module.
> Weird. I don't know how that happened. I certainly wouldn't have deliberately moved it.
Did it move again?  If you ran clang-format, I wonder if there's a bug in it (it might think it's moving the "module's" header to the top, but have incorrect module-header detection).

Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D74819