[PATCH] D125781: [llvm-dva] 06 - Warning and internal options

[Carlos Alberto Enciso via Phabricator via llvm-commits]

CarlosAlbertoEnciso added inline comments.


================
Comment at: llvm/include/llvm/DebugInfo/LogicalView/Core/LVSupport.h:162
+  else {
+    List = new ListType();
+    Map->insert(std::make_pair(Key, List));
----------------
psamolysov wrote:
> I'm sorry but I cannot find where the `List` is deleted. So, I get the `Map` takes ownership of the `List` but there is no destructor of function where the `Map`'s entries (of the `DebugTags` map, for example) is deleted.
Very good catch. The destructor part is missing.
```
// Delete the map contained list.
template <typename MapType> void deleteList(MapType &Map) {
  for (typename MapType::const_reference Entry : Map)
    delete Entry.second;
}
```


================
Comment at: llvm/include/llvm/DebugInfo/LogicalView/Core/LVSupport.h:163
+    List = new ListType();
+    Map->insert(std::make_pair(Key, List));
+  }
----------------
psamolysov wrote:
> psamolysov wrote:
> > I believe this should work too.
> Or, it could be better `Map->emplace(Key, List);`.
Replaced with `Map->emplace(Key, List);`.


================
Comment at: llvm/lib/DebugInfo/LogicalView/Core/LVReader.cpp:41
+    if (Iter == Integrity.end())
+      Integrity.insert(std::make_pair(Element, Scope));
+    else
----------------
psamolysov wrote:
> psamolysov wrote:
> > I believe this should work.
> Or, it could be better `Integrity.emplace(Element, Scope);`.
Replaced with `Integrity.emplace(Element, Scope);`.


================
Comment at: llvm/lib/DebugInfo/LogicalView/Core/LVReader.cpp:44
+      // We found a duplicated.
+      Duplicate.push_back(LVDuplicateEntry(Element, Scope, Iter->second));
+  };
----------------
psamolysov wrote:
> To create an `LVDuplicateEntry` in place.
Replaced with `Duplicate.emplace_back(Element, Scope, Iter->second);`


================
Comment at: llvm/lib/DebugInfo/LogicalView/Core/LVReader.cpp:72
+
+    auto printIndex = [&](unsigned Index) {
+      if (Index)
----------------
psamolysov wrote:
> If `format` is the `llvm::format` function, the lambda captures nothing.
You are correct.
Changed to `auto PrintIndex = [](unsigned Index) {`.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D125781