[NFC] Add DiagnosticStateRAII

DiagnosticEngine tracks whether an error/warning was ignored so that it can also automatically ignore related notes. This can go rather poorly if you have logic between the warning/error and notes to determine which notes to emit, and this logic emits ignored diagnostics.

Add an RAII type to handle this situation and use it at all entry points into TypeCheckDeclObjC.cpp, since this code is about to start using ignored diagnostics pretty heavily.

Author: beccadax

include/swift/AST/DiagnosticEngine.h

@@ -29,6 +29,7 @@
 #include "llvm/Support/Allocator.h"
 #include "llvm/Support/FileSystem.h"
 #include "llvm/Support/Path.h"
+#include "llvm/Support/SaveAndRestore.h"
 #include "llvm/Support/VersionTuple.h"
 
 namespace swift {
@@ -640,6 +641,8 @@ namespace swift {
     /// Track which diagnostics should be ignored.
     llvm::BitVector ignoredDiagnostics;
 
+    friend class DiagnosticStateRAII;
+
   public:
     DiagnosticState();
 
@@ -740,6 +743,7 @@ namespace swift {
     friend class InFlightDiagnostic;
     friend class DiagnosticTransaction;
     friend class CompoundDiagnosticTransaction;
+    friend class DiagnosticStateRAII;
 
   public:
     explicit DiagnosticEngine(SourceManager &SourceMgr)
@@ -1053,6 +1057,33 @@ namespace swift {
     }
   };
 
+  /// Remember details about the state of a diagnostic engine and restore them
+  /// when the object is destroyed.
+  ///
+  /// Diagnostic engines contain state about the most recent diagnostic emitted
+  /// which influences subsequent emissions; in particular, if you try to emit
+  /// a note and the previous diagnostic was ignored, the note will be ignored
+  /// too. This can be a problem in code structured like:
+  ///
+  ///     D->diagnose(diag::an_error);
+  ///     if (conditionWhichMightEmitDiagnostics())
+  ///        D->diagnose(diag::a_note); // might be affected by diagnostics from
+  ///                                   // conditionWhichMightEmitDiagnostics()!
+  ///
+  /// To prevent this, functions which are called for their return values but
+  /// may emit diagnostics as a side effect can use \c DiagnosticStateRAII to
+  /// ensure that their changes to diagnostic engine state don't leak out and
+  /// affect the caller's diagnostics.
+  class DiagnosticStateRAII {
+    llvm::SaveAndRestore<DiagnosticBehavior> previousBehavior;
+
+  public:
+    DiagnosticStateRAII(DiagnosticEngine &diags)
+      : previousBehavior(diags.state.previousBehavior) {}
+
+    ~DiagnosticStateRAII() {}
+  };
+
   class BufferIndirectlyCausingDiagnosticRAII {
   private:
     DiagnosticEngine &Diags;

lib/Sema/TypeCheckDeclObjC.cpp

@@ -553,6 +553,7 @@ bool swift::isRepresentableInObjC(
 
   // If you change this function, you must add or modify a test in PrintAsObjC.
   ASTContext &ctx = AFD->getASTContext();
+  DiagnosticStateRAII diagState(ctx.Diags);
 
   bool Diagnose = shouldDiagnoseObjCReason(Reason, ctx);
 
@@ -985,6 +986,8 @@ bool swift::isRepresentableInObjC(const VarDecl *VD, ObjCReason Reason) {
     T = RST->getReferentType();
   }
   ASTContext &ctx = VD->getASTContext();
+  DiagnosticStateRAII diagState(ctx.Diags);
+
   bool Result = T->isRepresentableIn(ForeignLanguage::ObjectiveC,
                                      VD->getDeclContext());
   bool Diagnose = shouldDiagnoseObjCReason(Reason, ctx);
@@ -1018,6 +1021,7 @@ bool swift::isRepresentableInObjC(const VarDecl *VD, ObjCReason Reason) {
 bool swift::isRepresentableInObjC(const SubscriptDecl *SD, ObjCReason Reason) {
   // If you change this function, you must add or modify a test in PrintAsObjC.
   ASTContext &ctx = SD->getASTContext();
+  DiagnosticStateRAII diagState(ctx.Diags);
   bool Diagnose = shouldDiagnoseObjCReason(Reason, ctx);
 
   if (checkObjCInForeignClassContext(SD, Reason))
@@ -1219,6 +1223,7 @@ static Optional<ObjCReason> shouldMarkClassAsObjC(const ClassDecl *CD) {
 }
 
 /// Figure out if a declaration should be exported to Objective-C.
+static
 Optional<ObjCReason> shouldMarkAsObjC(const ValueDecl *VD, bool allowImplicit) {
   // If Objective-C interoperability is disabled, nothing gets marked as @objc.
   if (!VD->getASTContext().LangOpts.EnableObjCInterop)
@@ -1472,6 +1477,8 @@ static void markAsObjC(ValueDecl *D, ObjCReason reason,
 
 
 bool IsObjCRequest::evaluate(Evaluator &evaluator, ValueDecl *VD) const {
+  DiagnosticStateRAII diagState(VD->getASTContext().Diags);
+
   // Access notes may add attributes that affect this calculus.
   (void)evaluateOrDefault(evaluator, ApplyAccessNoteRequest{VD}, {});
 
@@ -2180,6 +2187,7 @@ lookupOverridenObjCMethod(ClassDecl *classDecl, AbstractFunctionDecl *method,
 
 bool swift::diagnoseUnintendedObjCMethodOverrides(SourceFile &sf) {
   auto &Ctx = sf.getASTContext();
+  DiagnosticStateRAII diagState(Ctx.Diags);
   auto &methods = sf.ObjCMethodList;
 
   // If no Objective-C methods were defined in this file, we're done.
@@ -2284,6 +2292,7 @@ bool swift::diagnoseObjCMethodConflicts(SourceFile &sf) {
     return false;
 
   auto &Ctx = sf.getASTContext();
+  DiagnosticStateRAII diagState(Ctx.Diags);
   OrderDeclarations ordering;
 
   // Sort the set of conflicts so we get a deterministic order for
@@ -2389,6 +2398,7 @@ bool swift::diagnoseObjCUnsatisfiedOptReqConflicts(SourceFile &sf) {
     return false;
 
   auto &Ctx = sf.getASTContext();
+  DiagnosticStateRAII diagState(Ctx.Diags);
 
   // Sort the set of local unsatisfied requirements, so we get a
   // deterministic order for diagnostics.