Add option to control access note diagnostics

This will allow teams writing access notes to use -Raccess-note=all-validate to check that their access notes are correct, or teams working around problems to use -Raccess-note=failures or -Raccess-note=none to suppress diagnostics.

Author: beccadax

include/swift/AST/DiagnosticsSema.def

@@ -5873,16 +5873,18 @@ REMARK(attr_objc_name_changed_by_access_note, none,
 NOTE(fixit_attr_objc_name_changed_by_access_note, none,
      "change '@objc' name in source code explicitly to silence this warning",
      ())
-REMARK(attr_objc_name_conflicts_with_access_note, none,
-       "access note for %0 changes the '@objc' name of this %1 to %2, but "
-       "source code specifies %3; the access note will be ignored",
-       (StringRef, DescriptiveDeclKind, ObjCSelector, ObjCSelector))
-
-// Used on invalid @objc diagnostics
-REMARK(wrap_invalid_attr_added_by_access_note, none,
-       "access note for %1 failed to add invalid "
-       "%select{attribute|modifier}2 '%3': %0",
-       (DiagnosticInfo *, StringRef, bool, StringRef))
+
+// Bad access note diagnostics. These are usually emitted as remarks, but
+// '-Raccess-note=all-validate' emits them as errors.
+
+ERROR(attr_objc_name_conflicts_with_access_note, none,
+      "access note for %0 changes the '@objc' name of this %1 to %2, but "
+      "source code specifies %3; the access note will be ignored",
+      (StringRef, DescriptiveDeclKind, ObjCSelector, ObjCSelector))
+ERROR(wrap_invalid_attr_added_by_access_note, none,
+      "access note for %1 failed to add invalid "
+      "%select{attribute|modifier}2 '%3': %0",
+      (DiagnosticInfo *, StringRef, bool, StringRef))
 
 #define UNDEFINE_DIAGNOSTIC_MACROS
 #include "DefineDiagnosticMacros.h"

include/swift/Basic/LangOptions.h

@@ -36,6 +36,8 @@
 
 namespace swift {
 
+  enum class DiagnosticBehavior : uint8_t;
+
   /// Kind of implicit platform conditions.
   enum class PlatformConditionKind {
 #define PLATFORM_CONDITION(LABEL, IDENTIFIER) LABEL,
@@ -69,6 +71,13 @@ namespace swift {
     Other
   };
 
+  enum class AccessNoteDiagnosticBehavior : uint8_t {
+    Ignore,
+    RemarkOnFailure,
+    RemarkOnFailureOrSuccess,
+    ErrorOnFailureRemarkOnSuccess
+  };
+
   /// A collection of options that affect the language dialect and
   /// provide compiler debugging facilities.
   class LangOptions final {
@@ -339,6 +348,17 @@ namespace swift {
     std::shared_ptr<llvm::Regex> OptimizationRemarkPassedPattern;
     std::shared_ptr<llvm::Regex> OptimizationRemarkMissedPattern;
 
+    /// How should we emit diagnostics about access notes?
+    AccessNoteDiagnosticBehavior AccessNoteBehavior =
+        AccessNoteDiagnosticBehavior::RemarkOnFailureOrSuccess;
+
+    DiagnosticBehavior getAccessNoteFailureLimit() const;
+
+    bool shouldRemarkOnAccessNoteSuccess() const {
+      return AccessNoteBehavior >=
+          AccessNoteDiagnosticBehavior::RemarkOnFailureOrSuccess;
+    }
+
     /// Whether collect tokens during parsing for syntax coloring.
     bool CollectParsedToken = false;
 

include/swift/Option/FrontendOptions.td

@@ -196,6 +196,12 @@ def enable_infer_public_concurrent_value : Flag<["-"], "enable-infer-public-send
 def disable_infer_public_concurrent_value : Flag<["-"], "disable-infer-public-sendable">,
     HelpText<"Disable inference of Sendable conformances for public structs and enums">;
 
+def Raccess_note : Separate<["-"], "Raccess-note">,
+    MetaVarName<"none|failures|all|all-validate">,
+    HelpText<"Control access note remarks (default: all)">;
+def Raccess_note_EQ : Joined<["-"], "Raccess-note=">,
+    Alias<Raccess_note>;
+
 } // end let Flags = [FrontendOption, NoDriverOption]
 
 def debug_crash_Group : OptionGroup<"<automatic crashing options>">;

lib/Basic/LangOptions.cpp

@@ -16,6 +16,7 @@
 //===----------------------------------------------------------------------===//
 
 #include "swift/Basic/LangOptions.h"
+#include "swift/AST/DiagnosticEngine.h"
 #include "swift/Basic/Feature.h"
 #include "swift/Basic/Platform.h"
 #include "swift/Basic/Range.h"
@@ -391,3 +392,17 @@ llvm::StringRef swift::getFeatureName(Feature feature) {
 #include "swift/Basic/Features.def"
   }
 }
+
+DiagnosticBehavior LangOptions::getAccessNoteFailureLimit() const {
+  switch (AccessNoteBehavior) {
+  case AccessNoteDiagnosticBehavior::Ignore:
+    return DiagnosticBehavior::Ignore;
+
+  case AccessNoteDiagnosticBehavior::RemarkOnFailure:
+  case AccessNoteDiagnosticBehavior::RemarkOnFailureOrSuccess:
+    return DiagnosticBehavior::Remark;
+
+  case AccessNoteDiagnosticBehavior::ErrorOnFailureRemarkOnSuccess:
+    return DiagnosticBehavior::Error;
+  }
+}

lib/Frontend/CompilerInvocation.cpp

@@ -609,6 +609,21 @@ static bool ParseLangArgs(LangOptions &Opts, ArgList &Args,
     Opts.OptimizationRemarkMissedPattern =
         generateOptimizationRemarkRegex(Diags, Args, A);
 
+  if (Arg *A = Args.getLastArg(OPT_Raccess_note)) {
+    auto value = llvm::StringSwitch<Optional<AccessNoteDiagnosticBehavior>>(A->getValue())
+      .Case("none", AccessNoteDiagnosticBehavior::Ignore)
+      .Case("failures", AccessNoteDiagnosticBehavior::RemarkOnFailure)
+      .Case("all", AccessNoteDiagnosticBehavior::RemarkOnFailureOrSuccess)
+      .Case("all-validate", AccessNoteDiagnosticBehavior::ErrorOnFailureRemarkOnSuccess)
+      .Default(None);
+
+    if (value)
+      Opts.AccessNoteBehavior = *value;
+    else
+      Diags.diagnose(SourceLoc(), diag::error_invalid_arg_value,
+                     A->getAsString(Args), A->getValue());
+  }
+
   Opts.EnableConcisePoundFile =
       Args.hasArg(OPT_enable_experimental_concise_pound_file);
   Opts.EnableFuzzyForwardScanTrailingClosureMatching =

lib/Sema/TypeCheckDeclObjC.cpp

@@ -55,7 +55,7 @@ swift::behaviorLimitForObjCReason(ObjCReason reason, ASTContext &ctx) {
     return DiagnosticBehavior::Ignore;
 
   case ObjCReason::ExplicitlyObjCByAccessNote:
-    return DiagnosticBehavior::Remark;
+    return ctx.LangOpts.getAccessNoteFailureLimit();
 
   case ObjCReason::MemberOfObjCSubclass:
   case ObjCReason::MemberOfObjCMembersClass:

lib/Sema/TypeCheckDeclPrimary.cpp

@@ -1456,9 +1456,10 @@ static void addOrRemoveAttr(ValueDecl *VD, const AccessNotesFile &notes,
       SF->AttrsAddedByAccessNotes.emplace_back(VD, attr);
   } else {
     VD->getAttrs().removeAttribute(attr);
-    diagnoseChangeByAccessNote(diag::attr_removed_by_access_note,
-                               diag::fixit_attr_removed_by_access_note)
-      .fixItRemove(attr->getRangeWithAt());
+    if (VD->getASTContext().LangOpts.shouldRemarkOnAccessNoteSuccess())
+      diagnoseChangeByAccessNote(diag::attr_removed_by_access_note,
+                                 diag::fixit_attr_removed_by_access_note)
+        .fixItRemove(attr->getRangeWithAt());
   }
 }
 
@@ -1468,9 +1469,11 @@ swift::softenIfAccessNote(const Decl *D, const DeclAttribute *attr,
   if (!attr || !attr->getAddedByAccessNote())
     return std::move(diag);
 
+  auto behavior = D->getASTContext().LangOpts.getAccessNoteFailureLimit();
   return std::move(diag.wrapIn(diag::wrap_invalid_attr_added_by_access_note,
                                D->getModuleContext()->getAccessNotes().Reason,
-                               attr->isDeclModifier(), attr->getAttrName()));
+                               attr->isDeclModifier(), attr->getAttrName())
+                        .limitBehavior(behavior));
 }
 
 static void applyAccessNote(ValueDecl *VD, const AccessNote &note,
@@ -1491,6 +1494,10 @@ static void applyAccessNote(ValueDecl *VD, const AccessNote &note,
 
     if (!attr->hasName()) {
       attr->setName(*note.ObjCName, true);
+
+      if (!ctx.LangOpts.shouldRemarkOnAccessNoteSuccess())
+        return;
+
       diagnoseAtAttrOrDecl(attr, VD,
                            diag::attr_objc_name_changed_by_access_note,
                            notes.Reason, VD->getDescriptiveKind(),
@@ -1512,10 +1519,12 @@ static void applyAccessNote(ValueDecl *VD, const AccessNote &note,
         note.fixItInsertAfter(attr->getLocation(), newNameString);
     }
     else if (attr->getName() != *note.ObjCName) {
+      auto behavior = ctx.LangOpts.getAccessNoteFailureLimit();
       diagnoseAtAttrOrDecl(attr, VD,
                            diag::attr_objc_name_conflicts_with_access_note,
                            notes.Reason, VD->getDescriptiveKind(),
-                           *note.ObjCName, *attr->getName());
+                           *note.ObjCName, *attr->getName())
+          .limitBehavior(behavior);
     }
   }
 }
@@ -1526,6 +1535,9 @@ void TypeChecker::applyAccessNote(ValueDecl *VD) {
 }
 
 void swift::diagnoseAttrsAddedByAccessNote(SourceFile &SF) {
+  if (!SF.getASTContext().LangOpts.shouldRemarkOnAccessNoteSuccess())
+    return;
+
   for (auto declAndAttr : SF.AttrsAddedByAccessNotes) {
     auto VD = std::get<0>(declAndAttr);
     auto attr = std::get<1>(declAndAttr);

test/Sema/Inputs/extra.accessnotes

@@ -5,3 +5,9 @@ Notes:
 - Name: 'fn()'
   CorinthianLeather: 'rich'
   # expected-remark@-1 {{ignored invalid content in access notes file: unknown key 'CorinthianLeather'}}
+  
+# These notes should apply, or attempt to apply, despite the other problems.
+- Name: 'Extant.good(_:)'
+  ObjC: true
+- Name: 'Extant.bad(_:)'
+  ObjC: true

test/Sema/access-notes-invalid.swift

@@ -5,6 +5,31 @@
 
 // RUN: %target-typecheck-verify-swift -access-notes-path %S/Inputs/extra.accessnotes -verify-additional-file %S/Inputs/extra.accessnotes
 
+// RUN: %target-swift-frontend -typecheck -primary-file %/s -access-notes-path %/S/Inputs/extra.accessnotes -Raccess-note=none 2>&1 | %FileCheck --check-prefixes=GOOD-IGNORE,BAD-IGNORE %s
+// RUN: %target-swift-frontend -typecheck -primary-file %/s -access-notes-path %/S/Inputs/extra.accessnotes -Raccess-note=failures 2>&1 | %FileCheck --check-prefixes=GOOD-IGNORE,BAD-REMARK %s
+// RUN: %target-swift-frontend -typecheck -primary-file %/s -access-notes-path %/S/Inputs/extra.accessnotes -Raccess-note=all 2>&1 | %FileCheck --check-prefixes=GOOD-REMARK,BAD-REMARK %s
+// RUN: not %target-swift-frontend -typecheck -primary-file %/s -access-notes-path %/S/Inputs/extra.accessnotes -Raccess-note=all-validate 2>&1 | %FileCheck --check-prefixes=GOOD-REMARK,BAD-ERROR %s
+
+// Default should be 'all'.
+// RUN: %target-swift-frontend -typecheck -primary-file %/s -access-notes-path %/S/Inputs/extra.accessnotes 2>&1 | %FileCheck --check-prefixes=GOOD-REMARK,BAD-REMARK %s
+
+class Extant {
+  func good(_: Int) {} // expected-remark * {{}} expected-note * {{}}
+  // GOOD-IGNORE-NOT: access-notes-invalid.swift:[[@LINE-1]]:{{[0-9]+}}: remark: access note for Access notes containing future, unknown syntax adds attribute 'objc' to this instance method
+  // GOOD-IGNORE-NOT: access-notes-invalid.swift:[[@LINE-2]]:{{[0-9]+}}: note: add attribute explicitly to silence this warning
+  // GOOD-REMARK-DAG: access-notes-invalid.swift:[[@LINE-3]]:{{[0-9]+}}: remark: access note for Access notes containing future, unknown syntax adds attribute 'objc' to this instance method
+  // GOOD-REMARK-DAG: access-notes-invalid.swift:[[@LINE-4]]:{{[0-9]+}}: note: add attribute explicitly to silence this warning
+
+  func bad(_: Int?) {} // expected-remark * {{}}
+  // BAD-IGNORE-NOT: access-notes-invalid.swift:[[@LINE-1]]:{{[0-9]+}}: remark: access note for Access notes containing future, unknown syntax failed to add invalid attribute 'objc': method cannot be marked @objc by an access note because the type of the parameter cannot be represented in Objective-C
+  // BAD-IGNORE-NOT: access-notes-invalid.swift:[[@LINE-2]]:{{[0-9]+}}: error: access note for Access notes containing future, unknown syntax failed to add invalid attribute 'objc': method cannot be marked @objc by an access note because the type of the parameter cannot be represented in Objective-C
+  // BAD-REMARK-DAG: access-notes-invalid.swift:[[@LINE-3]]:{{[0-9]+}}: remark: access note for Access notes containing future, unknown syntax failed to add invalid attribute 'objc': method cannot be marked @objc by an access note because the type of the parameter cannot be represented in Objective-C
+  // BAD-ERROR-DAG: access-notes-invalid.swift:[[@LINE-4]]:{{[0-9]+}}: error: access note for Access notes containing future, unknown syntax failed to add invalid attribute 'objc': method cannot be marked @objc by an access note because the type of the parameter cannot be represented in Objective-C
+}
+
 // FIXME: Should diagnose multiple access notes for the same decl
 
 // FIXME: Should diagnose access notes that don't match a decl in the source code
+
+// Only valid on platforms where @objc is valid.
+// REQUIRES: objc_interop