[PATCH] D122840: [llvm-readobj][nfc] Run clang-format on the directory's .cpp and .h files

James Henderson via Phabricator via llvm-commits llvm-commits at lists.llvm.org
Thu Mar 31 22:31:58 PDT 2022


jhenderson added inline comments.


================
Comment at: llvm/tools/llvm-readobj/ARMEHABIPrinter.h:1
-//===--- ARMEHABIPrinter.h - ARM EHABI Unwind Information Printer ----------===//
+//===--- ARMEHABIPrinter.h - ARM EHABI Unwind Information Printer ---------===//
 //
----------------
Perhaps while you're at it put the C++ marker here.


================
Comment at: llvm/tools/llvm-readobj/COFFDumper.cpp:365
+    LLVM_READOBJ_ENUM_ENT(COFF, IMAGE_FILE_MACHINE_THUMB),
+    LLVM_READOBJ_ENUM_ENT(COFF, IMAGE_FILE_MACHINE_WCEMIPSV2)};
 
----------------
Not that I'm suggesting you change what clang-format has done here, but I find it odd that the closing brace is on the same line as an entry, but the opening one isn't. Perhaps worth asking a clang-format person about,


================
Comment at: llvm/tools/llvm-readobj/COFFDumper.cpp:1765-1772
-  case COFF::IMAGE_REL_BASED_ABSOLUTE: return "ABSOLUTE";
-  case COFF::IMAGE_REL_BASED_HIGH: return "HIGH";
-  case COFF::IMAGE_REL_BASED_LOW: return "LOW";
-  case COFF::IMAGE_REL_BASED_HIGHLOW: return "HIGHLOW";
-  case COFF::IMAGE_REL_BASED_HIGHADJ: return "HIGHADJ";
-  case COFF::IMAGE_REL_BASED_ARM_MOV32T: return "ARM_MOV32(T)";
-  case COFF::IMAGE_REL_BASED_DIR64: return "DIR64";
----------------
There's an active discussion on discourse about whether this style should be allowed. I'm not sure we should change it at this time, although preventing clang-format doing anything to it seems wrong too.

https://discourse.llvm.org/t/enable-single-line-case-statements-in-llvm-clang-format-style/61062

I've asked there for thoughts.


================
Comment at: llvm/tools/llvm-readobj/ELFDumper.cpp:83
 #define ENUM_ENT(enum, altName)                                                \
-  { #enum, altName, ELF::enum }
+  {                                                                            \
+#enum, altName, ELF::enum                                                  \
----------------
Looks like clang-format doesn't agree with itself?


================
Comment at: llvm/tools/llvm-readobj/ELFDumper.cpp:988-990
-  {"None",   "none",   ELF::ELFCLASSNONE},
-  {"32-bit", "ELF32",  ELF::ELFCLASS32},
-  {"64-bit", "ELF64",  ELF::ELFCLASS64},
----------------
Here and throughout these tables, I think the existing style is more readable, so possibly decorate them (though do remove the extra space in this column for example).

If this style isn't available as a clang-format option, please make a feature request. If it is, I'd start a Discourse thread on enabling it, citing this code as an example.


================
Comment at: llvm/tools/llvm-readobj/ELFDumper.cpp:2088
 #define LLVM_READOBJ_DT_FLAG_ENT(prefix, enum)                                 \
-  { #enum, prefix##_##enum }
+  {                                                                            \
+#enum, prefix##_##enum                                                     \
----------------
Another linter complaint.


================
Comment at: llvm/tools/llvm-readobj/WasmDumper.cpp:25
 #define ENUM_ENTRY(X)                                                          \
-  { #X, wasm::WASM_SYMBOL_TYPE_##X }
+  {                                                                            \
+#X, wasm::WASM_SYMBOL_TYPE_##X                                             \
----------------
And more here.


================
Comment at: llvm/tools/llvm-readobj/XCOFFDumper.cpp:141
 #define ECase(X)                                                               \
-  { #X, XCOFF::X }
+  {                                                                            \
+#X, XCOFF::X                                                               \
----------------
And more (multiple times in this file).


================
Comment at: llvm/tools/llvm-readobj/llvm-readobj.h:47
+#define LLVM_READOBJ_ENUM_ENT(ns, enum)                                        \
+  {                                                                            \
+#enum, ns::enum                                                            \
----------------
Ditto.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D122840



More information about the llvm-commits mailing list