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 _______________________________________________ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
