[PATCH] D65189: [MC] Support returning a structured rich disassembly output

Jordan Rupprecht via Phabricator via llvm-commits llvm-commits at lists.llvm.org
Wed Aug 7 10:05:27 PDT 2019


rupprecht accepted this revision.
rupprecht added inline comments.


================
Comment at: llvm/include/llvm/MC/MCInstPrinter.h:72
+
+struct MarkupStart {
+  bool Enabled;
----------------
Can you add brief class-level comments for MarkupStart/MarkupEnd too?


================
Comment at: llvm/lib/MC/MCInstPrinter.cpp:159
+    if (!M.Spans.empty()) {
+      assert(M.Spans.size() > 1 && "Missing the corresponding markupStart().");
+      M.Spans.pop();
----------------
Another design consideration would be to put the functionality of `endMarkup()` into the destructor `MarkupStart::~MarkupStart()`

e.g.
```
OS << startMarkup(MarkupType::Reg) << '%' << getRegisterName(RegNo)
     << endMarkup();
```
turns into:
```
OS << startMarkup(MarkupType::Reg) << '%' << getRegisterName(RegNo); /* ~MarkupStart() ends the markup */
```

Though it would require changing all the call sites so the register/operand/whatever can be printed as part of the same `OS << ...` expression, so I wouldn't recommend trying that until later. But it may be worth looking into to avoid programmer error of forgetting either of markupStart()/markupEnd()


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D65189





More information about the llvm-commits mailing list