Merge pull request swiftlang#36125 from beccadax/behave-yourself

beccadax · web-flow · commit 93ffb88219d2 · 2021-03-04T17:13:03.000-08:00
[NFC] Allow errors to be conditionally emitted as warnings
diff --git a/include/swift/AST/DiagnosticEngine.h b/include/swift/AST/DiagnosticEngine.h
@@ -23,6 +23,7 @@
 #include "swift/AST/DiagnosticConsumer.h"
 #include "swift/AST/TypeLoc.h"
 #include "swift/Localization/LocalizationFormat.h"
+#include "llvm/ADT/BitVector.h"
 #include "llvm/ADT/StringRef.h"
 #include "llvm/ADT/StringSet.h"
 #include "llvm/Support/Allocator.h"
@@ -342,7 +343,19 @@ namespace swift {
       return ActorIsolationVal;
     }
   };
-  
+
+  /// Describes the current behavior to take with a diagnostic.
+  /// Ordered from most severe to least.
+  enum class DiagnosticBehavior : uint8_t {
+    Unspecified = 0,
+    Fatal,
+    Error,
+    Warning,
+    Remark,
+    Note,
+    Ignore,
+  };
+
   struct DiagnosticFormatOptions {
     const std::string OpeningQuotationMark;
     const std::string ClosingQuotationMark;
@@ -393,6 +406,7 @@ namespace swift {
     SourceLoc Loc;
     bool IsChildNote = false;
     const swift::Decl *Decl = nullptr;
+    DiagnosticBehavior BehaviorLimit = DiagnosticBehavior::Unspecified;
 
     friend DiagnosticEngine;
 
@@ -420,10 +434,12 @@ namespace swift {
     bool isChildNote() const { return IsChildNote; }
     SourceLoc getLoc() const { return Loc; }
     const class Decl *getDecl() const { return Decl; }
+    DiagnosticBehavior getBehaviorLimit() const { return BehaviorLimit; }
 
     void setLoc(SourceLoc loc) { Loc = loc; }
     void setIsChildNote(bool isChildNote) { IsChildNote = isChildNote; }
     void setDecl(const class Decl *decl) { Decl = decl; }
+    void setBehaviorLimit(DiagnosticBehavior limit){ BehaviorLimit = limit; }
 
     /// Returns true if this object represents a particular diagnostic.
     ///
@@ -493,6 +509,11 @@ namespace swift {
     /// Flush the active diagnostic to the diagnostic output engine.
     void flush();
 
+    /// Prevent the diagnostic from behaving more severely than \p limit. For
+    /// instance, if \c DiagnosticBehavior::Warning is passed, an error will be
+    /// emitted as a warning, but a note will still be emitted as a note.
+    InFlightDiagnostic &limitBehavior(DiagnosticBehavior limit);
+
     /// Add a token-based range to the currently-active diagnostic.
     InFlightDiagnostic &highlight(SourceRange R);
 
@@ -597,19 +618,6 @@ namespace swift {
   /// Class to track, map, and remap diagnostic severity and fatality
   ///
   class DiagnosticState {
-  public:
-    /// Describes the current behavior to take with a diagnostic
-    enum class Behavior : uint8_t {
-      Unspecified,
-      Ignore,
-      Note,
-      Remark,
-      Warning,
-      Error,
-      Fatal,
-    };
-
-  private:
     /// Whether we should continue to emit diagnostics, even after a
     /// fatal error
     bool showDiagnosticsAfterFatalError = false;
@@ -627,17 +635,17 @@ namespace swift {
     bool anyErrorOccurred = false;
 
     /// Track the previous emitted Behavior, useful for notes
-    Behavior previousBehavior = Behavior::Unspecified;
+    DiagnosticBehavior previousBehavior = DiagnosticBehavior::Unspecified;
 
-    /// Track settable, per-diagnostic state that we store
-    std::vector<Behavior> perDiagnosticBehavior;
+    /// Track which diagnostics should be ignored.
+    llvm::BitVector ignoredDiagnostics;
 
   public:
     DiagnosticState();
 
     /// Figure out the Behavior for the given diagnostic, taking current
     /// state such as fatality into account.
-    Behavior determineBehavior(DiagID id);
+    DiagnosticBehavior determineBehavior(const Diagnostic &diag);
 
     bool hadAnyError() const { return anyErrorOccurred; }
     bool hasFatalErrorOccurred() const { return fatalErrorOccurred; }
@@ -662,9 +670,9 @@ namespace swift {
       fatalErrorOccurred = false;
     }
 
-    /// Set per-diagnostic behavior
-    void setDiagnosticBehavior(DiagID id, Behavior behavior) {
-      perDiagnosticBehavior[(unsigned)id] = behavior;
+    /// Set whether a diagnostic should be ignored.
+    void setIgnoredDiagnostic(DiagID id, bool ignored) {
+      ignoredDiagnostics[(unsigned)id] = ignored;
     }
 
   private:
@@ -810,7 +818,7 @@ namespace swift {
     }
 
     void ignoreDiagnostic(DiagID id) {
-      state.setDiagnosticBehavior(id, DiagnosticState::Behavior::Ignore);
+      state.setIgnoredDiagnostic(id, true);
     }
 
     void resetHadAnyError() {
@@ -1106,9 +1114,9 @@ namespace swift {
                                        Engine.TentativeDiagnostics.end());
 
       for (auto &diagnostic : diagnostics) {
-        auto behavior = Engine.state.determineBehavior(diagnostic.getID());
-        if (behavior == DiagnosticState::Behavior::Fatal ||
-            behavior == DiagnosticState::Behavior::Error)
+        auto behavior = Engine.state.determineBehavior(diagnostic);
+        if (behavior == DiagnosticBehavior::Fatal ||
+            behavior == DiagnosticBehavior::Error)
           return true;
       }
 
diff --git a/include/swift/AST/DiagnosticsSema.def b/include/swift/AST/DiagnosticsSema.def
@@ -4375,38 +4375,20 @@ ERROR(non_concurrent_type_member,none,
       "%select{stored property %1|associated value %1}0 of "
       "'ConcurrentValue'-conforming %2 %3 has non-concurrent-value type %4",
       (bool, DeclName, DescriptiveDeclKind, DeclName, Type))
-WARNING(non_concurrent_type_member_warn,none,
-      "%select{stored property %1|associated value %1}0 of "
-      "'ConcurrentValue'-conforming %2 %3 has non-concurrent-value type %4",
-      (bool, DeclName, DescriptiveDeclKind, DeclName, Type))
 ERROR(concurrent_value_class_mutable_property,none,
       "stored property %0 of 'ConcurrentValue'-conforming %1 %2 is mutable",
       (DeclName, DescriptiveDeclKind, DeclName))
-WARNING(concurrent_value_class_mutable_property_warn,none,
-      "stored property %0 of 'ConcurrentValue'-conforming %1 %2 is mutable",
-      (DeclName, DescriptiveDeclKind, DeclName))
 ERROR(concurrent_value_outside_source_file,none,
       "conformance to 'ConcurrentValue' must occur in the same source file as "
       "%0 %1; use 'UnsafeConcurrentValue' for retroactive conformance",
       (DescriptiveDeclKind, DeclName))
-WARNING(concurrent_value_outside_source_file_warn,none,
-      "conformance to 'ConcurrentValue' must occur in the same source file as "
-      "%0 %1; use 'UnsafeConcurrentValue' for retroactive conformance",
-      (DescriptiveDeclKind, DeclName))
 ERROR(concurrent_value_nonfinal_class,none,
       "non-final class %0 cannot conform to `ConcurrentValue`; "
       "use `UnsafeConcurrentValue`", (DeclName))
-WARNING(concurrent_value_nonfinal_class_warn,none,
-      "non-final class %0 cannot conform to `ConcurrentValue`; "
-      "use `UnsafeConcurrentValue`", (DeclName))
 ERROR(concurrent_value_inherit,none,
       "`ConcurrentValue` class %1 cannot inherit from another class"
       "%select{| other than 'NSObject'}0",
       (bool, DeclName))
-WARNING(concurrent_value_inherit_warn,none,
-        "`ConcurrentValue` class %1 cannot inherit from another class"
-        "%select{| other than 'NSObject'}0",
-        (bool, DeclName))
 
 ERROR(actorindependent_let,none,
       "'@actorIndependent' is meaningless on 'let' declarations because "
diff --git a/lib/AST/DiagnosticEngine.cpp b/lib/AST/DiagnosticEngine.cpp
@@ -136,8 +136,8 @@ static constexpr EducationalNotes<LocalDiagID::NumDiags> _EducationalNotes = Edu
 static constexpr auto educationalNotes = _EducationalNotes.value;
 
 DiagnosticState::DiagnosticState() {
-  // Initialize our per-diagnostic state to default
-  perDiagnosticBehavior.resize(LocalDiagID::NumDiags, Behavior::Unspecified);
+  // Initialize our ignored diagnostics to default
+  ignoredDiagnostics.resize(LocalDiagID::NumDiags);
 }
 
 static CharSourceRange toCharSourceRange(SourceManager &SM, SourceRange SR) {
@@ -289,6 +289,12 @@ InFlightDiagnostic &InFlightDiagnostic::fixItExchange(SourceRange R1,
   return *this;
 }
 
+InFlightDiagnostic &
+InFlightDiagnostic::limitBehavior(DiagnosticBehavior limit) {
+  Engine->getActiveDiagnostic().setBehaviorLimit(limit);
+  return *this;
+}
+
 void InFlightDiagnostic::flush() {
   if (!IsActive)
     return;
@@ -779,26 +785,41 @@ void DiagnosticEngine::formatDiagnosticText(
   }
 }
 
-static DiagnosticKind toDiagnosticKind(DiagnosticState::Behavior behavior) {
+static DiagnosticKind toDiagnosticKind(DiagnosticBehavior behavior) {
   switch (behavior) {
-  case DiagnosticState::Behavior::Unspecified:
+  case DiagnosticBehavior::Unspecified:
     llvm_unreachable("unspecified behavior");
-  case DiagnosticState::Behavior::Ignore:
+  case DiagnosticBehavior::Ignore:
     llvm_unreachable("trying to map an ignored diagnostic");
-  case DiagnosticState::Behavior::Error:
-  case DiagnosticState::Behavior::Fatal:
+  case DiagnosticBehavior::Error:
+  case DiagnosticBehavior::Fatal:
     return DiagnosticKind::Error;
-  case DiagnosticState::Behavior::Note:
+  case DiagnosticBehavior::Note:
     return DiagnosticKind::Note;
-  case DiagnosticState::Behavior::Warning:
+  case DiagnosticBehavior::Warning:
     return DiagnosticKind::Warning;
-  case DiagnosticState::Behavior::Remark:
+  case DiagnosticBehavior::Remark:
     return DiagnosticKind::Remark;
   }
 
   llvm_unreachable("Unhandled DiagnosticKind in switch.");
 }
 
+static
+DiagnosticBehavior toDiagnosticBehavior(DiagnosticKind kind, bool isFatal) {
+  switch (kind) {
+  case DiagnosticKind::Note:
+    return DiagnosticBehavior::Note;
+  case DiagnosticKind::Error:
+    return isFatal ? DiagnosticBehavior::Fatal : DiagnosticBehavior::Error;
+  case DiagnosticKind::Warning:
+    return DiagnosticBehavior::Warning;
+  case DiagnosticKind::Remark:
+    return DiagnosticBehavior::Remark;
+  }
+  llvm_unreachable("Unhandled DiagnosticKind in switch.");
+}
+
 // A special option only for compiler writers that causes Diagnostics to assert
 // when a failure diagnostic is emitted. Intended for use in the debugger.
 llvm::cl::opt<bool> AssertOnError("swift-diagnostics-assert-on-error",
@@ -808,72 +829,63 @@ llvm::cl::opt<bool> AssertOnError("swift-diagnostics-assert-on-error",
 llvm::cl::opt<bool> AssertOnWarning("swift-diagnostics-assert-on-warning",
                                     llvm::cl::init(false));
 
-DiagnosticState::Behavior DiagnosticState::determineBehavior(DiagID id) {
-  auto set = [this](DiagnosticState::Behavior lvl) {
-    if (lvl == Behavior::Fatal) {
-      fatalErrorOccurred = true;
-      anyErrorOccurred = true;
-    } else if (lvl == Behavior::Error) {
-      anyErrorOccurred = true;
-    }
-
-    assert((!AssertOnError || !anyErrorOccurred) && "We emitted an error?!");
-    assert((!AssertOnWarning || (lvl != Behavior::Warning)) &&
-           "We emitted a warning?!");
-    previousBehavior = lvl;
-    return lvl;
-  };
-
+DiagnosticBehavior DiagnosticState::determineBehavior(const Diagnostic &diag) {
   // We determine how to handle a diagnostic based on the following rules
-  //   1) If current state dictates a certain behavior, follow that
-  //   2) If the user provided a behavior for this specific diagnostic, follow
-  //      that
-  //   3) If the user provided a behavior for this diagnostic's kind, follow
-  //      that
-  //   4) Otherwise remap the diagnostic kind
-
-  auto diagInfo = storedDiagnosticInfos[(unsigned)id];
-  bool isNote = diagInfo.kind == DiagnosticKind::Note;
-
-  //   1) If current state dictates a certain behavior, follow that
+  //   1) Map the diagnostic to its "intended" behavior, applying the behavior
+  //      limit for this particular emission
+  //   2) If current state dictates a certain behavior, follow that
+  //   3) If the user ignored this specific diagnostic, follow that
+  //   4) If the user substituted a different behavior for this behavior, apply
+  //      that change
+  //   5) Update current state for use during the next diagnostic
+
+  //   1) Map the diagnostic to its "intended" behavior, applying the behavior
+  //      limit for this particular emission
+  auto diagInfo = storedDiagnosticInfos[(unsigned)diag.getID()];
+  DiagnosticBehavior lvl =
+      std::max(toDiagnosticBehavior(diagInfo.kind, diagInfo.isFatal),
+               diag.getBehaviorLimit());
+  assert(lvl != DiagnosticBehavior::Unspecified);
+
+  //   2) If current state dictates a certain behavior, follow that
 
   // Notes relating to ignored diagnostics should also be ignored
-  if (previousBehavior == Behavior::Ignore && isNote)
-    return set(Behavior::Ignore);
+  if (previousBehavior == DiagnosticBehavior::Ignore
+      && lvl == DiagnosticBehavior::Note)
+    lvl = DiagnosticBehavior::Ignore;
 
   // Suppress diagnostics when in a fatal state, except for follow-on notes
   if (fatalErrorOccurred)
-    if (!showDiagnosticsAfterFatalError && !isNote)
-      return set(Behavior::Ignore);
+    if (!showDiagnosticsAfterFatalError && lvl != DiagnosticBehavior::Note)
+      lvl = DiagnosticBehavior::Ignore;
 
-  //   2) If the user provided a behavior for this specific diagnostic, follow
-  //      that
+  //   3) If the user ignored this specific diagnostic, follow that
+  if (ignoredDiagnostics[(unsigned)diag.getID()])
+    lvl = DiagnosticBehavior::Ignore;
 
-  if (perDiagnosticBehavior[(unsigned)id] != Behavior::Unspecified)
-    return set(perDiagnosticBehavior[(unsigned)id]);
-
-  //   3) If the user provided a behavior for this diagnostic's kind, follow
-  //      that
-  if (diagInfo.kind == DiagnosticKind::Warning) {
-    if (suppressWarnings)
-      return set(Behavior::Ignore);
+  //   4) If the user substituted a different behavior for this behavior, apply
+  //      that change
+  if (lvl == DiagnosticBehavior::Warning) {
     if (warningsAsErrors)
-      return set(Behavior::Error);
+      lvl = DiagnosticBehavior::Error;
+    if (suppressWarnings)
+      lvl = DiagnosticBehavior::Ignore;
   }
 
-  //   4) Otherwise remap the diagnostic kind
-  switch (diagInfo.kind) {
-  case DiagnosticKind::Note:
-    return set(Behavior::Note);
-  case DiagnosticKind::Error:
-    return set(diagInfo.isFatal ? Behavior::Fatal : Behavior::Error);
-  case DiagnosticKind::Warning:
-    return set(Behavior::Warning);
-  case DiagnosticKind::Remark:
-    return set(Behavior::Remark);
+  //   5) Update current state for use during the next diagnostic
+  if (lvl == DiagnosticBehavior::Fatal) {
+    fatalErrorOccurred = true;
+    anyErrorOccurred = true;
+  } else if (lvl == DiagnosticBehavior::Error) {
+    anyErrorOccurred = true;
   }
 
-  llvm_unreachable("Unhandled DiagnosticKind in switch.");
+  assert((!AssertOnError || !anyErrorOccurred) && "We emitted an error?!");
+  assert((!AssertOnWarning || (lvl != DiagnosticBehavior::Warning)) &&
+         "We emitted a warning?!");
+
+  previousBehavior = lvl;
+  return lvl;
 }
 
 void DiagnosticEngine::flushActiveDiagnostic() {
@@ -910,8 +922,8 @@ static AccessLevel getBufferAccessLevel(const Decl *decl) {
 
 Optional<DiagnosticInfo>
 DiagnosticEngine::diagnosticInfoForDiagnostic(const Diagnostic &diagnostic) {
-  auto behavior = state.determineBehavior(diagnostic.getID());
-  if (behavior == DiagnosticState::Behavior::Ignore)
+  auto behavior = state.determineBehavior(diagnostic);
+  if (behavior == DiagnosticBehavior::Ignore)
     return None;
 
   // Figure out the source location.
diff --git a/lib/Sema/TypeCheckConcurrency.cpp b/lib/Sema/TypeCheckConcurrency.cpp
