[PATCH] D65191: [llvm-objdump] Implement highlighting

[Seiya Nuta via Phabricator via llvm-commits]

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


================
Comment at: llvm/tools/llvm-objdump/llvm-objdump.cpp:365
+
+static WithColor withHighlightColor(ObjdumpColor Color) {
+  auto Pair = HighlightColors.find(Color);
----------------
rupprecht wrote:
> IIUC, using `WithColor` to do nested coloring doesn't really work -- `~WithColor` resets the terminal color to plain, *not* to what the color was before. So if you have some stream of characters like:
> AAABBBCCCBBB
> where A maps to no coloring (X), B maps to red (R), and C maps to green (G), you should want to see:
> XXXRRRGGGRRR
> But I think it will actually be:
> XXXRRRGGGXXX
> because ~WithColor will reset to plain after closing the span of green.
> 
> In practice, I'm not sure any assembly actually has anything after nested spans -- the end of any nested span will line up with the span it's nested in. Still, it seems fragile/something that should be checked for somehow. Is this something you're handling somewhere that I'm just missing?
Good point. As you said, this patch doesn't support such a case. A good example is memory operand:

```
movq  <imm:$1>,  <mem:(<reg:%rax>, <reg:%rbx>)>
                      ^          ^                        
                Change color.    WithColor resets the color here.
```

I'll consider implementing a new class `WithHighlightColor`, which behaves like `WithColor`, but it saves/restores colors using a stack or something.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D65191