[PATCH] D54466: [Analyzer] Iterator Checkers - Use the region of the topmost base class for iterators stored in a region

[Artem Dergachev via Phabricator via cfe-commits]

NoQ added a comment.

Huh, gotcha :) The nomenclature is mostly correct, just feels weird.

Super-region of a region is a larger region. Sub-region is a smaller region. Base region is the largest non-memspace superregion, so it's a //larger// region.

Derived class object is a large object. Base class object is a small object within the derived class object, so it should be represented by a //smaller// region.

Hence we have `CXXBaseObjectRegion` which is a small sub-region that corresponds to the object of the base class within the object of a derived class. Which means that yes, you obtain the base region by removing `CXXBaseObjectRegion` layers (i.e., making the region larger), and that is fine because "base region" in our terminology means a completely different thing.

Now, right, there's the `CXXDerivedObjectRegion` that i added recently. It represents the region of a derived object if its super-object isn't a `CXXBaseObjectRegion` (otherwise we can just unwrap the `CXXBaseObjectRegion`). So it kinda sounds like a sub-region that is larger than its superregion, which contradicts the tradition. But this is fine because if the program under analysis is valid (which we assume unless we can prove the opposite), the only valid use case for `CXXDerivedObjectRegion` is to place it on top of a `SymbolicRegion` which points to an object of unknown size. So we kinda don't really know what's larger here. Also, that's not the only example of the situation when a sub-region is not a sub-segment of its super-region. We can always add an `ElementRegion` with an arbitrary offset over almost any other region and it'll be equally wonky (i.e., a valid program shouldn't make us do that, but if it does, then, well, it's not surprising that weird things are happening).



================
Comment at: include/clang/StaticAnalyzer/Core/PathSensitive/MemRegion.h:121
 
+  const MemRegion *getMostDerivedObjectRegion() const;
+
----------------
Let's start writing doxygen thingies!


================
Comment at: lib/StaticAnalyzer/Core/MemRegion.cpp:1182-1188
+  while (true) {
+    if (R->getKind() == MemRegion::CXXBaseObjectRegionKind) {
+        R = cast<SubRegion>(R)->getSuperRegion();
+        continue;
+    }
+    break;
+  }
----------------
My attempt:
```lang=c++
while (const auto *BR = dyn_cast<CXXBaseObjectRegion>(R))
  R = BR->getSuperRegion();
```


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

https://reviews.llvm.org/D54466