<html><head><meta http-equiv="Content-Type" content="text/html charset=iso-8859-1"></head><body style="word-wrap: break-word; -webkit-nbsp-mode: space; -webkit-line-break: after-white-space;"><div>Oops, no. Thanks for keeping me honest. Will add soon.</div><br><div><div>On Jul 24, 2013, at 8:33 , Manuel Klimek <<a href="mailto:klimek@google.com">klimek@google.com</a>> wrote:</div><br class="Apple-interchange-newline"><blockquote type="cite"><div style="font-family: Helvetica; font-size: 12px; font-style: normal; font-variant: normal; font-weight: normal; letter-spacing: normal; line-height: normal; orphans: auto; text-align: start; text-indent: 0px; text-transform: none; white-space: normal; widows: auto; word-spacing: 0px; -webkit-text-stroke-width: 0px;"><div dir="ltr">Jordan, did you add a regression test for this?</div><div class="gmail_extra"><br><br><div class="gmail_quote">On Tue, Jul 23, 2013 at 4:29 AM, Jordan Rose<span class="Apple-converted-space"> </span><span dir="ltr"><<a href="mailto:jordan_rose@apple.com" target="_blank">jordan_rose@apple.com</a>></span><span class="Apple-converted-space"> </span>wrote:<br><blockquote class="gmail_quote" style="margin: 0px 0px 0px 0.8ex; border-left-width: 1px; border-left-color: rgb(204, 204, 204); border-left-style: solid; padding-left: 1ex;"><div style="word-wrap: break-word;"><div>I did get around to reverting this, Pavel. Apart from <a href="http://llvm.org/bugs/show_bug.cgi?id=16664" target="_blank">PR16664</a>, I realized that neither the binary operators nor their subexpressions are still live when the analyzer goes back through the chain of logical expressions to see which destructors to run. (You can see how this is working using the debug.DumpCFG checker.) Without the values previously computed for the expression, the analyzer won't actually be consistent about which branches to take.</div><div><br></div><div>I don't have an immediate solution in mind. It's sort of non-trivial to teach the liveness analysis that an expression is going to be revisited later, but the only alternative I can think of is crawling backwards through the evaluated blocks to find out which branch we took, which is a horrible idea because it could include inlined calls. If you come up with anything, please send it up for review!</div><span class="HOEnZb"><font color="#888888"><div><br></div><div>Jordan</div></font></span><div><div class="h5"><div><br></div><br><div><div>On Jul 22, 2013, at 19:15 , Jordan Rose <<a href="mailto:jordan_rose@apple.com" target="_blank">jordan_rose@apple.com</a>> wrote:</div><br><blockquote type="cite">Author: jrose<br>Date: Mon Jul 22 21:15:11 2013<br>New Revision: 186925<br><br>URL:<span class="Apple-converted-space"> </span><a href="http://llvm.org/viewvc/llvm-project?rev=186925&view=rev" target="_blank">http://llvm.org/viewvc/llvm-project?rev=186925&view=rev</a><br>Log:<br>Revert "[analyzer] Add very limited support for temporary destructors"<br><br>The analyzer doesn't currently expect CFG blocks with terminators to be<br>empty, but this can happen when generating conditional destructors for<br>a complex logical expression, such as (a && (b || Temp{})). Moreover,<br>the branch conditions for these expressions are not persisted in the<br>state. Even for handling noreturn destructors this needs more work.<br><br>This reverts r186498.<br><br>Modified:<br> cfe/trunk/lib/StaticAnalyzer/Core/AnalyzerOptions.cpp<br> cfe/trunk/lib/StaticAnalyzer/Core/ExprEngine.cpp<br> cfe/trunk/lib/StaticAnalyzer/Core/ExprEngineCallAndReturn.cpp<br> cfe/trunk/test/Analysis/analyzer-config.c<br> cfe/trunk/test/Analysis/analyzer-config.cpp<br> cfe/trunk/test/Analysis/dtor.cpp<br> cfe/trunk/test/Analysis/temp-obj-dtors-cfg-output.cpp<br><br>Modified: cfe/trunk/lib/StaticAnalyzer/Core/AnalyzerOptions.cpp<br>URL:<span class="Apple-converted-space"> </span><a href="http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/StaticAnalyzer/Core/AnalyzerOptions.cpp?rev=186925&r1=186924&r2=186925&view=diff" target="_blank">http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/StaticAnalyzer/Core/AnalyzerOptions.cpp?rev=186925&r1=186924&r2=186925&view=diff</a><br>==============================================================================<br>--- cfe/trunk/lib/StaticAnalyzer/Core/AnalyzerOptions.cpp (original)<br>+++ cfe/trunk/lib/StaticAnalyzer/Core/AnalyzerOptions.cpp Mon Jul 22 21:15:11 2013<br>@@ -119,7 +119,7 @@ bool AnalyzerOptions::getBooleanOption(O<br>bool AnalyzerOptions::includeTemporaryDtorsInCFG() {<br> return getBooleanOption(IncludeTemporaryDtorsInCFG,<br> "cfg-temporary-dtors",<br>- /* Default = */ true);<br>+ /* Default = */ false);<br>}<br><br>bool AnalyzerOptions::mayInlineCXXStandardLibrary() {<br><br>Modified: cfe/trunk/lib/StaticAnalyzer/Core/ExprEngine.cpp<br>URL:<span class="Apple-converted-space"> </span><a href="http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/StaticAnalyzer/Core/ExprEngine.cpp?rev=186925&r1=186924&r2=186925&view=diff" target="_blank">http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/StaticAnalyzer/Core/ExprEngine.cpp?rev=186925&r1=186924&r2=186925&view=diff</a><br>==============================================================================<br>--- cfe/trunk/lib/StaticAnalyzer/Core/ExprEngine.cpp (original)<br>+++ cfe/trunk/lib/StaticAnalyzer/Core/ExprEngine.cpp Mon Jul 22 21:15:11 2013<br>@@ -590,15 +590,7 @@ void ExprEngine::ProcessMemberDtor(const<br><br>void ExprEngine::ProcessTemporaryDtor(const CFGTemporaryDtor D,<br> ExplodedNode *Pred,<br>- ExplodedNodeSet &Dst) {<br>-<br>- QualType varType = D.getBindTemporaryExpr()->getSubExpr()->getType();<br>-<br>- // FIXME: Inlining of temporary destructors is not supported yet anyway, so we<br>- // just put a NULL region for now. This will need to be changed later.<br>- VisitCXXDestructor(varType, NULL, D.getBindTemporaryExpr(),<br>- /*IsBase=*/ false, Pred, Dst);<br>-}<br>+ ExplodedNodeSet &Dst) {}<br><br>void ExprEngine::Visit(const Stmt *S, ExplodedNode *Pred,<br> ExplodedNodeSet &DstTop) {<br><br>Modified: cfe/trunk/lib/StaticAnalyzer/Core/ExprEngineCallAndReturn.cpp<br>URL:<span class="Apple-converted-space"> </span><a href="http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/StaticAnalyzer/Core/ExprEngineCallAndReturn.cpp?rev=186925&r1=186924&r2=186925&view=diff" target="_blank">http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/StaticAnalyzer/Core/ExprEngineCallAndReturn.cpp?rev=186925&r1=186924&r2=186925&view=diff</a><br>==============================================================================<br>--- cfe/trunk/lib/StaticAnalyzer/Core/ExprEngineCallAndReturn.cpp (original)<br>+++ cfe/trunk/lib/StaticAnalyzer/Core/ExprEngineCallAndReturn.cpp Mon Jul 22 21:15:11 2013<br>@@ -807,12 +807,6 @@ bool ExprEngine::shouldInlineCall(const<br> AnalysisDeclContextManager &ADCMgr = AMgr.getAnalysisDeclContextManager();<br> AnalysisDeclContext *CalleeADC = ADCMgr.getContext(D);<br><br>- // Temporary object destructor processing is currently broken, so we never<br>- // inline them.<br>- // FIME: Remove this once temp destructors are working.<br>- if ((*currBldrCtx->getBlock())[currStmtIdx].getAs<CFGTemporaryDtor>())<br>- return false;<br>-<br> // The auto-synthesized bodies are essential to inline as they are<br> // usually small and commonly used. Note: we should do this check early on to<br> // ensure we always inline these calls.<br><br>Modified: cfe/trunk/test/Analysis/analyzer-config.c<br>URL:<span class="Apple-converted-space"> </span><a href="http://llvm.org/viewvc/llvm-project/cfe/trunk/test/Analysis/analyzer-config.c?rev=186925&r1=186924&r2=186925&view=diff" target="_blank">http://llvm.org/viewvc/llvm-project/cfe/trunk/test/Analysis/analyzer-config.c?rev=186925&r1=186924&r2=186925&view=diff</a><br>==============================================================================<br>--- cfe/trunk/test/Analysis/analyzer-config.c (original)<br>+++ cfe/trunk/test/Analysis/analyzer-config.c Mon Jul 22 21:15:11 2013<br>@@ -6,7 +6,7 @@ void foo() { bar(); }<br><br>// CHECK: [config]<br>// CHECK-NEXT: cfg-conditional-static-initializers = true<br>-// CHECK-NEXT: cfg-temporary-dtors = true<br>+// CHECK-NEXT: cfg-temporary-dtors = false<br>// CHECK-NEXT: faux-bodies = true<br>// CHECK-NEXT: graph-trim-interval = 1000<br>// CHECK-NEXT: ipa = dynamic-bifurcate<br><br>Modified: cfe/trunk/test/Analysis/analyzer-config.cpp<br>URL:<span class="Apple-converted-space"> </span><a href="http://llvm.org/viewvc/llvm-project/cfe/trunk/test/Analysis/analyzer-config.cpp?rev=186925&r1=186924&r2=186925&view=diff" target="_blank">http://llvm.org/viewvc/llvm-project/cfe/trunk/test/Analysis/analyzer-config.cpp?rev=186925&r1=186924&r2=186925&view=diff</a><br>==============================================================================<br>--- cfe/trunk/test/Analysis/analyzer-config.cpp (original)<br>+++ cfe/trunk/test/Analysis/analyzer-config.cpp Mon Jul 22 21:15:11 2013<br>@@ -17,7 +17,7 @@ public:<br>// CHECK-NEXT: c++-stdlib-inlining = true<br>// CHECK-NEXT: c++-template-inlining = true<br>// CHECK-NEXT: cfg-conditional-static-initializers = true<br>-// CHECK-NEXT: cfg-temporary-dtors = true<br>+// CHECK-NEXT: cfg-temporary-dtors = false<br>// CHECK-NEXT: faux-bodies = true<br>// CHECK-NEXT: graph-trim-interval = 1000<br>// CHECK-NEXT: ipa = dynamic-bifurcate<br><br>Modified: cfe/trunk/test/Analysis/dtor.cpp<br>URL:<span class="Apple-converted-space"> </span><a href="http://llvm.org/viewvc/llvm-project/cfe/trunk/test/Analysis/dtor.cpp?rev=186925&r1=186924&r2=186925&view=diff" target="_blank">http://llvm.org/viewvc/llvm-project/cfe/trunk/test/Analysis/dtor.cpp?rev=186925&r1=186924&r2=186925&view=diff</a><br>==============================================================================<br>--- cfe/trunk/test/Analysis/dtor.cpp (original)<br>+++ cfe/trunk/test/Analysis/dtor.cpp Mon Jul 22 21:15:11 2013<br>@@ -416,19 +416,4 @@ namespace NoReturn {<br> f(&x);<br> *x = 47; // no warning<br> }<br>-<br>- void g2(int *x) {<br>- if (! x) NR();<br>- *x = 47; // no warning<br>- }<br>-<br>- void f3(int **x) {<br>- NR();<br>- }<br>-<br>- void g3() {<br>- int *x;<br>- f3(&x);<br>- *x = 47; // no warning<br>- }<br>}<br><br>Modified: cfe/trunk/test/Analysis/temp-obj-dtors-cfg-output.cpp<br>URL:<span class="Apple-converted-space"> </span><a href="http://llvm.org/viewvc/llvm-project/cfe/trunk/test/Analysis/temp-obj-dtors-cfg-output.cpp?rev=186925&r1=186924&r2=186925&view=diff" target="_blank">http://llvm.org/viewvc/llvm-project/cfe/trunk/test/Analysis/temp-obj-dtors-cfg-output.cpp?rev=186925&r1=186924&r2=186925&view=diff</a><br>==============================================================================<br>--- cfe/trunk/test/Analysis/temp-obj-dtors-cfg-output.cpp (original)<br>+++ cfe/trunk/test/Analysis/temp-obj-dtors-cfg-output.cpp Mon Jul 22 21:15:11 2013<br>@@ -108,24 +108,6 @@ TestCtorInits::TestCtorInits()<br> : a(int(A()) + int(B()))<br> , b() {}<br><br>-class NoReturn {<br>-public:<br>- ~NoReturn() __attribute__((noreturn));<br>- void f();<br>-};<br>-<br>-void test_noreturn1() {<br>- int a;<br>- NoReturn().f();<br>- int b;<br>-}<br>-<br>-void test_noreturn2() {<br>- int a;<br>- NoReturn(), 47;<br>- int b;<br>-}<br>-<br>// CHECK: [B1 (ENTRY)]<br>// CHECK: Succs (1): B0<br>// CHECK: [B0 (EXIT)]<br>@@ -864,36 +846,3 @@ void test_noreturn2() {<br>// CHECK: [B0 (EXIT)]<br>// CHECK: Preds (1): B1<br><br>-// CHECK: [B3 (ENTRY)]<br>-// CHECK: Succs (1): B2<br>-// CHECK: [B1]<br>-// CHECK: 1: int b;<br>-// CHECK: Succs (1): B0<br>-// CHECK: [B2]<br>-// CHECK: 1: int a;<br>-// CHECK: 2: NoReturn() (CXXConstructExpr, class NoReturn)<br>-// CHECK: 3: [B2.2] (BindTemporary)<br>-// CHECK: 4: [B2.3].f<br>-// CHECK: 5: [B2.4]()<br>-// CHECK: 6: ~NoReturn() (Temporary object destructor)<br>-// CHECK: Preds (1): B3<br>-// CHECK: Succs (1): B0<br>-// CHECK: [B0 (EXIT)]<br>-// CHECK: Preds (2): B1 B2<br>-<br>-// CHECK: [B3 (ENTRY)]<br>-// CHECK: Succs (1): B2<br>-// CHECK: [B1]<br>-// CHECK: 1: int b;<br>-// CHECK: Succs (1): B0<br>-// CHECK: [B2]<br>-// CHECK: 1: int a;<br>-// CHECK: 2: NoReturn() (CXXConstructExpr, class NoReturn)<br>-// CHECK: 3: [B2.2] (BindTemporary)<br>-// CHECK: 4: 47<br>-// CHECK: 5: ... , [B2.4]<br>-// CHECK: 6: ~NoReturn() (Temporary object destructor)<br>-// CHECK: Preds (1): B3<br>-// CHECK: Succs (1): B0<br>-// CHECK: [B0 (EXIT)]<br>-// CHECK: Preds (2): B1 B2<br><br><br>_______________________________________________<br>cfe-commits mailing list<br><a href="mailto:cfe-commits@cs.uiuc.edu" target="_blank">cfe-commits@cs.uiuc.edu</a><br><a href="http://lists.cs.uiuc.edu/mailman/listinfo/cfe-commits" target="_blank">http://lists.cs.uiuc.edu/mailman/listinfo/cfe-commits</a><br></blockquote></div><br></div></div></div><br>_______________________________________________<br>cfe-commits mailing list<br><a href="mailto:cfe-commits@cs.uiuc.edu">cfe-commits@cs.uiuc.edu</a><br><a href="http://lists.cs.uiuc.edu/mailman/listinfo/cfe-commits" target="_blank">http://lists.cs.uiuc.edu/mailman/listinfo/cfe-commits</a></blockquote></div></div></div></blockquote></div><br></body></html>