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

[Henry Wong via Phabricator via cfe-commits]

MTC marked 2 inline comments as done.
MTC 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;
+
----------------
NoQ wrote:
> 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.
Make sense, will do.


================
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");
+
----------------
NoQ wrote:
> 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.
Excellent advice, thank you! Will do.


Repository:
  rC Clang

https://reviews.llvm.org/D45491