[PATCH] D65190: [X86] X86ATTInstPrinter: replace markup with startMarkup/endMarkup

[Seiya Nuta via Phabricator via llvm-commits]

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


================
Comment at: llvm/lib/Target/X86/MCTargetDesc/X86ATTInstPrinter.cpp:379-381
+    O << startMarkup(MarkupType::Imm) << '$';
     Op.getExpr()->print(O, &MAI);
+    O << endMarkup();
----------------
jhenderson wrote:
> seiya wrote:
> > jhenderson wrote:
> > > seiya wrote:
> > > > jhenderson wrote:
> > > > > No need for this to be part of this change, but I find it slightly odd that you can't use the withMarkup pattern here. I take it that making the print function return a stream wouldn't be straightforward?
> > > > > I take it that making the print function return a stream wouldn't be straightforward?
> > > > Does "return a stream" mean `raw_ostream &MCExpr::print(raw_ostream &OS, ...);` instead of `void MCExpr::print(...)`?
> > > Yes, that's what I meant.
> > How can I utilize that returned stream?  `Op.getExpr()->print(O, &MAI) << withMarkup(MarkupType::Imm) <<'$';` came to my mind but it doesn't work because the `Expr::print` is evaluated before prepending the starting tag (`<imm:`) by withMarkup.
> > 
> I might be misunderstanding something, but couldn't you do the following?
> 
> `withMarkup(O, MarkupType::Imm) << '$' << Op.getExpr()->print(O, &MAI)`
> 
> 
No we can't. `Op.getExpr()->print()` prints the disassembly into `O` by itself and returns void. Making it return something streamable won't be straightforward to implement.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D65190