[PATCH] D25826: [ELF] Show error location for 'undefined symbol' errors

Rui Ueyama via llvm-commits llvm-commits at lists.llvm.org
Thu Oct 20 11:48:01 PDT 2016

ruiu added a comment.

Thank you for doing this. This would be very useful.

Comment at: ELF/Error.cpp:42
+void elf::error(const Twine &Source, const Twine &Msg) {
+  *ErrorOS << Source << ": error: " << Msg << "\n";
   HasError = true;
I think I'm inclined to showing ARGV[0] even for undefined symbol errors to keep them consistent with other linker-issued errors. I do not see a reason to not do except for making error lines a bit shorter.

Comment at: ELF/InputFiles.cpp:405
 template <class ELFT>
+DefinedRegular<ELFT> *
A brief comment would be appreciated. Please note that this is not an optimized function but it is okay because this is for creating an error message.

Comment at: ELF/InputFiles.cpp:407
+DefinedRegular<ELFT> *
+elf::ObjectFile<ELFT>::getEnclosingSymbol(InputSectionBase<ELFT> *S,
+                                          uintX_t Offset) {
This function can be implemented as a non-member function, no? Non-member functions are preferred over member functions if they access only public members.

Comment at: ELF/InputFiles.cpp:409-411
+  for (SymbolBody *B : SymbolBodies) {
+    auto *D = dyn_cast<DefinedRegular<ELFT>>(B);
+    if (!D)
  for (...)
     if (auto *D = ...)
       if (D->Value <= ...)
          return D;
  return nullptr;

Comment at: ELF/InputFiles.h:178-179
+  // Returns name of source file or empty string, if it cannot be fetched.
+  StringRef getSourceFile() { return SourceFile; }
I'd make `SourceFile` a public member variable and remove the accessor, as the accessor doesn't contain a logic.

Comment at: ELF/InputFiles.h:182
+  // Gets symbol which encloses given offset and belong to given section
+  DefinedRegular<ELFT> *getEnclosingSymbol(InputSectionBase<ELFT> *S,
+                                           uintX_t Offset);
Probably a personal taste, but how about `getSymbolAt`?


More information about the llvm-commits mailing list