Index: include/clang/StaticAnalyzer/Core/PathSensitive/ExprEngine.h =================================================================== --- include/clang/StaticAnalyzer/Core/PathSensitive/ExprEngine.h +++ include/clang/StaticAnalyzer/Core/PathSensitive/ExprEngine.h @@ -780,9 +780,30 @@ llvm::PointerUnion P, const LocationContext *LC); + /// If the given expression corresponds to a temporary that was used for + /// passing into an elidable copy/move constructor and that constructor + /// was actually elided, track that we also need to elide the destructor. + static ProgramStateRef elideDestructor(ProgramStateRef State, + const CXXBindTemporaryExpr *BTE, + const LocationContext *LC); + + /// Stop tracking the destructor that corresponds to an elided constructor. + static ProgramStateRef + cleanupElidedDestructor(ProgramStateRef State, + const CXXBindTemporaryExpr *BTE, + const LocationContext *LC); + + /// Returns true if the given expression corresponds to a temporary that + /// was constructed for passing into an elidable copy/move constructor + /// and that constructor was actually elided. + static bool isDestructorElided(ProgramStateRef State, + const CXXBindTemporaryExpr *BTE, + const LocationContext *LC); + /// Check if all objects under construction have been fully constructed /// for the given context range (including FromLC, not including ToLC). - /// This is useful for assertions. + /// This is useful for assertions. Also checks if elided destructors + /// were cleaned up. static bool areAllObjectsFullyConstructed(ProgramStateRef State, const LocationContext *FromLC, const LocationContext *ToLC); Index: lib/StaticAnalyzer/Core/ExprEngine.cpp =================================================================== --- lib/StaticAnalyzer/Core/ExprEngine.cpp +++ lib/StaticAnalyzer/Core/ExprEngine.cpp @@ -139,7 +139,8 @@ // encountered. This list may be expanded when new actions are implemented. assert(getCXXCtorInitializer() || isa(getStmt()) || isa(getStmt()) || isa(getStmt()) || - isa(getStmt())); + isa(getStmt()) || + isa(getStmt())); } const Stmt *getStmt() const { @@ -183,6 +184,14 @@ REGISTER_TRAIT_WITH_PROGRAMSTATE(ObjectsUnderConstruction, ObjectsUnderConstructionMap) +// Additionally, track a set of destructors that correspond to elided +// constructors when copy elision occurs. +typedef std::pair + ElidedDestructorItem; +typedef llvm::ImmutableSet + ElidedDestructorSet; +REGISTER_TRAIT_WITH_PROGRAMSTATE(ElidedDestructors, + ElidedDestructorSet); //===----------------------------------------------------------------------===// // Engine construction and deletion. @@ -358,14 +367,12 @@ // a new temporary region out of thin air and copy the contents of the object // (which are currently present in the Environment, because Init is an rvalue) // into that region. This is not correct, but it is better than nothing. - bool FoundOriginalMaterializationRegion = false; const TypedValueRegion *TR = nullptr; if (const auto *MT = dyn_cast(Result)) { if (Optional V = getObjectUnderConstruction(State, MT, LC)) { - FoundOriginalMaterializationRegion = true; - TR = cast(V->getAsRegion()); - assert(TR); State = finishObjectConstruction(State, MT, LC); + State = State->BindExpr(Result, LC, *V); + return State; } else { StorageDuration SD = MT->getStorageDuration(); // If this object is bound to a reference with static storage duration, we @@ -402,35 +409,33 @@ } } - if (!FoundOriginalMaterializationRegion) { - // What remains is to copy the value of the object to the new region. - // FIXME: In other words, what we should always do is copy value of the - // Init expression (which corresponds to the bigger object) to the whole - // temporary region TR. However, this value is often no longer present - // in the Environment. If it has disappeared, we instead invalidate TR. - // Still, what we can do is assign the value of expression Ex (which - // corresponds to the sub-object) to the TR's sub-region Reg. At least, - // values inside Reg would be correct. - SVal InitVal = State->getSVal(Init, LC); - if (InitVal.isUnknown()) { - InitVal = getSValBuilder().conjureSymbolVal(Result, LC, Init->getType(), - currBldrCtx->blockCount()); - State = State->bindLoc(BaseReg.castAs(), InitVal, LC, false); - - // Then we'd need to take the value that certainly exists and bind it - // over. - if (InitValWithAdjustments.isUnknown()) { - // Try to recover some path sensitivity in case we couldn't - // compute the value. - InitValWithAdjustments = getSValBuilder().conjureSymbolVal( - Result, LC, InitWithAdjustments->getType(), - currBldrCtx->blockCount()); - } - State = - State->bindLoc(Reg.castAs(), InitValWithAdjustments, LC, false); - } else { - State = State->bindLoc(BaseReg.castAs(), InitVal, LC, false); + // What remains is to copy the value of the object to the new region. + // FIXME: In other words, what we should always do is copy value of the + // Init expression (which corresponds to the bigger object) to the whole + // temporary region TR. However, this value is often no longer present + // in the Environment. If it has disappeared, we instead invalidate TR. + // Still, what we can do is assign the value of expression Ex (which + // corresponds to the sub-object) to the TR's sub-region Reg. At least, + // values inside Reg would be correct. + SVal InitVal = State->getSVal(Init, LC); + if (InitVal.isUnknown()) { + InitVal = getSValBuilder().conjureSymbolVal(Result, LC, Init->getType(), + currBldrCtx->blockCount()); + State = State->bindLoc(BaseReg.castAs(), InitVal, LC, false); + + // Then we'd need to take the value that certainly exists and bind it + // over. + if (InitValWithAdjustments.isUnknown()) { + // Try to recover some path sensitivity in case we couldn't + // compute the value. + InitValWithAdjustments = getSValBuilder().conjureSymbolVal( + Result, LC, InitWithAdjustments->getType(), + currBldrCtx->blockCount()); } + State = + State->bindLoc(Reg.castAs(), InitValWithAdjustments, LC, false); + } else { + State = State->bindLoc(BaseReg.castAs(), InitVal, LC, false); } // The result expression would now point to the correct sub-region of the @@ -438,10 +443,8 @@ // correctly in case (Result == Init). State = State->BindExpr(Result, LC, Reg); - if (!FoundOriginalMaterializationRegion) { - // Notify checkers once for two bindLoc()s. - State = processRegionChange(State, TR, LC); - } + // Notify checkers once for two bindLoc()s. + State = processRegionChange(State, TR, LC); return State; } @@ -475,6 +478,30 @@ return State->remove(Key); } +ProgramStateRef ExprEngine::elideDestructor(ProgramStateRef State, + const CXXBindTemporaryExpr *BTE, + const LocationContext *LC) { + ElidedDestructorItem I(BTE, LC); + assert(!State->contains(I)); + return State->add(I); +} + +ProgramStateRef +ExprEngine::cleanupElidedDestructor(ProgramStateRef State, + const CXXBindTemporaryExpr *BTE, + const LocationContext *LC) { + ElidedDestructorItem I(BTE, LC); + assert(State->contains(I)); + return State->remove(I); +} + +bool ExprEngine::isDestructorElided(ProgramStateRef State, + const CXXBindTemporaryExpr *BTE, + const LocationContext *LC) { + ElidedDestructorItem I(BTE, LC); + return State->contains(I); +} + bool ExprEngine::areAllObjectsFullyConstructed(ProgramStateRef State, const LocationContext *FromLC, const LocationContext *ToLC) { @@ -485,6 +512,10 @@ if (I.first.getLocationContext() == LC) return false; + for (auto I: State->get()) + if (I.second == LC) + return false; + LC = LC->getParent(); } return true; @@ -529,6 +560,14 @@ Key.print(Out, nullptr, PP); Out << " : " << Value << NL; } + + for (auto I : State->get()) { + if (I.second != LC) + continue; + Out << '(' << I.second << ',' << (const void *)I.first << ") "; + I.first->printPretty(Out, nullptr, PP); + Out << " : (constructor elided)" << NL; + } } void ExprEngine::printState(raw_ostream &Out, ProgramStateRef State, @@ -1003,10 +1042,11 @@ void ExprEngine::ProcessTemporaryDtor(const CFGTemporaryDtor D, ExplodedNode *Pred, ExplodedNodeSet &Dst) { - ExplodedNodeSet CleanDtorState; - StmtNodeBuilder StmtBldr(Pred, CleanDtorState, *currBldrCtx); + const CXXBindTemporaryExpr *BTE = D.getBindTemporaryExpr(); ProgramStateRef State = Pred->getState(); + const LocationContext *LC = Pred->getLocationContext(); const MemRegion *MR = nullptr; + if (Optional V = getObjectUnderConstruction(State, D.getBindTemporaryExpr(), Pred->getLocationContext())) { @@ -1017,6 +1057,21 @@ Pred->getLocationContext()); MR = V->getAsRegion(); } + + // If copy elision has occured, and the constructor corresponding to the + // destructor was elided, we need to skip the destructor as well. + if (isDestructorElided(State, BTE, LC)) { + State = cleanupElidedDestructor(State, BTE, LC); + NodeBuilder Bldr(Pred, Dst, *currBldrCtx); + PostImplicitCall PP(D.getDestructorDecl(getContext()), + D.getBindTemporaryExpr()->getLocStart(), + Pred->getLocationContext()); + Bldr.generateNode(PP, State, Pred); + return; + } + + ExplodedNodeSet CleanDtorState; + StmtNodeBuilder StmtBldr(Pred, CleanDtorState, *currBldrCtx); StmtBldr.generateNode(D.getBindTemporaryExpr(), Pred, State); QualType T = D.getBindTemporaryExpr()->getSubExpr()->getType(); @@ -2201,8 +2256,21 @@ // it must be a separate problem. assert(isa(I.first.getStmt())); State = State->remove(I.first); + // Also cleanup the elided destructor info. + ElidedDestructorItem Item( + cast(I.first.getStmt()), + I.first.getLocationContext()); + State = State->remove(Item); } + // Also suppress the assertion for elided destructors when temporary + // destructors are not provided at all by the CFG, because there's no + // good place to clean them up. + if (!AMgr.getAnalyzerOptions().includeTemporaryDtorsInCFG()) + for (auto I : State->get()) + if (I.second == LC) + State = State->remove(I); + LC = LC->getParent(); } if (State != Pred->getState()) { Index: lib/StaticAnalyzer/Core/ExprEngineCXX.cpp =================================================================== --- lib/StaticAnalyzer/Core/ExprEngineCXX.cpp +++ lib/StaticAnalyzer/Core/ExprEngineCXX.cpp @@ -209,12 +209,37 @@ } llvm_unreachable("Unhandled return value construction context!"); } - case ConstructionContext::ElidedTemporaryObjectKind: + case ConstructionContext::ElidedTemporaryObjectKind: { assert(AMgr.getAnalyzerOptions().shouldElideConstructors()); - // FALL-THROUGH + const auto *TCC = cast(CC); + const CXXBindTemporaryExpr *BTE = TCC->getCXXBindTemporaryExpr(); + const MaterializeTemporaryExpr *MTE = TCC->getMaterializedTemporaryExpr(); + const CXXConstructExpr *CE = TCC->getConstructorAfterElision(); + + // Support pre-C++17 copy elision. We'll have the elidable copy + // constructor in the AST and in the CFG, but we'll skip it + // and construct directly into the final object. This call + // also sets the CallOpts flags for us. + SVal V; + std::tie(State, V) = prepareForObjectConstruction( + CE, State, LCtx, TCC->getConstructionContextAfterElision(), CallOpts); + + // Remember that we've elided the constructor. + State = addObjectUnderConstruction(State, CE, LCtx, V); + + // Remember that we've elided the destructor. + if (BTE) + State = elideDestructor(State, BTE, LCtx); + + // Instead of materialization, shamelessly return + // the final object destination. + if (MTE) + State = addObjectUnderConstruction(State, MTE, LCtx, V); + + return std::make_pair(State, V); + } case ConstructionContext::SimpleTemporaryObjectKind: { - // TODO: Copy elision implementation goes here. - const auto *TCC = cast(CC); + const auto *TCC = cast(CC); const CXXBindTemporaryExpr *BTE = TCC->getCXXBindTemporaryExpr(); const MaterializeTemporaryExpr *MTE = TCC->getMaterializedTemporaryExpr(); SVal V = UnknownVal(); @@ -266,6 +291,20 @@ SVal Target = UnknownVal(); + if (Optional ElidedTarget = + getObjectUnderConstruction(State, CE, LCtx)) { + // We've previously modeled an elidable constructor by pretending that it in + // fact constructs into the correct target. This constructor can therefore + // be skipped. + Target = *ElidedTarget; + StmtNodeBuilder Bldr(Pred, destNodes, *currBldrCtx); + State = finishObjectConstruction(State, CE, LCtx); + if (auto L = Target.getAs()) + State = State->BindExpr(CE, LCtx, State->getSVal(*L, CE->getType())); + Bldr.generateNode(CE, Pred, State); + return; + } + // FIXME: Handle arrays, which run the same constructor for every element. // For now, we just run the first constructor (which should still invalidate // the entire array). Index: test/Analysis/cxx17-mandatory-elision.cpp =================================================================== --- test/Analysis/cxx17-mandatory-elision.cpp +++ test/Analysis/cxx17-mandatory-elision.cpp @@ -1,5 +1,17 @@ // RUN: %clang_analyze_cc1 -analyzer-checker=core,debug.ExprInspection -std=c++11 -verify %s // RUN: %clang_analyze_cc1 -analyzer-checker=core,debug.ExprInspection -std=c++17 -verify %s +// RUN: %clang_analyze_cc1 -analyzer-checker=core,debug.ExprInspection -std=c++11 -analyzer-config elide-constructors=false -DNO_ELIDE_FLAG -verify %s +// RUN: %clang_analyze_cc1 -analyzer-checker=core,debug.ExprInspection -std=c++17 -analyzer-config elide-constructors=false -DNO_ELIDE_FLAG -verify %s + +// Copy elision always occurs in C++17, otherwise it's under +// an on-by-default flag. +#if __cplusplus >= 201703L + #define ELIDE 1 +#else + #ifndef NO_ELIDE_FLAG + #define ELIDE 1 + #endif +#endif void clang_analyzer_eval(bool); @@ -39,9 +51,9 @@ C() : t(T(4)) { S s = {1, 2, 3}; t.s = s; - // FIXME: Should be TRUE in C++11 as well. + // FIXME: Should be TRUE regardless of copy elision. clang_analyzer_eval(t.w == 4); -#if __cplusplus >= 201703L +#ifdef ELIDE // expected-warning@-2{{TRUE}} #else // expected-warning@-4{{UNKNOWN}} @@ -149,7 +161,7 @@ AddressVector v; ClassWithoutDestructor c = make3(v); -#if __cplusplus >= 201703L +#if ELIDE clang_analyzer_eval(v.len == 1); // expected-warning{{TRUE}} clang_analyzer_eval(v.buf[0] == &c); // expected-warning{{TRUE}} #else @@ -184,13 +196,13 @@ ClassWithDestructor c = ClassWithDestructor(v); // Check if the last destructor is an automatic destructor. // A temporary destructor would have fired by now. -#if __cplusplus >= 201703L +#if ELIDE clang_analyzer_eval(v.len == 1); // expected-warning{{TRUE}} #else clang_analyzer_eval(v.len == 3); // expected-warning{{TRUE}} #endif } -#if __cplusplus >= 201703L +#if ELIDE // 0. Construct the variable. // 1. Destroy the variable. clang_analyzer_eval(v.len == 2); // expected-warning{{TRUE}} @@ -218,13 +230,13 @@ TestCtorInitializer t(v); // Check if the last destructor is an automatic destructor. // A temporary destructor would have fired by now. -#if __cplusplus >= 201703L +#if ELIDE clang_analyzer_eval(v.len == 1); // expected-warning{{TRUE}} #else clang_analyzer_eval(v.len == 3); // expected-warning{{TRUE}} #endif } -#if __cplusplus >= 201703L +#if ELIDE // 0. Construct the member variable. // 1. Destroy the member variable. clang_analyzer_eval(v.len == 2); // expected-warning{{TRUE}} @@ -257,14 +269,14 @@ ClassWithDestructor c = make3(v); // Check if the last destructor is an automatic destructor. // A temporary destructor would have fired by now. -#if __cplusplus >= 201703L +#if ELIDE clang_analyzer_eval(v.len == 1); // expected-warning{{TRUE}} #else clang_analyzer_eval(v.len == 9); // expected-warning{{TRUE}} #endif } -#if __cplusplus >= 201703L +#if ELIDE // 0. Construct the variable. Yes, constructor in make1() constructs // the variable 'c'. // 1. Destroy the variable. Index: test/Analysis/gtest.cpp =================================================================== --- test/Analysis/gtest.cpp +++ test/Analysis/gtest.cpp @@ -154,13 +154,11 @@ void testAssertSymbolicPtr(const bool *b) { ASSERT_TRUE(*b); // no-crash - // FIXME: Our solver doesn't handle this well yet. - clang_analyzer_eval(*b); // expected-warning{{UNKNOWN}} + clang_analyzer_eval(*b); // expected-warning{{TRUE}} } void testAssertSymbolicRef(const bool &b) { ASSERT_TRUE(b); // no-crash - // FIXME: Our solver doesn't handle this well yet. - clang_analyzer_eval(b); // expected-warning{{UNKNOWN}} + clang_analyzer_eval(b); // expected-warning{{TRUE}} } Index: test/Analysis/inlining/temp-dtors-path-notes.cpp =================================================================== --- test/Analysis/inlining/temp-dtors-path-notes.cpp +++ test/Analysis/inlining/temp-dtors-path-notes.cpp @@ -30,19 +30,14 @@ }; void test(int coin) { - // We'd divide by zero in the temporary destructor that goes after the - // elidable copy-constructor from C(0) to the lifetime-extended temporary. - // So in fact this example has nothing to do with lifetime extension. - // Actually, it would probably be better to elide the constructor, and - // put the note for the destructor call at the closing brace after nop. + // We'd divide by zero in the automatic destructor for variable 'c'. const C &c = coin ? C(1) : C(0); // expected-note {{Assuming 'coin' is 0}} // expected-note@-1{{'?' condition is false}} // expected-note@-2{{Passing the value 0 via 1st parameter 'x'}} // expected-note@-3{{Calling constructor for 'C'}} // expected-note@-4{{Returning from constructor for 'C'}} - // expected-note@-5{{Calling '~C'}} c.nop(); -} +} // expected-note{{Calling '~C'}} } // end namespace test_lifetime_extended_temporary namespace test_bug_after_dtor { Index: test/Analysis/lifetime-extension.cpp =================================================================== --- test/Analysis/lifetime-extension.cpp +++ test/Analysis/lifetime-extension.cpp @@ -97,13 +97,10 @@ void f2() { C *after, *before; - C c = C(1, &after, &before); - clang_analyzer_eval(after == before); -#ifdef TEMPORARIES - // expected-warning@-2{{TRUE}} -#else - // expected-warning@-4{{UNKNOWN}} -#endif + { + C c = C(1, &after, &before); + } + clang_analyzer_eval(after == before); // expected-warning{{TRUE}} } void f3(bool coin) { @@ -129,6 +126,7 @@ // operator. Ideally also add support for the binary conditional operator in // C++. Because for now it calls the constructor for the condition twice. if (coin) { + // FIXME: Should not warn. clang_analyzer_eval(after == before); #ifdef TEMPORARIES // expected-warning@-2{{The left operand of '==' is a garbage value}} @@ -136,13 +134,12 @@ // expected-warning@-4{{UNKNOWN}} #endif } else { + // FIXME: Should be TRUE. clang_analyzer_eval(after == before); #ifdef TEMPORARIES - // Seems to work at the moment, but also seems accidental. - // Feel free to break. - // expected-warning@-4{{TRUE}} + // expected-warning@-2{{FALSE}} #else - // expected-warning@-6{{UNKNOWN}} + // expected-warning@-4{{UNKNOWN}} #endif } } @@ -234,24 +231,25 @@ } // end namespace maintain_original_object_address_on_move namespace maintain_address_of_copies { +class C; -template struct AddressVector { - const T *buf[10]; +struct AddressVector { + C *buf[10]; int len; AddressVector() : len(0) {} - void push(const T *t) { - buf[len] = t; + void push(C *c) { + buf[len] = c; ++len; } }; class C { - AddressVector &v; + AddressVector &v; public: - C(AddressVector &v) : v(v) { v.push(this); } + C(AddressVector &v) : v(v) { v.push(this); } ~C() { v.push(this); } #ifdef MOVES @@ -267,132 +265,70 @@ #endif } // no-warning - static C make(AddressVector &v) { return C(v); } + static C make(AddressVector &v) { return C(v); } }; void f1() { - AddressVector v; + AddressVector v; { C c = C(v); } - // 0. Create the original temporary and lifetime-extend it into variable 'c' - // construction argument. - // 1. Construct variable 'c' (elidable copy/move). - // 2. Destroy the temporary. - // 3. Destroy variable 'c'. - clang_analyzer_eval(v.len == 4); - clang_analyzer_eval(v.buf[0] == v.buf[2]); - clang_analyzer_eval(v.buf[1] == v.buf[3]); -#ifdef TEMPORARIES - // expected-warning@-4{{TRUE}} - // expected-warning@-4{{TRUE}} - // expected-warning@-4{{TRUE}} -#else - // expected-warning@-8{{UNKNOWN}} - // expected-warning@-8{{UNKNOWN}} - // expected-warning@-8{{UNKNOWN}} -#endif + // 0. Construct variable 'c' (copy/move elided). + // 1. Destroy variable 'c'. + clang_analyzer_eval(v.len == 2); // expected-warning{{TRUE}} + clang_analyzer_eval(v.buf[0] == v.buf[1]); // expected-warning{{TRUE}} } void f2() { - AddressVector v; + AddressVector v; { const C &c = C::make(v); } - // 0. Construct the original temporary within make(), - // 1. Construct the return value of make() (elidable copy/move) and - // lifetime-extend it via reference 'c', - // 2. Destroy the temporary within make(), - // 3. Destroy the temporary lifetime-extended by 'c'. - clang_analyzer_eval(v.len == 4); - clang_analyzer_eval(v.buf[0] == v.buf[2]); - clang_analyzer_eval(v.buf[1] == v.buf[3]); + // 0. Construct the return value of make() (copy/move elided) and + // lifetime-extend it directly via reference 'c', + // 1. Destroy the temporary lifetime-extended by 'c'. + clang_analyzer_eval(v.len == 2); + clang_analyzer_eval(v.buf[0] == v.buf[1]); #ifdef TEMPORARIES - // expected-warning@-4{{TRUE}} - // expected-warning@-4{{TRUE}} - // expected-warning@-4{{TRUE}} + // expected-warning@-3{{TRUE}} + // expected-warning@-3{{TRUE}} #else - // expected-warning@-8{{UNKNOWN}} - // expected-warning@-8{{UNKNOWN}} - // expected-warning@-8{{UNKNOWN}} + // expected-warning@-6{{UNKNOWN}} + // expected-warning@-6{{UNKNOWN}} #endif } void f3() { - AddressVector v; + AddressVector v; { C &&c = C::make(v); } - // 0. Construct the original temporary within make(), - // 1. Construct the return value of make() (elidable copy/move) and - // lifetime-extend it via reference 'c', - // 2. Destroy the temporary within make(), - // 3. Destroy the temporary lifetime-extended by 'c'. - clang_analyzer_eval(v.len == 4); - clang_analyzer_eval(v.buf[0] == v.buf[2]); - clang_analyzer_eval(v.buf[1] == v.buf[3]); + // 0. Construct the return value of make() (copy/move elided) and + // lifetime-extend it directly via reference 'c', + // 1. Destroy the temporary lifetime-extended by 'c'. + clang_analyzer_eval(v.len == 2); + clang_analyzer_eval(v.buf[0] == v.buf[1]); #ifdef TEMPORARIES - // expected-warning@-4{{TRUE}} - // expected-warning@-4{{TRUE}} - // expected-warning@-4{{TRUE}} + // expected-warning@-3{{TRUE}} + // expected-warning@-3{{TRUE}} #else - // expected-warning@-8{{UNKNOWN}} - // expected-warning@-8{{UNKNOWN}} - // expected-warning@-8{{UNKNOWN}} + // expected-warning@-6{{UNKNOWN}} + // expected-warning@-6{{UNKNOWN}} #endif } -C doubleMake(AddressVector &v) { +C doubleMake(AddressVector &v) { return C::make(v); } void f4() { - AddressVector v; + AddressVector v; { C c = doubleMake(v); } - // 0. Construct the original temporary within make(), - // 1. Construct the return value of make() (elidable copy/move) and - // lifetime-extend it into the return value constructor argument within - // doubleMake(), - // 2. Destroy the temporary within make(), - // 3. Construct the return value of doubleMake() (elidable copy/move) and - // lifetime-extend it into the variable 'c' constructor argument, - // 4. Destroy the return value of make(), - // 5. Construct variable 'c' (elidable copy/move), - // 6. Destroy the return value of doubleMake(), - // 7. Destroy variable 'c'. - clang_analyzer_eval(v.len == 8); - clang_analyzer_eval(v.buf[0] == v.buf[2]); - clang_analyzer_eval(v.buf[1] == v.buf[4]); - clang_analyzer_eval(v.buf[3] == v.buf[6]); - clang_analyzer_eval(v.buf[5] == v.buf[7]); -#ifdef TEMPORARIES - // expected-warning@-6{{TRUE}} - // expected-warning@-6{{TRUE}} - // expected-warning@-6{{TRUE}} - // expected-warning@-6{{TRUE}} - // expected-warning@-6{{TRUE}} -#else - // expected-warning@-12{{UNKNOWN}} - // expected-warning@-12{{UNKNOWN}} - // expected-warning@-12{{UNKNOWN}} - // expected-warning@-12{{UNKNOWN}} - // expected-warning@-12{{UNKNOWN}} -#endif + // 0. Construct variable 'c' (all copies/moves elided), + // 1. Destroy variable 'c'. + clang_analyzer_eval(v.len == 2); // expected-warning{{TRUE}} + clang_analyzer_eval(v.buf[0] == v.buf[1]); // expected-warning{{TRUE}} } - -class NoDtor { - AddressVector &v; - -public: - NoDtor(AddressVector &v) : v(v) { v.push(this); } -}; - -void f5() { - AddressVector v; - const NoDtor &N = NoDtor(v); - clang_analyzer_eval(v.buf[0] == &N); // expected-warning{{TRUE}} -} - } // end namespace maintain_address_of_copies Index: test/Analysis/temporaries.cpp =================================================================== --- test/Analysis/temporaries.cpp +++ test/Analysis/temporaries.cpp @@ -612,107 +612,44 @@ clang_analyzer_eval(c3.getY() == 2); // expected-warning{{TRUE}} C c4 = returnTemporaryWithConstruction(); - clang_analyzer_eval(c4.getX() == 1); - clang_analyzer_eval(c4.getY() == 2); -#ifdef TEMPORARY_DTORS - // expected-warning@-3{{TRUE}} - // expected-warning@-3{{TRUE}} -#else - // expected-warning@-6{{UNKNOWN}} - // expected-warning@-6{{UNKNOWN}} -#endif + clang_analyzer_eval(c4.getX() == 1); // expected-warning{{TRUE}} + clang_analyzer_eval(c4.getY() == 2); // expected-warning{{TRUE}} C c5 = returnTemporaryWithAnotherFunctionWithConstruction(); - clang_analyzer_eval(c5.getX() == 1); - clang_analyzer_eval(c5.getY() == 2); -#ifdef TEMPORARY_DTORS - // expected-warning@-3{{TRUE}} - // expected-warning@-3{{TRUE}} -#else - // expected-warning@-6{{UNKNOWN}} - // expected-warning@-6{{UNKNOWN}} -#endif + clang_analyzer_eval(c5.getX() == 1); // expected-warning{{TRUE}} + clang_analyzer_eval(c5.getY() == 2); // expected-warning{{TRUE}} C c6 = returnTemporaryWithCopyConstructionWithConstruction(); - clang_analyzer_eval(c5.getX() == 1); - clang_analyzer_eval(c5.getY() == 2); -#ifdef TEMPORARY_DTORS - // expected-warning@-3{{TRUE}} - // expected-warning@-3{{TRUE}} -#else - // expected-warning@-6{{UNKNOWN}} - // expected-warning@-6{{UNKNOWN}} -#endif + clang_analyzer_eval(c5.getX() == 1); // expected-warning{{TRUE}} + clang_analyzer_eval(c5.getY() == 2); // expected-warning{{TRUE}} #if __cplusplus >= 201103L C c7 = returnTemporaryWithBraces(); - clang_analyzer_eval(c7.getX() == 1); - clang_analyzer_eval(c7.getY() == 2); -#ifdef TEMPORARY_DTORS - // expected-warning@-3{{TRUE}} - // expected-warning@-3{{TRUE}} -#else - // expected-warning@-6{{UNKNOWN}} - // expected-warning@-6{{UNKNOWN}} -#endif + clang_analyzer_eval(c7.getX() == 1); // expected-warning{{TRUE}} + clang_analyzer_eval(c7.getY() == 2); // expected-warning{{TRUE}} C c8 = returnTemporaryWithAnotherFunctionWithBraces(); - clang_analyzer_eval(c8.getX() == 1); - clang_analyzer_eval(c8.getY() == 2); -#ifdef TEMPORARY_DTORS - // expected-warning@-3{{TRUE}} - // expected-warning@-3{{TRUE}} -#else - // expected-warning@-6{{UNKNOWN}} - // expected-warning@-6{{UNKNOWN}} -#endif + clang_analyzer_eval(c8.getX() == 1); // expected-warning{{TRUE}} + clang_analyzer_eval(c8.getY() == 2); // expected-warning{{TRUE}} C c9 = returnTemporaryWithCopyConstructionWithBraces(); - clang_analyzer_eval(c9.getX() == 1); - clang_analyzer_eval(c9.getY() == 2); -#ifdef TEMPORARY_DTORS - // expected-warning@-3{{TRUE}} - // expected-warning@-3{{TRUE}} -#else - // expected-warning@-6{{UNKNOWN}} - // expected-warning@-6{{UNKNOWN}} -#endif + clang_analyzer_eval(c9.getX() == 1); // expected-warning{{TRUE}} + clang_analyzer_eval(c9.getY() == 2); // expected-warning{{TRUE}} #endif // C++11 D d1 = returnTemporaryWithVariableAndNonTrivialCopy(); - clang_analyzer_eval(d1.getX() == 1); - clang_analyzer_eval(d1.getY() == 2); -#ifdef TEMPORARY_DTORS - // expected-warning@-3{{TRUE}} - // expected-warning@-3{{TRUE}} -#else - // expected-warning@-6{{UNKNOWN}} - // expected-warning@-6{{UNKNOWN}} -#endif + clang_analyzer_eval(d1.getX() == 1); // expected-warning{{TRUE}} + clang_analyzer_eval(d1.getY() == 2); // expected-warning{{TRUE}} D d2 = returnTemporaryWithAnotherFunctionWithVariableAndNonTrivialCopy(); - clang_analyzer_eval(d2.getX() == 1); - clang_analyzer_eval(d2.getY() == 2); -#ifdef TEMPORARY_DTORS - // expected-warning@-3{{TRUE}} - // expected-warning@-3{{TRUE}} -#else - // expected-warning@-6{{UNKNOWN}} - // expected-warning@-6{{UNKNOWN}} -#endif + clang_analyzer_eval(d2.getX() == 1); // expected-warning{{TRUE}} + clang_analyzer_eval(d2.getY() == 2); // expected-warning{{TRUE}} D d3 = returnTemporaryWithCopyConstructionWithVariableAndNonTrivialCopy(); - clang_analyzer_eval(d3.getX() == 1); - clang_analyzer_eval(d3.getY() == 2); -#ifdef TEMPORARY_DTORS - // expected-warning@-3{{TRUE}} - // expected-warning@-3{{TRUE}} -#else - // expected-warning@-6{{UNKNOWN}} - // expected-warning@-6{{UNKNOWN}} -#endif + clang_analyzer_eval(d3.getX() == 1); // expected-warning{{TRUE}} + clang_analyzer_eval(d3.getY() == 2); // expected-warning{{TRUE}} } } // namespace test_return_temporary @@ -767,19 +704,9 @@ C c2 = coin ? C(1) : C(2); if (coin) { - clang_analyzer_eval(c2.getX() == 1); -#ifdef TEMPORARY_DTORS - // expected-warning@-2{{TRUE}} -#else - // expected-warning@-4{{UNKNOWN}} -#endif + clang_analyzer_eval(c2.getX() == 1); // expected-warning{{TRUE}} } else { - clang_analyzer_eval(c2.getX() == 2); -#ifdef TEMPORARY_DTORS - // expected-warning@-2{{TRUE}} -#else - // expected-warning@-4{{UNKNOWN}} -#endif + clang_analyzer_eval(c2.getX() == 2); // expected-warning{{TRUE}} } } @@ -816,16 +743,9 @@ { C c = C(x, y); } - // Two constructors (temporary object expr and copy) and two destructors. - clang_analyzer_eval(x == 2); - clang_analyzer_eval(y == 2); -#ifdef TEMPORARY_DTORS - // expected-warning@-3{{TRUE}} - // expected-warning@-3{{TRUE}} -#else - // expected-warning@-6{{UNKNOWN}} - // expected-warning@-6{{UNKNOWN}} -#endif + // Only one constructor directly into the variable, and one destructor. + clang_analyzer_eval(x == 1); // expected-warning{{TRUE}} + clang_analyzer_eval(y == 1); // expected-warning{{TRUE}} } void test_ternary_temporary(int coin) { @@ -833,10 +753,10 @@ { const C &c = coin ? C(x, y) : C(z, w); } - // This time each branch contains an additional elidable copy constructor. + // Only one constructor on every branch, and one automatic destructor. if (coin) { - clang_analyzer_eval(x == 2); - clang_analyzer_eval(y == 2); + clang_analyzer_eval(x == 1); + clang_analyzer_eval(y == 1); #ifdef TEMPORARY_DTORS // expected-warning@-3{{TRUE}} // expected-warning@-3{{TRUE}} @@ -850,8 +770,8 @@ } else { clang_analyzer_eval(x == 0); // expected-warning{{TRUE}} clang_analyzer_eval(y == 0); // expected-warning{{TRUE}} - clang_analyzer_eval(z == 2); - clang_analyzer_eval(w == 2); + clang_analyzer_eval(z == 1); + clang_analyzer_eval(w == 1); #ifdef TEMPORARY_DTORS // expected-warning@-3{{TRUE}} // expected-warning@-3{{TRUE}} @@ -867,33 +787,18 @@ { C c = coin ? C(x, y) : C(z, w); } - // Temporary expression, elidable copy within branch, - // constructor for variable - 3 total. + // On each branch the variable is constructed directly. if (coin) { - clang_analyzer_eval(x == 3); - clang_analyzer_eval(y == 3); -#ifdef TEMPORARY_DTORS - // expected-warning@-3{{TRUE}} - // expected-warning@-3{{TRUE}} -#else - // expected-warning@-6{{UNKNOWN}} - // expected-warning@-6{{UNKNOWN}} -#endif + clang_analyzer_eval(x == 1); // expected-warning{{TRUE}} + clang_analyzer_eval(y == 1); // expected-warning{{TRUE}} clang_analyzer_eval(z == 0); // expected-warning{{TRUE}} clang_analyzer_eval(w == 0); // expected-warning{{TRUE}} } else { clang_analyzer_eval(x == 0); // expected-warning{{TRUE}} clang_analyzer_eval(y == 0); // expected-warning{{TRUE}} - clang_analyzer_eval(z == 3); - clang_analyzer_eval(w == 3); -#ifdef TEMPORARY_DTORS - // expected-warning@-3{{TRUE}} - // expected-warning@-3{{TRUE}} -#else - // expected-warning@-6{{UNKNOWN}} - // expected-warning@-6{{UNKNOWN}} -#endif + clang_analyzer_eval(z == 1); // expected-warning{{TRUE}} + clang_analyzer_eval(w == 1); // expected-warning{{TRUE}} } } } // namespace test_match_constructors_and_destructors @@ -928,12 +833,13 @@ } void testCopiedCall() { - C c = make(); - // Should have divided by zero in the temporary destructor. - clang_analyzer_warnIfReached(); -#ifndef TEMPORARY_DTORS - // expected-warning@-2{{REACHABLE}} -#endif + { + C c = make(); + // Should have elided the constructor/destructor for the temporary + clang_analyzer_warnIfReached(); // expected-warning{{REACHABLE}} + } + // Should have divided by zero in the destructor. + clang_analyzer_warnIfReached(); // no-warning } } // namespace destructors_for_return_values @@ -1018,20 +924,10 @@ #endif S s = 20; - clang_analyzer_eval(s.x == 20); -#ifdef TEMPORARY_DTORS - // expected-warning@-2{{TRUE}} -#else - // expected-warning@-4{{UNKNOWN}} -#endif + clang_analyzer_eval(s.x == 20); // expected-warning{{TRUE}} C c2 = s; - clang_analyzer_eval(c2.getX() == 20); -#ifdef TEMPORARY_DTORS - // expected-warning@-2{{TRUE}} -#else - // expected-warning@-4{{UNKNOWN}} -#endif + clang_analyzer_eval(c2.getX() == 20); // expected-warning{{TRUE}} } } // end namespace implicit_constructor_conversion