[PATCH] D65240: [XCOFF][AIX] Generate symbol table entries with llvm-readobj

[Hubert Tong via Phabricator via llvm-commits]

hubert.reinterpretcast added inline comments.


================
Comment at: llvm/lib/Object/XCOFFObjectFile.cpp:22
+  FUNCTION_SYM = 0x20,
+  RELOC_OVERFLOW = 65535,
+  SYM_TAB_ENTRY_SIZE = 18,
----------------
This is unused, and this should be something like `RELOC_LNNO_OVERFLOW` if we were to have it in this patch.


================
Comment at: llvm/tools/llvm-readobj/XCOFFDumper.cpp:131
+  W.printString("Name", FileName);
+  W.printEnum("Type", (uint8_t)AuxEntPtr->Type, makeArrayRef(FileStringType));
+}
----------------
Use `static_cast`.


================
Comment at: llvm/tools/llvm-readobj/XCOFFDumper.cpp:172
+  W.printEnum("SymbolType",
+              ((uint8_t)AuxEntPtr->SymbolAlignmentAndType & SymbolTypeMask),
+              makeArrayRef(CSectSymbolTypeClass));
----------------
If the previous line did not need the cast to do the bitwise-and, then neither does this one. Also, remove the redundant parentheses.


================
Comment at: llvm/tools/llvm-readobj/XCOFFDumper.cpp:174
+              makeArrayRef(CSectSymbolTypeClass));
+  W.printEnum("StorageMappingClass", (uint8_t)AuxEntPtr->StorageMappingClass,
+              makeArrayRef(CSectStorageMappingClass));
----------------
Use `static_cast`.


================
Comment at: llvm/tools/llvm-readobj/XCOFFDumper.cpp:186
+  W.printNumber("SectionLength", AuxEntPtr->SectionLength);
+  W.printNumber("NumberOfRelocEnt", AuxEntPtr->NumberOfRelocEnt);
+  W.printNumber("NumberOfLineNum", AuxEntPtr->NumberOfLineNum);
----------------
Overflow handling is not here, and there is no TODO.


================
Comment at: llvm/tools/llvm-readobj/XCOFFDumper.cpp:232
+  case XCOFF::C_DWARF:
+    llvm_unreachable("This StorageClass for the symbol is not yet implemented");
+  default:
----------------
Add a period to the end of the sentence.
Replace `llvm_unreachable(`//x//`)` with `assert(false && `//x//`)` here. I guess the assert makes sense to have (in place of just a TODO), so that developers can clue in when they are running this utility on an object with the unimplemented kinds.


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

https://reviews.llvm.org/D65240