martong added inline comments.
================
Comment at: clang/lib/StaticAnalyzer/Checkers/CheckPlacementNew.cpp:261
+
+ if (const VarRegion *TheVarRegion = BaseRegion->getAs<VarRegion>()) {
+ const VarDecl *TheVarDecl = TheVarRegion->getDecl();
----------------
Perhaps you could call instead `checkVarRegionAlign()`?
================
Comment at: clang/lib/StaticAnalyzer/Checkers/CheckPlacementNew.cpp:261
+
+ if (const VarRegion *TheVarRegion = BaseRegion->getAs<VarRegion>()) {
+ const VarDecl *TheVarDecl = TheVarRegion->getDecl();
----------------
martong wrote:
> Perhaps you could call instead `checkVarRegionAlign()`?
Also, I think `BaseRegion` can be an `ElementRegion` here as well. So, in that
case we should call into `checkElementRegionAlign()`, shouldn't we? This draws
a pattern that we should recursively descend down to the top most base region.
I.e. the different `check*RegionAlign` methods should call into each other
until we reach the top level base region.
The observation here is that the alignment of a region can be correct only if
we can prove that its base region is aligned properly (and other requirements,
e.g. the offset is divisible). But the base region may have another base region
and we have to prove the alignment correctness to that as well.
I hope this makes sense, please correct me if I am wrong.
================
Comment at: clang/test/Analysis/placement-new.cpp:245
+
+ // ok. 2(short align) + 3*2(index '3' offset)
+ ::new (&b[3]) long;
----------------
So these tests failed after you rewrote ElementRegion processing, right?
Actually, I wonder why we thought that if x is divisible by 2 then (x+6) will
be divisible by 8 unconditionally.It's good you have this fixed.
Repository:
rG LLVM Github Monorepo
CHANGES SINCE LAST ACTION
https://reviews.llvm.org/D76996/new/
https://reviews.llvm.org/D76996
_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
