From: Richard Smith Date: Thu, 20 Oct 2011 21:42:12 +0000 (+0000) Subject: Add -Wc++98-compat diagnostics for jumps which bypass initialization of non-POD X-Git-Url: https://granicus.if.org/sourcecode?a=commitdiff_plain;h=0e9e9814a7f8313c0c02b6afea71f0e4c411873e;p=clang Add -Wc++98-compat diagnostics for jumps which bypass initialization of non-POD but trivially constructible and destructible variables in C++11 mode. Also incidentally improve the precision of the wording for jump diagnostics in C++98 mode. git-svn-id: https://llvm.org/svn/llvm-project/cfe/trunk@142619 91177308-0d34-0410-b5e6-96231b3b80d8 --- diff --git a/include/clang/Basic/DiagnosticSemaKinds.td b/include/clang/Basic/DiagnosticSemaKinds.td index 3267635249..38ba954d35 100644 --- a/include/clang/Basic/DiagnosticSemaKinds.td +++ b/include/clang/Basic/DiagnosticSemaKinds.td @@ -2891,17 +2891,28 @@ def warn_unused_label : Warning<"unused label %0">, def err_goto_into_protected_scope : Error<"goto into protected scope">; def warn_goto_into_protected_scope : ExtWarn<"goto into protected scope">, InGroup; +def warn_cxx98_compat_goto_into_protected_scope : Warning< + "goto would jump into protected scope in C++98">, + InGroup, DefaultIgnore; def err_switch_into_protected_scope : Error< "switch case is in protected scope">; +def warn_cxx98_compat_switch_into_protected_scope : Warning< + "switch case would be in a protected scope in C++98">, + InGroup, DefaultIgnore; def err_indirect_goto_without_addrlabel : Error< "indirect goto in function with no address-of-label expressions">; def err_indirect_goto_in_protected_scope : Error< "indirect goto might cross protected scopes">; +def warn_cxx98_compat_indirect_goto_in_protected_scope : Warning< + "indirect goto might cross protected scopes in C++98">, + InGroup, DefaultIgnore; def note_indirect_goto_target : Note<"possible target of indirect goto">; def note_protected_by_variable_init : Note< "jump bypasses variable initialization">; def note_protected_by_variable_nontriv_destructor : Note< "jump bypasses variable with a non-trivial destructor">; +def note_protected_by_variable_non_pod : Note< + "jump bypasses initialization of non-POD variable">; def note_protected_by_cleanup : Note< "jump bypasses initialization of variable with __attribute__((cleanup))">; def note_protected_by_vla_typedef : Note< diff --git a/lib/Sema/JumpDiagnostics.cpp b/lib/Sema/JumpDiagnostics.cpp index 3e74dfccbd..54013a8e9c 100644 --- a/lib/Sema/JumpDiagnostics.cpp +++ b/lib/Sema/JumpDiagnostics.cpp @@ -42,10 +42,10 @@ class JumpScopeChecker { /// the parent scope is the function body. unsigned ParentScope; - /// InDiag - The diagnostic to emit if there is a jump into this scope. + /// InDiag - The note to emit if there is a jump into this scope. unsigned InDiag; - /// OutDiag - The diagnostic to emit if there is an indirect jump out + /// OutDiag - The note to emit if there is an indirect jump out /// of this scope. Direct jumps always clean up their current scope /// in an orderly way. unsigned OutDiag; @@ -74,10 +74,12 @@ private: void VerifyJumps(); void VerifyIndirectJumps(); + void NoteJumpIntoScopes(const SmallVectorImpl &ToScopes); void DiagnoseIndirectJump(IndirectGotoStmt *IG, unsigned IGScope, LabelDecl *Target, unsigned TargetScope); void CheckJump(Stmt *From, Stmt *To, SourceLocation DiagLoc, - unsigned JumpDiag, unsigned JumpDiagWarning); + unsigned JumpDiag, unsigned JumpDiagWarning, + unsigned JumpDiagCXX98Compat); unsigned GetDeepestCommonScope(unsigned A, unsigned B); }; @@ -148,7 +150,7 @@ static ScopePair GetDiagForGotoScopeDecl(ASTContext &Context, const Decl *D) { } if (Context.getLangOptions().CPlusPlus && VD->hasLocalStorage()) { - // C++0x [stmt.dcl]p3: + // 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 // is ill-formed unless the variable has scalar type, class type with @@ -177,13 +179,12 @@ static ScopePair GetDiagForGotoScopeDecl(ASTContext &Context, const Decl *D) { record = ctor->getParent(); if (ctor->isTrivial() && ctor->isDefaultConstructor()) { - if (Context.getLangOptions().CPlusPlus0x) { - inDiagToUse = (record->hasTrivialDestructor() ? 0 : - diag::note_protected_by_variable_nontriv_destructor); - } else { - if (record->isPOD()) - inDiagToUse = 0; - } + if (!record->hasTrivialDestructor()) + inDiagToUse = diag::note_protected_by_variable_nontriv_destructor; + else if (!record->isPOD()) + inDiagToUse = diag::note_protected_by_variable_non_pod; + else + inDiagToUse = 0; } } else if (VD->getType()->isArrayType()) { record = VD->getType()->getBaseElementTypeUnsafe() @@ -258,7 +259,7 @@ void JumpScopeChecker::BuildScopeInformation(VarDecl *D, diag::note_exits_block_captures_weak); break; case QualType::DK_none: - llvm_unreachable("no-liftime captured variable"); + llvm_unreachable("non-lifetime captured variable"); } SourceLocation Loc = D->getLocation(); if (Loc.isInvalid()) @@ -477,7 +478,8 @@ void JumpScopeChecker::VerifyJumps() { if (GotoStmt *GS = dyn_cast(Jump)) { CheckJump(GS, GS->getLabel()->getStmt(), GS->getGotoLoc(), diag::err_goto_into_protected_scope, - diag::warn_goto_into_protected_scope); + diag::warn_goto_into_protected_scope, + diag::warn_cxx98_compat_goto_into_protected_scope); continue; } @@ -486,7 +488,8 @@ void JumpScopeChecker::VerifyJumps() { LabelDecl *Target = IGS->getConstantTarget(); CheckJump(IGS, Target->getStmt(), IGS->getGotoLoc(), diag::err_goto_into_protected_scope, - diag::warn_goto_into_protected_scope); + diag::warn_goto_into_protected_scope, + diag::warn_cxx98_compat_goto_into_protected_scope); continue; } @@ -495,7 +498,8 @@ void JumpScopeChecker::VerifyJumps() { SC = SC->getNextSwitchCase()) { assert(LabelAndGotoScopes.count(SC) && "Case not visited?"); CheckJump(SS, SC, SC->getLocStart(), - diag::err_switch_into_protected_scope, 0); + diag::err_switch_into_protected_scope, 0, + diag::warn_cxx98_compat_switch_into_protected_scope); } } } @@ -639,6 +643,40 @@ void JumpScopeChecker::VerifyIndirectJumps() { } } +/// Return true if a particular error+note combination must be downgraded to a +/// warning in Microsoft mode. +static bool IsMicrosoftJumpWarning(unsigned JumpDiag, unsigned InDiagNote) { + return (JumpDiag == diag::err_goto_into_protected_scope && + (InDiagNote == diag::note_protected_by_variable_init || + InDiagNote == diag::note_protected_by_variable_nontriv_destructor)); +} + +/// Return true if a particular note should be downgraded to a compatibility +/// warning in C++11 mode. +static bool IsCXX98CompatWarning(Sema &S, unsigned InDiagNote) { + return S.getLangOptions().CPlusPlus0x && + InDiagNote == diag::note_protected_by_variable_non_pod; +} + +/// Produce primary diagnostic for an indirect jump statement. +static void DiagnoseIndirectJumpStmt(Sema &S, IndirectGotoStmt *Jump, + LabelDecl *Target, bool &Diagnosed) { + if (Diagnosed) + return; + S.Diag(Jump->getGotoLoc(), diag::err_indirect_goto_in_protected_scope); + S.Diag(Target->getStmt()->getIdentLoc(), diag::note_indirect_goto_target); + Diagnosed = true; +} + +/// Produce note diagnostics for a jump into a protected scope. +void JumpScopeChecker::NoteJumpIntoScopes( + const SmallVectorImpl &ToScopes) { + assert(!ToScopes.empty()); + for (unsigned I = 0, E = ToScopes.size(); I != E; ++I) + if (Scopes[ToScopes[I]].InDiag) + S.Diag(Scopes[ToScopes[I]].Loc, Scopes[ToScopes[I]].InDiag); +} + /// Diagnose an indirect jump which is known to cross scopes. void JumpScopeChecker::DiagnoseIndirectJump(IndirectGotoStmt *Jump, unsigned JumpScope, @@ -646,36 +684,41 @@ void JumpScopeChecker::DiagnoseIndirectJump(IndirectGotoStmt *Jump, unsigned TargetScope) { assert(JumpScope != TargetScope); - S.Diag(Jump->getGotoLoc(), diag::err_indirect_goto_in_protected_scope); - S.Diag(Target->getStmt()->getIdentLoc(), diag::note_indirect_goto_target); - unsigned Common = GetDeepestCommonScope(JumpScope, TargetScope); + bool Diagnosed = false; // Walk out the scope chain until we reach the common ancestor. for (unsigned I = JumpScope; I != Common; I = Scopes[I].ParentScope) - if (Scopes[I].OutDiag) + if (Scopes[I].OutDiag) { + DiagnoseIndirectJumpStmt(S, Jump, Target, Diagnosed); S.Diag(Scopes[I].Loc, Scopes[I].OutDiag); + } + + SmallVector ToScopesCXX98Compat; // Now walk into the scopes containing the label whose address was taken. for (unsigned I = TargetScope; I != Common; I = Scopes[I].ParentScope) - if (Scopes[I].InDiag) + if (IsCXX98CompatWarning(S, Scopes[I].InDiag)) + ToScopesCXX98Compat.push_back(I); + else if (Scopes[I].InDiag) { + DiagnoseIndirectJumpStmt(S, Jump, Target, Diagnosed); S.Diag(Scopes[I].Loc, Scopes[I].InDiag); -} + } -/// Return true if a particular error+note combination must be downgraded -/// to a warning in Microsoft mode. -static bool IsMicrosoftJumpWarning(unsigned JumpDiag, unsigned InDiagNote) -{ - return (JumpDiag == diag::err_goto_into_protected_scope && - (InDiagNote == diag::note_protected_by_variable_init || - InDiagNote == diag::note_protected_by_variable_nontriv_destructor)); + // Diagnose this jump if it would be ill-formed in C++98. + if (!Diagnosed && !ToScopesCXX98Compat.empty()) { + S.Diag(Jump->getGotoLoc(), + diag::warn_cxx98_compat_indirect_goto_in_protected_scope); + S.Diag(Target->getStmt()->getIdentLoc(), diag::note_indirect_goto_target); + NoteJumpIntoScopes(ToScopesCXX98Compat); + } } - /// CheckJump - Validate that the specified jump statement is valid: that it is /// jumping within or out of its current scope, not into a deeper one. void JumpScopeChecker::CheckJump(Stmt *From, Stmt *To, SourceLocation DiagLoc, - unsigned JumpDiagError, unsigned JumpDiagWarning) { + unsigned JumpDiagError, unsigned JumpDiagWarning, + unsigned JumpDiagCXX98Compat) { assert(LabelAndGotoScopes.count(From) && "Jump didn't get added to scopes?"); unsigned FromScope = LabelAndGotoScopes[From]; @@ -691,12 +734,15 @@ void JumpScopeChecker::CheckJump(Stmt *From, Stmt *To, SourceLocation DiagLoc, if (CommonScope == ToScope) return; // Pull out (and reverse) any scopes we might need to diagnose skipping. + SmallVector ToScopesCXX98Compat; SmallVector ToScopesError; SmallVector ToScopesWarning; for (unsigned I = ToScope; I != CommonScope; I = Scopes[I].ParentScope) { if (S.getLangOptions().MicrosoftMode && JumpDiagWarning != 0 && IsMicrosoftJumpWarning(JumpDiagError, Scopes[I].InDiag)) ToScopesWarning.push_back(I); + else if (IsCXX98CompatWarning(S, Scopes[I].InDiag)) + ToScopesCXX98Compat.push_back(I); else if (Scopes[I].InDiag) ToScopesError.push_back(I); } @@ -704,16 +750,19 @@ void JumpScopeChecker::CheckJump(Stmt *From, Stmt *To, SourceLocation DiagLoc, // Handle warnings. if (!ToScopesWarning.empty()) { S.Diag(DiagLoc, JumpDiagWarning); - for (unsigned i = 0, e = ToScopesWarning.size(); i != e; ++i) - S.Diag(Scopes[ToScopesWarning[i]].Loc, Scopes[ToScopesWarning[i]].InDiag); + NoteJumpIntoScopes(ToScopesWarning); } // Handle errors. if (!ToScopesError.empty()) { S.Diag(DiagLoc, JumpDiagError); - // Emit diagnostics note for whatever is left in ToScopesError. - for (unsigned i = 0, e = ToScopesError.size(); i != e; ++i) - S.Diag(Scopes[ToScopesError[i]].Loc, Scopes[ToScopesError[i]].InDiag); + NoteJumpIntoScopes(ToScopesError); + } + + // Handle -Wc++98-compat warnings if the jump is well-formed. + if (ToScopesError.empty() && !ToScopesCXX98Compat.empty()) { + S.Diag(DiagLoc, JumpDiagCXX98Compat); + NoteJumpIntoScopes(ToScopesCXX98Compat); } } diff --git a/lib/Sema/SemaDecl.cpp b/lib/Sema/SemaDecl.cpp index f4c5237ff2..b06b884450 100644 --- a/lib/Sema/SemaDecl.cpp +++ b/lib/Sema/SemaDecl.cpp @@ -6358,7 +6358,7 @@ void Sema::ActOnUninitializedDecl(Decl *RealDecl, // Check for jumps past the implicit initializer. C++0x // clarifies that this applies to a "variable with automatic // storage duration", not a "local variable". - // C++0x [stmt.dcl]p3 + // 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 is // ill-formed unless the variable has scalar type, class type with a @@ -6369,10 +6369,10 @@ void Sema::ActOnUninitializedDecl(Decl *RealDecl, if (const RecordType *Record = Context.getBaseElementType(Type)->getAs()) { CXXRecordDecl *CXXRecord = cast(Record->getDecl()); - if ((!getLangOptions().CPlusPlus0x && !CXXRecord->isPOD()) || - (getLangOptions().CPlusPlus0x && - (!CXXRecord->hasTrivialDefaultConstructor() || - !CXXRecord->hasTrivialDestructor()))) + // Mark the function for further checking even if the looser rules of + // C++11 do not require such checks, so that we can diagnose + // incompatibilities with C++98. + if (!CXXRecord->isPOD()) getCurFunction()->setHasBranchProtectedScope(); } } diff --git a/test/CXX/stmt.stmt/stmt.dcl/p3.cpp b/test/CXX/stmt.stmt/stmt.dcl/p3.cpp index 18fd34039e..f52e3b6d14 100644 --- a/test/CXX/stmt.stmt/stmt.dcl/p3.cpp +++ b/test/CXX/stmt.stmt/stmt.dcl/p3.cpp @@ -30,7 +30,7 @@ struct Y { void test_Y() { goto end; // expected-error{{goto into protected scope}} - Y y; // expected-note{{jump bypasses variable initialization}} + Y y; // expected-note{{jump bypasses variable with a non-trivial destructor}} end: return; } @@ -41,7 +41,7 @@ struct Z { void test_Z() { goto end; // expected-error{{goto into protected scope}} - Z z; // expected-note{{jump bypasses variable initialization}} + Z z; // expected-note{{jump bypasses initialization of non-POD variable}} end: return; } diff --git a/test/SemaCXX/MicrosoftCompatibility.cpp b/test/SemaCXX/MicrosoftCompatibility.cpp index dfc47d6fc9..1dadfec700 100644 --- a/test/SemaCXX/MicrosoftCompatibility.cpp +++ b/test/SemaCXX/MicrosoftCompatibility.cpp @@ -35,7 +35,7 @@ struct Y { void jump_over_var_with_dtor() { goto end; // expected-warning{{goto into protected scope}} - Y y; // expected-note {{jump bypasses variable initialization}} + Y y; // expected-note {{jump bypasses variable with a non-trivial destructor}} end: ; } @@ -155,4 +155,4 @@ public: template class B; -} \ No newline at end of file +} diff --git a/test/SemaCXX/cxx98-compat.cpp b/test/SemaCXX/cxx98-compat.cpp index 260e86b0ec..051176bf9c 100644 --- a/test/SemaCXX/cxx98-compat.cpp +++ b/test/SemaCXX/cxx98-compat.cpp @@ -243,3 +243,20 @@ template void EnumNNSFn() { int k = T::enum_val; // expected-warning {{enumeration type in nested name specifier is incompatible with C++98}} }; template void EnumNNSFn(); // expected-note {{in instantiation}} + +void JumpDiagnostics(int n) { + goto DirectJump; // expected-warning {{goto would jump into protected scope in C++98}} + TrivialButNonPOD tnp1; // expected-note {{jump bypasses initialization of non-POD variable}} + +DirectJump: + void *Table[] = {&&DirectJump, &&Later}; + goto *Table[n]; // expected-warning {{indirect goto might cross protected scopes in C++98}} + + TrivialButNonPOD tnp2; // expected-note {{jump bypasses initialization of non-POD variable}} +Later: // expected-note {{possible target of indirect goto}} + switch (n) { + TrivialButNonPOD tnp3; // expected-note {{jump bypasses initialization of non-POD variable}} + default: // expected-warning {{switch case would be in a protected scope in C++98}} + return; + } +}