[PATCH] D125784: [llvm-debuginfo-analyzer] 09 - CodeView Reader

[Carlos Alberto Enciso via Phabricator via llvm-commits]

CarlosAlbertoEnciso added inline comments.


================
Comment at: llvm/lib/DebugInfo/LogicalView/Readers/LVCodeViewReader.cpp:697
+    if (SubType & SubsectionIgnoreFlag)
+      SubType &= ~SubsectionIgnoreFlag;
+
----------------
probinson wrote:
> Might as well turn this off unconditionally.
Changed:


```
    if (SubType & SubsectionIgnoreFlag)
      SubType &= ~SubsectionIgnoreFlag;
```
to
```
    // Process the subsection as normal even if the ignore bit is set.
    SubType &= ~SubsectionIgnoreFlag;
```


================
Comment at: llvm/lib/DebugInfo/LogicalView/Readers/LVCodeViewVisitor.cpp:221
+
+  using LookupSet = std::set<StringRef>;
+  LookupSet DeductedScopes;
----------------
psamolysov wrote:
> There is no based on comparison operations with `DeducedScopes`, `UnresolvedScopes` and `IdentifiedNamespaces`, the sets are used for searching only. It looks like the `std::unordered_set` with O(1) inserting/searching is enough (hm, maybe except the `UnresolvedScopes` variable because there is an iteration over the set and the order might be important).
This is a very good observation.

Few points:

Changing to `using LookupSet = std::unordered_set<StringRef>;`
```
compilation fails with:
'std::_Uhash_compare<_Kty,_Hasher,_Keyeq>::_Uhash_compare(const std::_Uhash_compare<_Kty,_Hasher,_Keyeq> &)': attempting to reference a deleted function
```
Changing to `using LookupSet = std::unordered_set<std::string>;`
```
Requires changes in API signatures for functions used in `LVNamespaceDeduction` that are expecting `StringRef` returned by `getInnerComponent` and `getAllLexicalIndexes`.
```
I am adding this optimization to the list of items to be addressed in later patches.



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

https://reviews.llvm.org/D125784