[PATCH] D73045: [obj2yaml] - Better indentations in the ELF output.

George Rimar via Phabricator via llvm-commits llvm-commits at lists.llvm.org
Mon Jan 20 07:22:09 PST 2020


grimar created this revision.
grimar added reviewers: jhenderson, MaskRay.
Herald added a subscriber: hiraditya.
Herald added a project: LLVM.
grimar added a parent revision: D73046: [YAML] - Move yaml::Input and yaml::Ouput classes declarations..
grimar added a comment.
grimar marked an inline comment as done.

It is rebased on top of D73046 <https://reviews.llvm.org/D73046>.



================
Comment at: llvm/test/tools/obj2yaml/elf-output-indentation.yaml:19
+# CHECK-NEXT:    Type:         SHT_PROGBITS
+# CHECK-NEXT:    AddressAlign: 0x0000000000000001
+# CHECK-NEXT:  - Name:        .rela.text
----------------
Here the additional indentation appears because of `AddressAlign`, which is
the largest key among others. The same situation is shown below.
I did not investigate how much hard to to improve such places too yet.


The current obj2yaml's output has an issue: it places
a lot of an excessive indentation spaces between keys and values.
Then we have to remove them manually to improve the readability of
test cases. I see such requests from reviewers regularly around
and it is not ideal.

The issue happens because `Output::paddedKey` has a trivial logic which
hardcodes the maximum of spaces used to align the values column:

  const char *spaces = "                ";
  if (key.size() < strlen(spaces))
    Padding = &spaces[key.size()];

I've experimented with a ways to improve this situation and
seems finally found the solution, though it is not that simple
as I've hoped when started.

The main problem is that until we performed a mapping of
fields, we know nothing about their names and hence
can't use this information to tweak the indentations on the fly.
At the same time we write the output during doing the mapping,
so it is too late to do something on this step.

So. In this patch I've introduced a "pre-mapping" pass. This is a
cheap "mapping" that is done right before the real mapping.
This pass only scans the keys which are on the current level of the output
and finds the maximum key length. This information is then used
to calculate and print a better indentation during the real mapping pass.

Given that the approach is not that simple, I'd like to discuss before
doing additional polishing. Perhaps the main question is: do we want
such complexity to improve indentations?


https://reviews.llvm.org/D73045

Files:
  llvm/include/llvm/Support/YAMLTraits.h
  llvm/lib/ObjectYAML/ELFYAML.cpp
  llvm/lib/Support/YAMLTraits.cpp
  llvm/test/tools/obj2yaml/elf-output-indentation.yaml
  llvm/tools/obj2yaml/elf2yaml.cpp

-------------- next part --------------
A non-text attachment was scrubbed...
Name: D73045.239123.patch
Type: text/x-patch
Size: 8714 bytes
Desc: not available
URL: <http://lists.llvm.org/pipermail/llvm-commits/attachments/20200120/32533969/attachment.bin>


More information about the llvm-commits mailing list