[PATCH] D44558: Change DT_* value definitions to macros in a separate file

[Alexander Richardson via Phabricator via llvm-commits]

arichardson added inline comments.


================
Comment at: include/llvm/BinaryFormat/DynamicTags.def:1
+
+#ifndef DYNAMIC_TAG
----------------
grimar wrote:
> Excessive empty line.
I did this for consistency with ElfRelocs/X86_64.def which also adds a newline here. Happy to remove it though since I also think it looks a bit strange.

It seems all the files in that folder except AMDGPU.def and BPF.def have that newline.


================
Comment at: tools/llvm-readobj/ELFDumper.cpp:1548
     switch (Type) {
-    LLVM_READOBJ_TYPE_CASE(HEXAGON_SYMSZ);
-    LLVM_READOBJ_TYPE_CASE(HEXAGON_VER);
-    LLVM_READOBJ_TYPE_CASE(HEXAGON_PLT);
+#define HEXAGON_DYNAMIC_TAG(name, value)                                       \
+  case DT_##name:                                                              \
----------------
grimar wrote:
> Personally  to me one-line short form here and below looks easier to read:
> 
> ```
> #define HEXAGON_DYNAMIC_TAG(name, value) case DT_##name: return #name;
> ```
This is what git-clang-format did to the file but I agree the short version is easier to understand. Fixed.


Repository:
  rL LLVM

https://reviews.llvm.org/D44558