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

[Peter Smith via Phabricator via llvm-commits]

peter.smith added a comment.

Just a couple of typos spotted in a comment.

One thing I find hard to reason about is the DeferredNodes. We add deferred nodes in `tryContextualLine`, but never in `tryContextualElement`, from that point onwards we can `flush` or `clear` only. As the latter two are read-write we can't use `const` to let readers know from the type signature what is and isn't expected. One possible way around this is to make deferred nodes its own type that implements more than one interface. An interface that permits adding, and one that does not (only supports `flush` and `clear`). The latter interface can be passed to tryContextualElement. Possibly overboard for this patch when there are only a few ContextualElements, but could be worth it if it gets more complicated.



================
Comment at: llvm/include/llvm/DebugInfo/Symbolize/MarkupFilter.h:120
 
+  // Nodes at the beginning of a possibly-conextual line tha have not yet been
+  // emitted. These may be elided if the line ends up being contextual.
----------------
typos: 
conextual -> contextual
tha -> that


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D129519