[PATCH] D125783: [llvm-dva] 08 - ELF Reader

[Carlos Alberto Enciso via Phabricator via llvm-commits]

CarlosAlbertoEnciso added inline comments.


================
Comment at: llvm/include/llvm/DebugInfo/LogicalView/Core/LVReader.h:90
   void addCompileUnitOffset(LVOffset Offset, LVScopeCompileUnit *CompileUnit) {
     CompileUnits.insert(std::make_pair(Offset, CompileUnit));
   }
----------------
psamolysov wrote:
> 
Replaced with `emplace`.


================
Comment at: llvm/include/llvm/DebugInfo/LogicalView/Core/LVReader.h:149-150
 
+  LVSectionIndex getDotTextSectionIndex() { return DotTextSectionIndex; }
+  virtual LVSectionIndex getSectionIndex(LVScope *Scope) {
+    return getDotTextSectionIndex();
----------------
psamolysov wrote:
> Should both member functions be `const`?
Marked `getDotTextSectionIndex` as `const`.
Marking `getSectionIndex` as `const` generates quite few errors as other called functions are required to be changed to `const`. Leaving it unchanged.


================
Comment at: llvm/include/llvm/DebugInfo/LogicalView/Core/LVReader.h:154
+
   virtual bool isSystemEntry(LVElement *Element, StringRef Name = {}) {
     return false;
----------------
psamolysov wrote:
> This function looks like `const` too.
Marked `isSystemEntry` as `const`.


================
Comment at: llvm/include/llvm/DebugInfo/LogicalView/Core/LVSupport.h:164
     List = new ListType();
     Map->insert(std::make_pair(Key, List));
   }
----------------
psamolysov wrote:
> 
Already changed to use `emplace` in previous patch.


================
Comment at: llvm/include/llvm/DebugInfo/LogicalView/Core/LVSupport.h:190
+      SecondMap = new LVSecondMapType();
+      FirstMap.insert(std::make_pair(FirstKey, SecondMap));
+    } else {
----------------
psamolysov wrote:
> 
Changed to use `emplace`.


================
Comment at: llvm/include/llvm/DebugInfo/LogicalView/Core/LVSupport.h:197
+    if (SecondMap && SecondMap->find(SecondKey) == SecondMap->end())
+      SecondMap->insert(std::make_pair(SecondKey, Value));
+  }
----------------
psamolysov wrote:
> 
Changed to use `emplace`.


================
Comment at: llvm/include/llvm/DebugInfo/LogicalView/Readers/LVBinaryReader.h:35
+// Logical scope, Section address, Section index, IsComdat.
+struct LVSymbolTableEntry {
+  LVScope *Scope = nullptr;
----------------
psamolysov wrote:
> The struct should either be marked as `final` or have a virtual destructor.
Marked as `final`.


================
Comment at: llvm/include/llvm/DebugInfo/LogicalView/Readers/LVBinaryReader.h:48
+// Function names extracted from the object symbol table.
+class LVSymbolTable {
+  using LVSymbolNames = std::map<std::string, LVSymbolTableEntry>;
----------------
psamolysov wrote:
> The class should either be marked as `final` or have a virtual destructor.
Marked as `final`.


================
Comment at: llvm/include/llvm/DebugInfo/LogicalView/Readers/LVBinaryReader.h:87
+    if (SectionAddresses.find(Section.getAddress()) == SectionAddresses.end())
+      SectionAddresses.insert(std::make_pair(Section.getAddress(), Section));
+  }
----------------
psamolysov wrote:
> 
Replace to use `emplace`.


================
Comment at: llvm/include/llvm/DebugInfo/LogicalView/Readers/LVELFReader.h:101
+      // know if the referenced element has been created.
+      GlobalOffsets.insert(std::make_pair(Offset, nullptr));
+  }
----------------
psamolysov wrote:
> 
Replaced to use `emplace`.


================
Comment at: llvm/lib/DebugInfo/LogicalView/Core/LVScope.cpp:1435
+         Iter != PublicNames.end(); ++Iter)
+      SortedNames.insert(std::make_pair(Iter->first->getOffset(), Iter));
+
----------------
psamolysov wrote:
> 
Replaced to use `emplace`.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D125783