From: Richard Smith Date: Thu, 12 Dec 2013 01:27:02 +0000 (+0000) Subject: PR18217: Rewrite JumpDiagnostics' handling of temporaries, to correctly handle X-Git-Url: https://granicus.if.org/sourcecode?a=commitdiff_plain;h=22ab81e8ac9402b830e2c123d8f9dddf4b1f2324;p=clang PR18217: Rewrite JumpDiagnostics' handling of temporaries, to correctly handle declarations that might lifetime-extend multiple temporaries. In passing, fix a crasher (PR18217) if an initializer was dependent and exactly the wrong shape, and remove a bogus function (Expr::findMaterializedTemporary) now its last use is gone. git-svn-id: https://llvm.org/svn/llvm-project/cfe/trunk@197103 91177308-0d34-0410-b5e6-96231b3b80d8 --- diff --git a/include/clang/AST/Expr.h b/include/clang/AST/Expr.h index b5dbc1b997..b59f961235 100644 --- a/include/clang/AST/Expr.h +++ b/include/clang/AST/Expr.h @@ -764,11 +764,6 @@ public: SmallVectorImpl &CommaLHS, SmallVectorImpl &Adjustments) const; - /// Skip irrelevant expressions to find what should be materialize for - /// binding with a reference. - const Expr * - findMaterializedTemporary(const MaterializeTemporaryExpr *&MTE) const; - static bool classof(const Stmt *T) { return T->getStmtClass() >= firstExprConstant && T->getStmtClass() <= lastExprConstant; diff --git a/include/clang/Basic/DiagnosticSemaKinds.td b/include/clang/Basic/DiagnosticSemaKinds.td index 46d152318b..4c710d53d5 100644 --- a/include/clang/Basic/DiagnosticSemaKinds.td +++ b/include/clang/Basic/DiagnosticSemaKinds.td @@ -3941,6 +3941,9 @@ def note_exits_cleanup : Note< "jump exits scope of variable with __attribute__((cleanup))">; def note_exits_dtor : Note< "jump exits scope of variable with non-trivial destructor">; +def note_exits_temporary_dtor : Note< + "jump exits scope of lifetime-extended temporary with non-trivial " + "destructor">; def note_exits___block : Note< "jump exits scope of __block variable">; def note_exits_objc_try : Note< diff --git a/lib/AST/Expr.cpp b/lib/AST/Expr.cpp index 4bc455837c..d9776a1422 100644 --- a/lib/AST/Expr.cpp +++ b/lib/AST/Expr.cpp @@ -105,37 +105,6 @@ const Expr *Expr::skipRValueSubobjectAdjustments( return E; } -const Expr * -Expr::findMaterializedTemporary(const MaterializeTemporaryExpr *&MTE) const { - const Expr *E = this; - - // This might be a default initializer for a reference member. Walk over the - // wrapper node for that. - if (const CXXDefaultInitExpr *DAE = dyn_cast(E)) - E = DAE->getExpr(); - - // Look through single-element init lists that claim to be lvalues. They're - // just syntactic wrappers in this case. - if (const InitListExpr *ILE = dyn_cast(E)) { - if (ILE->getNumInits() == 1 && ILE->isGLValue()) { - E = ILE->getInit(0); - if (const CXXDefaultInitExpr *DAE = dyn_cast(E)) - E = DAE->getExpr(); - } - } - - // Look through expressions for materialized temporaries (for now). - if (const MaterializeTemporaryExpr *M - = dyn_cast(E)) { - MTE = M; - E = M->GetTemporaryExpr(); - } - - if (const CXXDefaultArgExpr *DAE = dyn_cast(E)) - E = DAE->getExpr(); - return E; -} - /// isKnownToHaveBooleanValue - Return true if this is an integer expression /// that is known to return 0 or 1. This happens for _Bool/bool expressions /// but also int expressions which are produced by things like comparisons in diff --git a/lib/Sema/JumpDiagnostics.cpp b/lib/Sema/JumpDiagnostics.cpp index b58bc51807..a07f44221a 100644 --- a/lib/Sema/JumpDiagnostics.cpp +++ b/lib/Sema/JumpDiagnostics.cpp @@ -121,9 +121,11 @@ typedef std::pair ScopePair; /// GetDiagForGotoScopeDecl - If this decl induces a new goto scope, return a /// diagnostic that should be emitted if control goes over it. If not, return 0. -static ScopePair GetDiagForGotoScopeDecl(ASTContext &Context, const Decl *D) { +static ScopePair GetDiagForGotoScopeDecl(Sema &S, const Decl *D) { if (const VarDecl *VD = dyn_cast(D)) { unsigned InDiag = 0; + unsigned OutDiag = 0; + if (VD->getType()->isVariablyModifiedType()) InDiag = diag::note_protected_by_vla; @@ -135,21 +137,24 @@ static ScopePair GetDiagForGotoScopeDecl(ASTContext &Context, const Decl *D) { return ScopePair(diag::note_protected_by_cleanup, diag::note_exits_cleanup); - if (Context.getLangOpts().ObjCAutoRefCount && VD->hasLocalStorage()) { - switch (VD->getType().getObjCLifetime()) { - case Qualifiers::OCL_None: - case Qualifiers::OCL_ExplicitNone: - case Qualifiers::OCL_Autoreleasing: - break; - - case Qualifiers::OCL_Strong: - case Qualifiers::OCL_Weak: + if (VD->hasLocalStorage()) { + switch (VD->getType().isDestructedType()) { + case QualType::DK_objc_strong_lifetime: + case QualType::DK_objc_weak_lifetime: return ScopePair(diag::note_protected_by_objc_ownership, diag::note_exits_objc_ownership); + + case QualType::DK_cxx_destructor: + OutDiag = diag::note_exits_dtor; + break; + + case QualType::DK_none: + break; } } - if (Context.getLangOpts().CPlusPlus && VD->hasLocalStorage()) { + const Expr *Init = VD->getInit(); + if (S.Context.getLangOpts().CPlusPlus && VD->hasLocalStorage() && Init) { // C++11 [stmt.dcl]p3: // A program that jumps from a point where a variable with automatic // storage duration is not in scope to a point where it is in scope @@ -164,68 +169,34 @@ static ScopePair GetDiagForGotoScopeDecl(ASTContext &Context, const Decl *D) { // where it is in scope is ill-formed unless the variable has // POD type and is declared without an initializer. - const Expr *Init = VD->getInit(); - if (!Init) - return ScopePair(InDiag, 0); - - const ExprWithCleanups *EWC = dyn_cast(Init); - if (EWC) - Init = EWC->getSubExpr(); - - const MaterializeTemporaryExpr *M = NULL; - Init = Init->findMaterializedTemporary(M); - - SmallVector CommaLHSs; - SmallVector Adjustments; - Init = Init->skipRValueSubobjectAdjustments(CommaLHSs, Adjustments); + InDiag = diag::note_protected_by_variable_init; - QualType QT = Init->getType(); - if (QT.isNull()) - return ScopePair(diag::note_protected_by_variable_init, 0); - - const Type *T = QT.getTypePtr(); - if (T->isArrayType()) - T = T->getBaseElementTypeUnsafe(); - - const CXXRecordDecl *Record = T->getAsCXXRecordDecl(); - if (!Record) - return ScopePair(diag::note_protected_by_variable_init, 0); - - // If we need to call a non-trivial destructor for this variable, - // record an out diagnostic. - unsigned OutDiag = 0; - if (!Init->isGLValue() && !Record->hasTrivialDestructor()) - OutDiag = diag::note_exits_dtor; - - if (const CXXConstructExpr *cce = dyn_cast(Init)) { - const CXXConstructorDecl *ctor = cce->getConstructor(); - // For a variable declared without an initializer, we will have - // call-style initialization and the initializer will be the - // CXXConstructExpr with no intervening nodes. - if (ctor->isTrivial() && ctor->isDefaultConstructor() && - VD->getInit() == Init && VD->getInitStyle() == VarDecl::CallInit) { + // For a variable of (array of) class type declared without an + // initializer, we will have call-style initialization and the initializer + // will be the CXXConstructExpr with no intervening nodes. + if (const CXXConstructExpr *CCE = dyn_cast(Init)) { + const CXXConstructorDecl *Ctor = CCE->getConstructor(); + if (Ctor->isTrivial() && Ctor->isDefaultConstructor() && + VD->getInitStyle() == VarDecl::CallInit) { if (OutDiag) InDiag = diag::note_protected_by_variable_nontriv_destructor; - else if (!Record->isPOD()) + else if (!Ctor->getParent()->isPOD()) InDiag = diag::note_protected_by_variable_non_pod; - return ScopePair(InDiag, OutDiag); + else + InDiag = 0; } } - - return ScopePair(diag::note_protected_by_variable_init, OutDiag); } - return ScopePair(InDiag, 0); + return ScopePair(InDiag, OutDiag); } - if (const TypedefDecl *TD = dyn_cast(D)) { + if (const TypedefNameDecl *TD = dyn_cast(D)) { if (TD->getUnderlyingType()->isVariablyModifiedType()) - return ScopePair(diag::note_protected_by_vla_typedef, 0); - } - - if (const TypeAliasDecl *TD = dyn_cast(D)) { - if (TD->getUnderlyingType()->isVariablyModifiedType()) - return ScopePair(diag::note_protected_by_vla_type_alias, 0); + return ScopePair(isa(TD) + ? diag::note_protected_by_vla_typedef + : diag::note_protected_by_vla_type_alias, + 0); } return ScopePair(0U, 0U); @@ -234,7 +205,7 @@ static ScopePair GetDiagForGotoScopeDecl(ASTContext &Context, const Decl *D) { /// \brief Build scope information for a declaration that is part of a DeclStmt. void JumpScopeChecker::BuildScopeInformation(Decl *D, unsigned &ParentScope) { // If this decl causes a new scope, push and switch to it. - std::pair Diags = GetDiagForGotoScopeDecl(S.Context, D); + std::pair Diags = GetDiagForGotoScopeDecl(S, D); if (Diags.first || Diags.second) { Scopes.push_back(GotoScope(ParentScope, Diags.first, Diags.second, D->getLocation())); @@ -481,7 +452,26 @@ void JumpScopeChecker::BuildScopeInformation(Stmt *S, unsigned &origParentScope) } } } - + + // Disallow jumps out of scopes containing temporaries lifetime-extended to + // automatic storage duration. + if (MaterializeTemporaryExpr *MTE = + dyn_cast(SubStmt)) { + if (MTE->getStorageDuration() == SD_Automatic) { + SmallVector CommaLHS; + SmallVector Adjustments; + const Expr *ExtendedObject = + MTE->GetTemporaryExpr()->skipRValueSubobjectAdjustments( + CommaLHS, Adjustments); + if (ExtendedObject->getType().isDestructedType()) { + Scopes.push_back(GotoScope(ParentScope, 0, + diag::note_exits_temporary_dtor, + ExtendedObject->getExprLoc())); + ParentScope = Scopes.size()-1; + } + } + } + // Recursively walk the AST. BuildScopeInformation(SubStmt, ParentScope); } diff --git a/test/SemaCXX/scope-check.cpp b/test/SemaCXX/scope-check.cpp index 90c9317ecd..c5fdb09f23 100644 --- a/test/SemaCXX/scope-check.cpp +++ b/test/SemaCXX/scope-check.cpp @@ -226,7 +226,7 @@ namespace test12 { static void *ips[] = { &&l0 }; const C c0 = 17; l0: // expected-note {{possible target of indirect goto}} - const C &c1 = 42; // expected-note {{jump exits scope of variable with non-trivial destructor}} + const C &c1 = 42; // expected-note {{jump exits scope of lifetime-extended temporary with non-trivial destructor}} const C &c2 = c0; goto *ip; // expected-error {{indirect goto might cross protected scopes}} } @@ -241,7 +241,7 @@ namespace test13 { void f(void **ip) { static void *ips[] = { &&l0 }; l0: // expected-note {{possible target of indirect goto}} - const int &c1 = C(1).i; // expected-note {{jump exits scope of variable with non-trivial destructor}} + const int &c1 = C(1).i; // expected-note {{jump exits scope of lifetime-extended temporary with non-trivial destructor}} goto *ip; // expected-error {{indirect goto might cross protected scopes}} } } @@ -295,6 +295,120 @@ x: return s.get(); } #endif +namespace test18 { + struct A { ~A(); }; + struct B { const int &r; const A &a; }; + int f() { + void *p = &&x; + const A a = A(); + x: + B b = { 0, a }; // ok + goto *p; + } + int g() { + void *p = &&x; + x: // expected-note {{possible target of indirect goto}} + B b = { 0, A() }; // expected-note {{jump exits scope of lifetime-extended temporary with non-trivial destructor}} + goto *p; // expected-error {{indirect goto might cross protected scopes}} + } +} + +#if __cplusplus >= 201103L +namespace std { + typedef decltype(sizeof(int)) size_t; + template struct initializer_list { + const T *begin; + size_t size; + initializer_list(const T *, size_t); + }; +} +namespace test19 { + struct A { ~A(); }; + + int f() { + void *p = &&x; + A a; + x: // expected-note {{possible target of indirect goto}} + std::initializer_list il = { a }; // expected-note {{jump exits scope of lifetime-extended temporary with non-trivial destructor}} + goto *p; // expected-error {{indirect goto might cross protected scopes}} + } +} + +namespace test20 { + struct A { ~A(); }; + struct B { + const A &a; + }; + + int f() { + void *p = &&x; + A a; + x: + std::initializer_list il = { + a, + a + }; + goto *p; + } + int g() { + void *p = &&x; + A a; + x: // expected-note {{possible target of indirect goto}} + std::initializer_list il = { + a, + { A() } // expected-note {{jump exits scope of lifetime-extended temporary with non-trivial destructor}} + }; + goto *p; // expected-error {{indirect goto might cross protected scopes}} + } +} +#endif + +namespace test21 { + template void f() { + goto x; // expected-error {{protected scope}} + T t; // expected-note {{bypasses}} + x: return; + } + + template void f(); + struct X { ~X(); }; + template void f(); // expected-note {{instantiation of}} +} + +namespace PR18217 { + typedef int *X; + + template + class MyCl { + T mem; + }; + + class Source { + MyCl m; + public: + int getKind() const; + }; + + bool b; + template + static void foo(const Source &SF, MyCl Source::*m) { + switch (SF.getKind()) { + case 1: return; + case 2: break; + case 3: + case 4: return; + }; + if (b) { + auto &y = const_cast &>(SF.*m); // expected-warning 0-1{{extension}} + } + } + + int Source::getKind() const { + foo(*this, &Source::m); + return 0; + } +} + // This test must be last, because the error prohibits further jump diagnostics. namespace testInvalid { Invalid inv; // expected-error {{unknown type name}}