[PATCH] D132142: [analyzer] Prefer wrapping SymbolicRegions by ElementRegions

[Gabor Marton via Phabricator via cfe-commits]

martong accepted this revision.
martong added a comment.
This revision is now accepted and ready to land.

I am okay with this change, it does give a proper canonical form, which is good. On the other hand, I agree that (on the long term) base regions and offsets would be a better memory model than what we have now with field and element regions.



================
Comment at: clang/include/clang/StaticAnalyzer/Core/PathSensitive/MemRegion.h:799
+  /// we actually know their types.
+  QualType getApproximatedType() const {
+    return sym->getType()->getPointeeType();
----------------
I think we should express that this returns the


  - type of the pointee
  - the type that is the "static" type, id est, the type of the declaration; `void`, `char` and `base` in your example above.

In this sense, what about `getPointeeStaticType`?




================
Comment at: clang/test/Analysis/ctor.mm:221
       clang_analyzer_eval(p4.x > 0); // expected-warning{{TRUE}}
-    // FIXME: Element region gets in the way, so these aren't the same symbols
-    // as they should be.
-    clang_analyzer_eval(pp.x == p4.x); // expected-warning{{UNKNOWN}}
+    clang_analyzer_eval(pp.x == p4.x); // expected-warning{{TRUE}}
 
----------------
Nice!


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D132142/new/

https://reviews.llvm.org/D132142