[llvm] 31eeac9 - [llvm-readelf/obj] - Move unique warning handling logic to the `ObjDumper`.

Georgii Rymar via llvm-commits llvm-commits at lists.llvm.org
Mon Nov 30 23:59:47 PST 2020


Author: Georgii Rymar
Date: 2020-12-01T10:53:00+03:00
New Revision: 31eeac915a0a25c2690b956931c77684bc34da0b

URL: https://github.com/llvm/llvm-project/commit/31eeac915a0a25c2690b956931c77684bc34da0b
DIFF: https://github.com/llvm/llvm-project/commit/31eeac915a0a25c2690b956931c77684bc34da0b.diff

LOG: [llvm-readelf/obj] - Move unique warning handling logic to the `ObjDumper`.

This moves the `reportUniqueWarning` method to the base class.

My motivation is the following:
I've experimented with replacing `reportWarning` calls with `reportUniqueWarning`
in ELF dumper. I've found that for example for removing them from `DynRegionInfo` helper
class, it is worth to pass a dumper instance to it (to be able to call dumper()->reportUniqueWarning()).
The problem was that `ELFDumper<ELFT>` is a template class. I had to make `DynRegionInfo` to be templated
and do lots of minor changes everywhere what did not look reasonable/nice.

At the same time I guess one day other dumpers like COFF/MachO/Wasm etc might want to
start using `reportUniqueWarning` API too. Then it looks reasonable to move the logic to the
base class.

With that the problem of passing the dumper instance will be gone.

Differential revision: https://reviews.llvm.org/D92218

Added: 
    

Modified: 
    llvm/tools/llvm-readobj/COFFDumper.cpp
    llvm/tools/llvm-readobj/ELFDumper.cpp
    llvm/tools/llvm-readobj/MachODumper.cpp
    llvm/tools/llvm-readobj/ObjDumper.cpp
    llvm/tools/llvm-readobj/ObjDumper.h
    llvm/tools/llvm-readobj/WasmDumper.cpp
    llvm/tools/llvm-readobj/XCOFFDumper.cpp

Removed: 
    


################################################################################
diff  --git a/llvm/tools/llvm-readobj/COFFDumper.cpp b/llvm/tools/llvm-readobj/COFFDumper.cpp
index b1ac1d9d0f39..144ecf56d50a 100644
--- a/llvm/tools/llvm-readobj/COFFDumper.cpp
+++ b/llvm/tools/llvm-readobj/COFFDumper.cpp
@@ -77,7 +77,8 @@ class COFFDumper : public ObjDumper {
 public:
   friend class COFFObjectDumpDelegate;
   COFFDumper(const llvm::object::COFFObjectFile *Obj, ScopedPrinter &Writer)
-      : ObjDumper(Writer), Obj(Obj), Writer(Writer), Types(100) {}
+      : ObjDumper(Writer, Obj->getFileName()), Obj(Obj), Writer(Writer),
+        Types(100) {}
 
   void printFileHeaders() override;
   void printSectionHeaders() override;

diff  --git a/llvm/tools/llvm-readobj/ELFDumper.cpp b/llvm/tools/llvm-readobj/ELFDumper.cpp
index 890bb2a21e82..f89d88724a9a 100644
--- a/llvm/tools/llvm-readobj/ELFDumper.cpp
+++ b/llvm/tools/llvm-readobj/ELFDumper.cpp
@@ -64,7 +64,6 @@
 #include <memory>
 #include <string>
 #include <system_error>
-#include <unordered_set>
 #include <vector>
 
 using namespace llvm;
@@ -355,8 +354,6 @@ template <typename ELFT> class ELFDumper : public ObjDumper {
   };
   mutable SmallVector<Optional<VersionEntry>, 16> VersionMap;
 
-  std::unordered_set<std::string> Warnings;
-
   std::string describe(const Elf_Shdr &Sec) const;
 
 public:
@@ -435,9 +432,6 @@ template <typename ELFT> class ELFDumper : public ObjDumper {
 
   Expected<RelSymbol<ELFT>> getRelocationTarget(const Relocation<ELFT> &R,
                                                 const Elf_Shdr *SymTab) const;
-
-  std::function<Error(const Twine &Msg)> WarningHandler;
-  void reportUniqueWarning(Error Err) const;
 };
 
 template <class ELFT>
@@ -956,14 +950,6 @@ template <typename ELFT> class GNUStyle : public DumpStyle<ELFT> {
                                     const Twine &Label, unsigned EntriesNum);
 };
 
-template <class ELFT>
-void ELFDumper<ELFT>::reportUniqueWarning(Error Err) const {
-  handleAllErrors(std::move(Err), [&](const ErrorInfoBase &EI) {
-    cantFail(WarningHandler(EI.message()),
-             "WarningHandler should always return ErrorSuccess");
-  });
-}
-
 template <class ELFT>
 void DumpStyle<ELFT>::reportUniqueWarning(Error Err) const {
   this->dumper().reportUniqueWarning(std::move(Err));
@@ -2020,16 +2006,9 @@ void ELFDumper<ELFT>::loadDynamicTable() {
 template <typename ELFT>
 ELFDumper<ELFT>::ELFDumper(const object::ELFObjectFile<ELFT> &O,
                            ScopedPrinter &Writer)
-    : ObjDumper(Writer), ObjF(O), Obj(*O.getELFFile()), DynRelRegion(O),
-      DynRelaRegion(O), DynRelrRegion(O), DynPLTRelRegion(O), DynamicTable(O) {
-  // Dumper reports all non-critical errors as warnings.
-  // It does not print the same warning more than once.
-  WarningHandler = [this](const Twine &Msg) {
-    if (Warnings.insert(Msg.str()).second)
-      reportWarning(createError(Msg), ObjF.getFileName());
-    return Error::success();
-  };
-
+    : ObjDumper(Writer, O.getFileName()), ObjF(O), Obj(*O.getELFFile()),
+      DynRelRegion(O), DynRelaRegion(O), DynRelrRegion(O), DynPLTRelRegion(O),
+      DynamicTable(O) {
   if (opts::Output == opts::GNU)
     ELFDumperStyle.reset(new GNUStyle<ELFT>(Writer, *this));
   else

diff  --git a/llvm/tools/llvm-readobj/MachODumper.cpp b/llvm/tools/llvm-readobj/MachODumper.cpp
index 5c4960804e8f..c13b1f3bf2a0 100644
--- a/llvm/tools/llvm-readobj/MachODumper.cpp
+++ b/llvm/tools/llvm-readobj/MachODumper.cpp
@@ -27,7 +27,7 @@ namespace {
 class MachODumper : public ObjDumper {
 public:
   MachODumper(const MachOObjectFile *Obj, ScopedPrinter &Writer)
-      : ObjDumper(Writer), Obj(Obj) {}
+      : ObjDumper(Writer, Obj->getFileName()), Obj(Obj) {}
 
   void printFileHeaders() override;
   void printSectionHeaders() override;

diff  --git a/llvm/tools/llvm-readobj/ObjDumper.cpp b/llvm/tools/llvm-readobj/ObjDumper.cpp
index f46993b4f428..b830628e7f93 100644
--- a/llvm/tools/llvm-readobj/ObjDumper.cpp
+++ b/llvm/tools/llvm-readobj/ObjDumper.cpp
@@ -26,9 +26,23 @@ static inline Error createError(const Twine &Msg) {
   return createStringError(object::object_error::parse_failed, Msg);
 }
 
-ObjDumper::ObjDumper(ScopedPrinter &Writer) : W(Writer) {}
+ObjDumper::ObjDumper(ScopedPrinter &Writer, StringRef ObjName) : W(Writer) {
+  // Dumper reports all non-critical errors as warnings.
+  // It does not print the same warning more than once.
+  WarningHandler = [=](const Twine &Msg) {
+    if (Warnings.insert(Msg.str()).second)
+      reportWarning(createError(Msg), ObjName);
+    return Error::success();
+  };
+}
+
+ObjDumper::~ObjDumper() {}
 
-ObjDumper::~ObjDumper() {
+void ObjDumper::reportUniqueWarning(Error Err) const {
+  handleAllErrors(std::move(Err), [&](const ErrorInfoBase &EI) {
+    cantFail(WarningHandler(EI.message()),
+             "WarningHandler should always return ErrorSuccess");
+  });
 }
 
 static void printAsPrintable(raw_ostream &W, const uint8_t *Start, size_t Len) {

diff  --git a/llvm/tools/llvm-readobj/ObjDumper.h b/llvm/tools/llvm-readobj/ObjDumper.h
index fa1767fe8902..156c2d19c8f6 100644
--- a/llvm/tools/llvm-readobj/ObjDumper.h
+++ b/llvm/tools/llvm-readobj/ObjDumper.h
@@ -16,6 +16,8 @@
 #include "llvm/Object/ObjectFile.h"
 #include "llvm/Support/CommandLine.h"
 
+#include <unordered_set>
+
 namespace llvm {
 namespace object {
 class COFFImportFile;
@@ -32,7 +34,7 @@ class ScopedPrinter;
 
 class ObjDumper {
 public:
-  ObjDumper(ScopedPrinter &Writer);
+  ObjDumper(ScopedPrinter &Writer, StringRef ObjName);
   virtual ~ObjDumper();
 
   virtual bool canDumpContent() { return true; }
@@ -109,6 +111,9 @@ class ObjDumper {
   void printSectionsAsHex(const object::ObjectFile &Obj,
                           ArrayRef<std::string> Sections);
 
+  std::function<Error(const Twine &Msg)> WarningHandler;
+  void reportUniqueWarning(Error Err) const;
+
 protected:
   ScopedPrinter &W;
 
@@ -117,6 +122,8 @@ class ObjDumper {
   virtual void printDynamicSymbols() {}
   virtual void printProgramHeaders() {}
   virtual void printSectionMapping() {}
+
+  std::unordered_set<std::string> Warnings;
 };
 
 std::unique_ptr<ObjDumper> createCOFFDumper(const object::COFFObjectFile &Obj,

diff  --git a/llvm/tools/llvm-readobj/WasmDumper.cpp b/llvm/tools/llvm-readobj/WasmDumper.cpp
index bb07b9d5bd15..488c6faa26b8 100644
--- a/llvm/tools/llvm-readobj/WasmDumper.cpp
+++ b/llvm/tools/llvm-readobj/WasmDumper.cpp
@@ -57,7 +57,7 @@ static const EnumEntry<unsigned> WasmSymbolFlags[] = {
 class WasmDumper : public ObjDumper {
 public:
   WasmDumper(const WasmObjectFile *Obj, ScopedPrinter &Writer)
-      : ObjDumper(Writer), Obj(Obj) {}
+      : ObjDumper(Writer, Obj->getFileName()), Obj(Obj) {}
 
   void printFileHeaders() override;
   void printSectionHeaders() override;

diff  --git a/llvm/tools/llvm-readobj/XCOFFDumper.cpp b/llvm/tools/llvm-readobj/XCOFFDumper.cpp
index 3e34b9e104ed..8f0f18cedceb 100644
--- a/llvm/tools/llvm-readobj/XCOFFDumper.cpp
+++ b/llvm/tools/llvm-readobj/XCOFFDumper.cpp
@@ -24,7 +24,7 @@ class XCOFFDumper : public ObjDumper {
 
 public:
   XCOFFDumper(const XCOFFObjectFile &Obj, ScopedPrinter &Writer)
-      : ObjDumper(Writer), Obj(Obj) {}
+      : ObjDumper(Writer, Obj.getFileName()), Obj(Obj) {}
 
   void printFileHeaders() override;
   void printSectionHeaders() override;


        


More information about the llvm-commits mailing list