[PATCH] D129519: [Symbolizer] Implement contextual symbolizer markup elements.

[Peter Smith via Phabricator via llvm-commits]

peter.smith accepted this revision.
peter.smith added a comment.
This revision is now accepted and ready to land.

Thanks for the updates. I've no more substantial comments. Will be worth waiting a bit to see if anyone else on the list have anything more to add.



================
Comment at: llvm/lib/DebugInfo/Symbolize/MarkupFilter.cpp:137
+
+  // Only mmap elements can extend a previous module info line, and this node
+  // cannot be an mmap element.
----------------
May be worth making it more explicit that `mmap` needs to be processed before the other tags due to the presence of `endModuleInfoLine` here. Without looking carefully it could seem like new tags can be added anywhere. Some possibilities:
- Comment in header or by mmap.
- if (Element.Tag == "mmap") { ... } else { endModuleInfoLine(); ... }
- if (Element.Tag != "mmap") { endModuleInfoLine(); } else if (ElementTag == "mmap") { ... } if (Element.Tag == "reset") { ... }


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D129519