[PATCH] D130187: [Symbolizer] Implement data symbolizer markup element.

Daniel Thornburgh via Phabricator via llvm-commits llvm-commits at lists.llvm.org
Wed Aug 3 10:52:23 PDT 2022


mysterymath added inline comments.


================
Comment at: llvm/test/DebugInfo/symbolize-filter-markup-data.test:12
+
+CHECK: [[BEGIN:\[{3}]]ELF module #0x0 "a.o"; BuildID=abcdef 0x0(r)-0x10(r)[[END:\]{3}]]
+CHECK: long long byte
----------------
peter.smith wrote:
> I don't know how much scope there is to change the output format of the mmap information in the module line given that this is based on a pre-existing markup. When I saw `0x0(r) - 0x10(r)` I wasn't sure whether this was one contiguous range `[0x0, 0x10)` or two separate ranges, not necessarily contiguous with one starting at `0x0` and one at `0x10`. I guessed at the latter given the `{{{mmap}}}` elements but it would have been harder to understand if I'd just seen the output and not the input.
> 
> Perhaps use a comma instead of a dash to separate the mmaps? As an alternative I guess the size could be incorporated in some way.
> 
> No worries if this isn't possible.
> 
>  
Generally agree; there shouldn't be anything that hard-depends on the format, as this is intended to be human readable, not machine readable. I think we can mostly escape Hyrum's law too, since usage of this markup format is pretty limited at present.

Spent a bit of time thinking about this, and ended up with [0x0-0x9](r),[0x10-0x11](r). Using brackets agrees with the mathematical notation for inclusive ranges. Using inclusive ranges and dashes inside makes this also readable to those unfamiliar with that notation.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D130187



More information about the llvm-commits mailing list