[PATCH] D71591: [llvm-readob] - Refactor printing of sections flags. NFCI.

James Henderson via Phabricator via llvm-commits llvm-commits at lists.llvm.org
Tue Dec 17 02:32:25 PST 2019


jhenderson added inline comments.


================
Comment at: llvm/tools/llvm-readobj/ELFDumper.cpp:1455
 static const EnumEntry<unsigned> ElfARMSectionFlags[] = {
-  LLVM_READOBJ_ENUM_ENT(ELF, SHF_ARM_PURECODE)
+  ENUM_ENT(SHF_ARM_PURECODE, "y")
 };
----------------
Honestly, it seems a little weird to have this inconsistency in the different macros. As far as I can see, the only difference is to provide an AltName field, right? Would it be a good idea to change all the other flags to use `ENUM_ENT`, perhaps with an empty AltName?

This would also help ensure that people consider what, if any, GNU flag letter a new flag should have, and would also allow us to avoid the situation where some flags for a target might have a GNU representation, but not others.


================
Comment at: llvm/tools/llvm-readobj/ELFDumper.cpp:1527-1529
+      // An alternative name can only differ from a regular name when
+      // it is known to be used for the GNU style output.
+      return E.Value == Flag && E.AltName != E.Name;
----------------
With my suggestion above, this would then become a check against the empty string, which feels less fragile to me.


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

https://reviews.llvm.org/D71591





More information about the llvm-commits mailing list