martong accepted this revision.
martong added a comment.
This revision is now accepted and ready to land.
LGTM! Thanks! But I am not that confident with the element regions and field
regions, so @NoQ could you please take another look?
================
Comment at: clang/lib/StaticAnalyzer/Checkers/CheckPlacementNew.cpp:82
CharUnits TypeSize = AstContext.getTypeSizeInChars(ElementType);
- if (NE->isArray()) {
+ if (IsArray = NE->isArray()) {
const Expr *SizeExpr = *NE->getArraySize();
----------------
This will break build-bots that run with -Werror.
```
../../git/llvm-project/clang/lib/StaticAnalyzer/Checkers/CheckPlacementNew.cpp:82:15:
warning: suggest parentheses around assignment used as truth value
[-Wparentheses]
if (IsArray = NE->isArray()) {
```
================
Comment at: clang/test/Analysis/placement-new.cpp:256
+
+void f9() {
+ struct X {
----------------
First I was wondering if we indeed handle correctly structs with nested arrays
whose element's type is a structs with nested arrays (... and so on).
So, I tried the below test, and it seems okay. Thus I think it might be worth
to add something similar to it.
```
void f9_1() {
struct Y {
char a;
alignas(alignof(short)) char b[20];
};
struct X {
char e;
Y f[20];
} Xi; // expected-note {{'Xi' initialized here}}
// ok 2(custom align) + 6*1(index '6' offset)
::new (&Xi.f[6].b[6]) long;
// bad 2(custom align) + 1*1(index '1' offset)
::new (&Xi.f[1].b[1]) long; // expected-warning{{Storage type is aligned to 3
bytes but allocated type is aligned to 8 bytes}} expected-note 1 {{}}
}
```
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
