dcoughlin accepted this revision.
dcoughlin added a comment.
This revision is now accepted and ready to land.
Looks good to me. I have a comment about simplifying
createTemporaryRegionIfNeeded() (if possible) inline.
================
Comment at: lib/StaticAnalyzer/Core/ExprEngine.cpp:281
+
+ if (!TR) {
+ StorageDuration SD = MT->getStorageDuration();
----------------
Would it be safe for the body of the `if (!TR)` to be the else branch of `if
constCXXTempObjectionRegion *const *TRPtr = ...` rather then its own if
statement?
================
Comment at: lib/StaticAnalyzer/Core/ExprEngine.cpp:289
}
if (!TR)
TR = MRMgr.getCXXTempObjectRegion(Init, LC);
----------------
Would it be safe for `TR = MRMgr.getCXXTempObjectRegion(Init, LC);` to be the
else branch of `if (const MaterializeTemporaryExpr *MT =
dyn_cast<MaterializeTemporaryExpr>(Result))` rather than its own `if` statement?
Ideally the number paths through this function on which we call
`MRMgr.getCXXTempObjectRegion()` would be small and very clear.
================
Comment at: lib/StaticAnalyzer/Core/ExprEngine.cpp:490
+static void printTemporaryMaterializatinosForContext(
+ raw_ostream &Out, ProgramStateRef State, const char *NL, const char *Sep,
----------------
Nit: There is a typo in the name here ("Materializatinos"). I guess this is the
sub-atomic particle created as a byproduct of materializing a temporary! We
have a lot of materializatinos in Cupertino.
================
Comment at: test/Analysis/lifetime-extension.cpp:45
+ clang_analyzer_eval(z == 2);
+#ifdef TEMPORARIES
+ // expected-warning@-4{{TRUE}}
----------------
Yay!
================
Comment at: test/Analysis/lifetime-extension.cpp:78
+ {
+ const C &c = C(1, &after, &before);
+ }
----------------
Nice.
https://reviews.llvm.org/D43497
_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
