[PATCH] D76996: [analyzer] Improve PlacementNewChecker

[Gabor Marton via Phabricator via cfe-commits]

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