[PATCH] D125784: [llvm-dva] 09 - CodeView Reader

[Carlos Alberto Enciso via Phabricator via llvm-commits]

CarlosAlbertoEnciso added inline comments.


================
Comment at: llvm/lib/DebugInfo/LogicalView/Readers/LVCodeViewVisitor.cpp:287
+    if (Strings.find(TI) != Strings.end())
+      String = std::get<1>(Strings[TI]);
+    return String;
----------------
psamolysov wrote:
> Here we are looking up in the map again. `Strings.find(TI)` has already returned an iterator, could the iterator be reused?
Replaced with
```
  StringRef find(TypeIndex TI) {
    StringIds::iterator Iter = Strings.find(TI);
    return Iter != Strings.end() ? std::get<1>(Iter->second) : StringRef{};
  }
```



================
Comment at: llvm/lib/DebugInfo/LogicalView/Readers/LVCodeViewVisitor.cpp:294
+    if (Strings.find(TI) != Strings.end())
+      Index = std::get<0>(Strings[TI]);
+    return Index;
----------------
psamolysov wrote:
> Here we are looking up in the map again. `Strings.find(TI)` has already returned an iterator, could the iterator be reused?
Replaced with
```
  uint32_t findIndex(TypeIndex TI) {
    StringIds::iterator Iter = Strings.find(TI);
    return Iter != Strings.end() ? std::get<0>(Iter->second) : 0;
  }
```



================
Comment at: llvm/lib/DebugInfo/LogicalView/Readers/LVCodeViewVisitor.cpp:352
+  if (Target.find(TI) != Target.end()) {
+    Element = Target[TI].second;
+    if (Element)
----------------
psamolysov wrote:
> Here and few times below we are looking up in the map again and again. `Target.find(TI)` has already returned an iterator, could the iterator be reused?
Replaced to use the iterator returned by `Target.find(TI)`.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D125784