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.

llvm-svn: 142619

clang/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<Microsoft>;
+def warn_cxx98_compat_goto_into_protected_scope : Warning<
+  "goto would jump into protected scope in C++98">,
+  InGroup<CXX98Compat>, 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<CXX98Compat>, 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<CXX98Compat>, 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<

clang/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<unsigned> &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<GotoStmt>(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<unsigned> &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<unsigned, 10> 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<unsigned, 10> ToScopesCXX98Compat;
   SmallVector<unsigned, 10> ToScopesError;
   SmallVector<unsigned, 10> 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);
   }
 }
 

clang/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<RecordType>()) {
         CXXRecordDecl *CXXRecord = cast<CXXRecordDecl>(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();
       }
     }

clang/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;
 }

clang/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<int>;
 
-}
+}

clang/test/SemaCXX/cxx98-compat.cpp

@@ -243,3 +243,20 @@ template<typename T> void EnumNNSFn() {
   int k = T::enum_val; // expected-warning {{enumeration type in nested name specifier is incompatible with C++98}}
 };
 template void EnumNNSFn<Enum>(); // 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;
+  }
+}