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

[Carlos Alberto Enciso via Phabricator via llvm-commits]

CarlosAlbertoEnciso added inline comments.


================
Comment at: llvm/lib/DebugInfo/LogicalView/Core/LVScope.cpp:1118
+// Record scopes with invalid ranges.
+void LVScopeCompileUnit::addInvalidRange(LVLocation *Location) {
+  LVScope *Scope = Location->getParentScope();
----------------
psamolysov wrote:
> The code is mostly identical to `LVScopeCompileUnit::addInvalidLocation(LVLocation *Location)`, it may make sense to extract the body in a `static` free function and pass into it an additional parameter: `LVOffsetLocationsMap` which is either `&InvalidLocations` or `&InvalidRanges`.
Refactored as:
```
void LVScopeCompileUnit::addInvalidLocationOrRange(LVLocation *Location,
                                                   LVElement *Element,
                                                   LVOffsetLocationsMap *Map) {
  LVOffset Offset = Element->getOffset();
  addInvalidOffset(Offset, Element);
  addItem<LVOffsetLocationsMap, LVLocations, LVOffset, LVLocation *>(
      Map, Offset, Location);
}

// Record symbols with invalid locations.
void LVScopeCompileUnit::addInvalidLocation(LVLocation *Location) {
  addInvalidLocationOrRange(Location, Location->getParentSymbol(),
                            &InvalidLocations);
}

// Record scopes with invalid ranges.
void LVScopeCompileUnit::addInvalidRange(LVLocation *Location) {
  addInvalidLocationOrRange(Location, Location->getParentScope(),
                            &InvalidRanges);
}

```


================
Comment at: llvm/unittests/DebugInfo/LogicalView/WarningInternalTest.cpp:52-54
+  if (Iter != AddressToLineData.begin())
+    Iter = std::prev(Iter);
+  return (Iter != AddressToLineData.end()) ? Iter->second : nullptr;
----------------
psamolysov wrote:
> Here could be a bug: if `AddressToLineData.upper_bound(Address)` returns `AddressToLineData.end()`, `AddressToLineData.end()` is definitely not equal to `AddressToLineData.begin()`, `Iter` changes it's value to an element previous to the `end` and the check `Iter != AddressToLineData.end()` is forever be true.
Sorry, I'm afraid I don't follow you.

The logic behind that code, is to get a `LVLine` that corresponds to the given `Address`.

Let's assume we have the following values:
```
{ Address 100, Line-1 } => Line-1 represents the address range [100-199] and [...-100]
{ Address 200, Line-2 } => Line-2 represents the address range [200-299]
{ Address 300, Line-3 } => Line-3 represents the address range [300-...]
```
Then
```
lineUpperBound(50)  -> Line-1
lineUpperBound(100) -> Line-1
lineUpperBound(150) -> Line-1
lineUpperBound(200) -> Line-2
lineUpperBound(250) -> Line-2
lineUpperBound(300) -> Line-3
lineUpperBound(350) -> Line-3
```



Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D125781