Oops, no. Thanks for keeping me honest. Will add soon. On Jul 24, 2013, at 8:33 , Manuel Klimek <[email protected]> wrote:
> Jordan, did you add a regression test for this? > > > On Tue, Jul 23, 2013 at 4:29 AM, Jordan Rose <[email protected]> wrote: > I did get around to reverting this, Pavel. Apart from PR16664, 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. > > 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! > > Jordan > > > On Jul 22, 2013, at 19:15 , Jordan Rose <[email protected]> wrote: > >> Author: jrose >> Date: Mon Jul 22 21:15:11 2013 >> New Revision: 186925 >> >> URL: http://llvm.org/viewvc/llvm-project?rev=186925&view=rev >> Log: >> Revert "[analyzer] Add very limited support for temporary destructors" >> >> The analyzer doesn't currently expect CFG blocks with terminators to be >> empty, but this can happen when generating conditional destructors for >> a complex logical expression, such as (a && (b || Temp{})). Moreover, >> the branch conditions for these expressions are not persisted in the >> state. Even for handling noreturn destructors this needs more work. >> >> This reverts r186498. >> >> Modified: >> cfe/trunk/lib/StaticAnalyzer/Core/AnalyzerOptions.cpp >> cfe/trunk/lib/StaticAnalyzer/Core/ExprEngine.cpp >> cfe/trunk/lib/StaticAnalyzer/Core/ExprEngineCallAndReturn.cpp >> cfe/trunk/test/Analysis/analyzer-config.c >> cfe/trunk/test/Analysis/analyzer-config.cpp >> cfe/trunk/test/Analysis/dtor.cpp >> cfe/trunk/test/Analysis/temp-obj-dtors-cfg-output.cpp >> >> Modified: cfe/trunk/lib/StaticAnalyzer/Core/AnalyzerOptions.cpp >> URL: >> http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/StaticAnalyzer/Core/AnalyzerOptions.cpp?rev=186925&r1=186924&r2=186925&view=diff >> ============================================================================== >> --- cfe/trunk/lib/StaticAnalyzer/Core/AnalyzerOptions.cpp (original) >> +++ cfe/trunk/lib/StaticAnalyzer/Core/AnalyzerOptions.cpp Mon Jul 22 >> 21:15:11 2013 >> @@ -119,7 +119,7 @@ bool AnalyzerOptions::getBooleanOption(O >> bool AnalyzerOptions::includeTemporaryDtorsInCFG() { >> return getBooleanOption(IncludeTemporaryDtorsInCFG, >> "cfg-temporary-dtors", >> - /* Default = */ true); >> + /* Default = */ false); >> } >> >> bool AnalyzerOptions::mayInlineCXXStandardLibrary() { >> >> Modified: cfe/trunk/lib/StaticAnalyzer/Core/ExprEngine.cpp >> URL: >> http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/StaticAnalyzer/Core/ExprEngine.cpp?rev=186925&r1=186924&r2=186925&view=diff >> ============================================================================== >> --- cfe/trunk/lib/StaticAnalyzer/Core/ExprEngine.cpp (original) >> +++ cfe/trunk/lib/StaticAnalyzer/Core/ExprEngine.cpp Mon Jul 22 21:15:11 2013 >> @@ -590,15 +590,7 @@ void ExprEngine::ProcessMemberDtor(const >> >> void ExprEngine::ProcessTemporaryDtor(const CFGTemporaryDtor D, >> ExplodedNode *Pred, >> - ExplodedNodeSet &Dst) { >> - >> - QualType varType = D.getBindTemporaryExpr()->getSubExpr()->getType(); >> - >> - // FIXME: Inlining of temporary destructors is not supported yet anyway, >> so we >> - // just put a NULL region for now. This will need to be changed later. >> - VisitCXXDestructor(varType, NULL, D.getBindTemporaryExpr(), >> - /*IsBase=*/ false, Pred, Dst); >> -} >> + ExplodedNodeSet &Dst) {} >> >> void ExprEngine::Visit(const Stmt *S, ExplodedNode *Pred, >> ExplodedNodeSet &DstTop) { >> >> Modified: cfe/trunk/lib/StaticAnalyzer/Core/ExprEngineCallAndReturn.cpp >> URL: >> http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/StaticAnalyzer/Core/ExprEngineCallAndReturn.cpp?rev=186925&r1=186924&r2=186925&view=diff >> ============================================================================== >> --- cfe/trunk/lib/StaticAnalyzer/Core/ExprEngineCallAndReturn.cpp (original) >> +++ cfe/trunk/lib/StaticAnalyzer/Core/ExprEngineCallAndReturn.cpp Mon Jul 22 >> 21:15:11 2013 >> @@ -807,12 +807,6 @@ bool ExprEngine::shouldInlineCall(const >> AnalysisDeclContextManager &ADCMgr = AMgr.getAnalysisDeclContextManager(); >> AnalysisDeclContext *CalleeADC = ADCMgr.getContext(D); >> >> - // Temporary object destructor processing is currently broken, so we never >> - // inline them. >> - // FIME: Remove this once temp destructors are working. >> - if ((*currBldrCtx->getBlock())[currStmtIdx].getAs<CFGTemporaryDtor>()) >> - return false; >> - >> // The auto-synthesized bodies are essential to inline as they are >> // usually small and commonly used. Note: we should do this check early on >> to >> // ensure we always inline these calls. >> >> Modified: cfe/trunk/test/Analysis/analyzer-config.c >> URL: >> http://llvm.org/viewvc/llvm-project/cfe/trunk/test/Analysis/analyzer-config.c?rev=186925&r1=186924&r2=186925&view=diff >> ============================================================================== >> --- cfe/trunk/test/Analysis/analyzer-config.c (original) >> +++ cfe/trunk/test/Analysis/analyzer-config.c Mon Jul 22 21:15:11 2013 >> @@ -6,7 +6,7 @@ void foo() { bar(); } >> >> // CHECK: [config] >> // CHECK-NEXT: cfg-conditional-static-initializers = true >> -// CHECK-NEXT: cfg-temporary-dtors = true >> +// CHECK-NEXT: cfg-temporary-dtors = false >> // CHECK-NEXT: faux-bodies = true >> // CHECK-NEXT: graph-trim-interval = 1000 >> // CHECK-NEXT: ipa = dynamic-bifurcate >> >> Modified: cfe/trunk/test/Analysis/analyzer-config.cpp >> URL: >> http://llvm.org/viewvc/llvm-project/cfe/trunk/test/Analysis/analyzer-config.cpp?rev=186925&r1=186924&r2=186925&view=diff >> ============================================================================== >> --- cfe/trunk/test/Analysis/analyzer-config.cpp (original) >> +++ cfe/trunk/test/Analysis/analyzer-config.cpp Mon Jul 22 21:15:11 2013 >> @@ -17,7 +17,7 @@ public: >> // CHECK-NEXT: c++-stdlib-inlining = true >> // CHECK-NEXT: c++-template-inlining = true >> // CHECK-NEXT: cfg-conditional-static-initializers = true >> -// CHECK-NEXT: cfg-temporary-dtors = true >> +// CHECK-NEXT: cfg-temporary-dtors = false >> // CHECK-NEXT: faux-bodies = true >> // CHECK-NEXT: graph-trim-interval = 1000 >> // CHECK-NEXT: ipa = dynamic-bifurcate >> >> Modified: cfe/trunk/test/Analysis/dtor.cpp >> URL: >> http://llvm.org/viewvc/llvm-project/cfe/trunk/test/Analysis/dtor.cpp?rev=186925&r1=186924&r2=186925&view=diff >> ============================================================================== >> --- cfe/trunk/test/Analysis/dtor.cpp (original) >> +++ cfe/trunk/test/Analysis/dtor.cpp Mon Jul 22 21:15:11 2013 >> @@ -416,19 +416,4 @@ namespace NoReturn { >> f(&x); >> *x = 47; // no warning >> } >> - >> - void g2(int *x) { >> - if (! x) NR(); >> - *x = 47; // no warning >> - } >> - >> - void f3(int **x) { >> - NR(); >> - } >> - >> - void g3() { >> - int *x; >> - f3(&x); >> - *x = 47; // no warning >> - } >> } >> >> Modified: cfe/trunk/test/Analysis/temp-obj-dtors-cfg-output.cpp >> URL: >> http://llvm.org/viewvc/llvm-project/cfe/trunk/test/Analysis/temp-obj-dtors-cfg-output.cpp?rev=186925&r1=186924&r2=186925&view=diff >> ============================================================================== >> --- cfe/trunk/test/Analysis/temp-obj-dtors-cfg-output.cpp (original) >> +++ cfe/trunk/test/Analysis/temp-obj-dtors-cfg-output.cpp Mon Jul 22 >> 21:15:11 2013 >> @@ -108,24 +108,6 @@ TestCtorInits::TestCtorInits() >> : a(int(A()) + int(B())) >> , b() {} >> >> -class NoReturn { >> -public: >> - ~NoReturn() __attribute__((noreturn)); >> - void f(); >> -}; >> - >> -void test_noreturn1() { >> - int a; >> - NoReturn().f(); >> - int b; >> -} >> - >> -void test_noreturn2() { >> - int a; >> - NoReturn(), 47; >> - int b; >> -} >> - >> // CHECK: [B1 (ENTRY)] >> // CHECK: Succs (1): B0 >> // CHECK: [B0 (EXIT)] >> @@ -864,36 +846,3 @@ void test_noreturn2() { >> // CHECK: [B0 (EXIT)] >> // CHECK: Preds (1): B1 >> >> -// CHECK: [B3 (ENTRY)] >> -// CHECK: Succs (1): B2 >> -// CHECK: [B1] >> -// CHECK: 1: int b; >> -// CHECK: Succs (1): B0 >> -// CHECK: [B2] >> -// CHECK: 1: int a; >> -// CHECK: 2: NoReturn() (CXXConstructExpr, class NoReturn) >> -// CHECK: 3: [B2.2] (BindTemporary) >> -// CHECK: 4: [B2.3].f >> -// CHECK: 5: [B2.4]() >> -// CHECK: 6: ~NoReturn() (Temporary object destructor) >> -// CHECK: Preds (1): B3 >> -// CHECK: Succs (1): B0 >> -// CHECK: [B0 (EXIT)] >> -// CHECK: Preds (2): B1 B2 >> - >> -// CHECK: [B3 (ENTRY)] >> -// CHECK: Succs (1): B2 >> -// CHECK: [B1] >> -// CHECK: 1: int b; >> -// CHECK: Succs (1): B0 >> -// CHECK: [B2] >> -// CHECK: 1: int a; >> -// CHECK: 2: NoReturn() (CXXConstructExpr, class NoReturn) >> -// CHECK: 3: [B2.2] (BindTemporary) >> -// CHECK: 4: 47 >> -// CHECK: 5: ... , [B2.4] >> -// CHECK: 6: ~NoReturn() (Temporary object destructor) >> -// CHECK: Preds (1): B3 >> -// CHECK: Succs (1): B0 >> -// CHECK: [B0 (EXIT)] >> -// CHECK: Preds (2): B1 B2 >> >> >> _______________________________________________ >> cfe-commits mailing list >> [email protected] >> http://lists.cs.uiuc.edu/mailman/listinfo/cfe-commits > > > _______________________________________________ > cfe-commits mailing list > [email protected] > http://lists.cs.uiuc.edu/mailman/listinfo/cfe-commits
_______________________________________________ cfe-commits mailing list [email protected] http://lists.cs.uiuc.edu/mailman/listinfo/cfe-commits
