[PATCH] D153821: [Symbolizer] Ignroe unknown additional symbolizer markup fields

[Daniel Thornburgh via Phabricator via llvm-commits]

mysterymath added inline comments.


================
Comment at: llvm/lib/DebugInfo/Symbolize/MarkupFilter.cpp:136
       filterNode(Node);
-    highlight();
-    OS << "[[[reset]]]" << lineEnding();
-    restoreColor();
+    printRawElement(Node);
+    OS << lineEnding();
----------------
mcgrathr wrote:
> I don't know of a reason to print anything at all when a reset element is encountered.
> 
This is mostly just to provide context for error messages; it would be highly surprising to see a "no mmap covers address" error not too long after a mmap that appears to cover the address, but where a reset was present in-between.


================
Comment at: llvm/test/DebugInfo/symbolize-filter-markup-reset.test:24
 
 {{{reset:}}}
+{{{module:0:a.o:elf:ab}}}
----------------
mcgrathr wrote:
> A trailing colon is an odd case, though I see no reason not to accept it. But it's never what an expected future extension would look like. Probably the test should cover `reset:this=1:that`, etc.
> 
Good point; the tool is agnostic to what the field looks like, and these tests are fairly white-box, so I've changed these to add an "ext" field to make the purpose of the case clearer.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D153821