[PATCH] D45491: [analyzer] Do not invalidate the `this` pointer.

[Artem Dergachev via Phabricator via cfe-commits]

NoQ added inline comments.


================
Comment at: lib/StaticAnalyzer/Core/RegionStore.cpp:1024-1027
+  // 'this' pointer is not an lvalue, we should not invalidate it.
+  if (CXXThisRegion::classof(baseR))
+    return;
+
----------------
I don't think this run-time check belongs here. The fix should be isolated in loop widening because everything else is already known to work correctly. The invalidation worker is not to be blamed for other entities throwing incorrect stuff into it.


================
Comment at: lib/StaticAnalyzer/Core/RegionStore.cpp:2057-2060
+  assert((!CXXThisRegion::classof(R) ||
+          CXXThisRegion::classof(R) && !B.lookup(R)) &&
+         "'this' pointer is not an l-value and is not assignable");
+
----------------
This assertion is great to have.

Please use `!isa<CXXThisRegion>(R)` instead of `CXXThisRegion::classof(R)`. The left argument of `&&` is always true, so you can omit it.


================
Comment at: lib/StaticAnalyzer/Core/RegionStore.cpp:2062
   // Clear out bindings that may overlap with this binding.
-  RegionBindingsRef NewB = removeSubRegionBindings(B, cast<SubRegion>(R));
+  RegionBindingsRef NewB = removeSubRegionBindings(B, cast<SubRegion>(R));  
   return NewB.addBinding(BindingKey::Make(R, BindingKey::Direct), V);
----------------
An accidental whitespace change here.


Repository:
  rC Clang

https://reviews.llvm.org/D45491