[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