brad.king created this revision.

In commit r303930 (Switch from using a DiagnosticTrap and a note...,
2017-05-25) use of DiagnosticErrorTrap was removed because it was no
longer needed to detect whether to add a diagnostic note.  However, the
trap was also necessary to correctly detect and mark invalid special
members under `Sema.getDiagnostics().setSuppressAllDiagnostics(true)`.
Restore use of the trap to fix semantic checking when diagnostics are
suppressed.

This is useful for external tools that need to detect the set of valid
special members without issuing diagnostics on the invalid ones.


https://reviews.llvm.org/D37381

Files:
  lib/Sema/SemaDeclCXX.cpp

Index: lib/Sema/SemaDeclCXX.cpp
===================================================================
--- lib/Sema/SemaDeclCXX.cpp
+++ lib/Sema/SemaDeclCXX.cpp
@@ -10423,6 +10423,7 @@
   assert(ClassDecl && "DefineImplicitDefaultConstructor - invalid constructor");
 
   SynthesizedFunctionScope Scope(*this, Constructor);
+  DiagnosticErrorTrap Trap(Diags);
 
   // The exception specification is needed because we are defining the
   // function.
@@ -10433,7 +10434,8 @@
   // Add a context note for diagnostics produced after this point.
   Scope.addContextNote(CurrentLocation);
 
-  if (SetCtorInitializers(Constructor, /*AnyErrors=*/false)) {
+  if (SetCtorInitializers(Constructor, /*AnyErrors=*/false) ||
+      Trap.hasErrorOccurred()) {
     Constructor->setInvalidDecl();
     return;
   }
@@ -10558,6 +10560,7 @@
   // Initializations are performed "as if by a defaulted default constructor",
   // so enter the appropriate scope.
   SynthesizedFunctionScope Scope(*this, Constructor);
+  DiagnosticErrorTrap Trap(Diags);
 
   // The exception specification is needed because we are defining the
   // function.
@@ -10612,7 +10615,8 @@
   // We now proceed as if for a defaulted default constructor, with the relevant
   // initializers replaced.
 
-  if (SetCtorInitializers(Constructor, /*AnyErrors*/false, Inits)) {
+  if (SetCtorInitializers(Constructor, /*AnyErrors*/false, Inits) ||
+      Trap.hasErrorOccurred()) {
     Constructor->setInvalidDecl();
     return;
   }
@@ -10701,6 +10705,7 @@
   assert(ClassDecl && "DefineImplicitDestructor - invalid destructor");
 
   SynthesizedFunctionScope Scope(*this, Destructor);
+  DiagnosticErrorTrap Trap(Diags);
 
   // The exception specification is needed because we are defining the
   // function.
@@ -10714,7 +10719,7 @@
   MarkBaseAndMemberDestructorsReferenced(Destructor->getLocation(),
                                          Destructor->getParent());
 
-  if (CheckDestructor(Destructor)) {
+  if (CheckDestructor(Destructor) || Trap.hasErrorOccurred()) {
     Destructor->setInvalidDecl();
     return;
   }
@@ -11351,6 +11356,7 @@
   }
 
   SynthesizedFunctionScope Scope(*this, CopyAssignOperator);
+  DiagnosticErrorTrap Trap(Diags);
 
   // The exception specification is needed because we are defining the
   // function.
@@ -11513,8 +11519,11 @@
     StmtResult Return = BuildReturnStmt(Loc, ThisObj.get());
     if (Return.isInvalid())
       Invalid = true;
-    else
+    else {
       Statements.push_back(Return.getAs<Stmt>());
+      if (Trap.hasErrorOccurred())
+        Invalid = true;
+    }
   }
 
   if (Invalid) {
@@ -11725,6 +11734,7 @@
   checkMoveAssignmentForRepeatedMove(*this, ClassDecl, CurrentLocation);
 
   SynthesizedFunctionScope Scope(*this, MoveAssignOperator);
+  DiagnosticErrorTrap Trap(Diags);
 
   // The exception specification is needed because we are defining the
   // function.
@@ -11884,8 +11894,11 @@
     StmtResult Return = BuildReturnStmt(Loc, ThisObj.get());
     if (Return.isInvalid())
       Invalid = true;
-    else
+    else {
       Statements.push_back(Return.getAs<Stmt>());
+      if (Trap.hasErrorOccurred())
+        Invalid = true;
+    }
   }
 
   if (Invalid) {
@@ -12003,6 +12016,7 @@
   assert(ClassDecl && "DefineImplicitCopyConstructor - invalid constructor");
 
   SynthesizedFunctionScope Scope(*this, CopyConstructor);
+  DiagnosticErrorTrap Trap(Diags);
 
   // The exception specification is needed because we are defining the
   // function.
@@ -12020,7 +12034,8 @@
   if (getLangOpts().CPlusPlus11 && CopyConstructor->isImplicit())
     diagnoseDeprecatedCopyOperation(*this, CopyConstructor);
 
-  if (SetCtorInitializers(CopyConstructor, /*AnyErrors=*/false)) {
+  if (SetCtorInitializers(CopyConstructor, /*AnyErrors=*/false) ||
+      Trap.hasErrorOccurred()) {
     CopyConstructor->setInvalidDecl();
   }  else {
     SourceLocation Loc = CopyConstructor->getLocEnd().isValid()
@@ -12126,6 +12141,7 @@
   assert(ClassDecl && "DefineImplicitMoveConstructor - invalid constructor");
 
   SynthesizedFunctionScope Scope(*this, MoveConstructor);
+  DiagnosticErrorTrap Trap(Diags);
 
   // The exception specification is needed because we are defining the
   // function.
@@ -12136,7 +12152,8 @@
   // Add a context note for diagnostics produced after this point.
   Scope.addContextNote(CurrentLocation);
 
-  if (SetCtorInitializers(MoveConstructor, /*AnyErrors=*/false)) {
+  if (SetCtorInitializers(MoveConstructor, /*AnyErrors=*/false) ||
+      Trap.hasErrorOccurred()) {
     MoveConstructor->setInvalidDecl();
   } else {
     SourceLocation Loc = MoveConstructor->getLocEnd().isValid()
_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

Reply via email to