From 976f1494f51226ca65a3fd91f6979e7bf6acd72f Mon Sep 17 00:00:00 2001
From: John McCall <rjmccall@apple.com>
Date: Fri, 1 Mar 2024 01:30:33 -0500
Subject: [PATCH 1/3] [NFC] Introduce DiagRef and use it throughout the parser

The goal is to have a lightweight way to pass an unapplied
diagnostic to general routines.  Constructing a Diagnostic
is quite expensive as something we're potentially doing in
hot paths, as opposed to just when we're actually emitting
the diagnostic.  This design allows the expense to be delayed
until we need it.

I've also optimized the Diagnostic constructor to avoid
copying arguments unnecessarily; this is a relatively small
expense, since arguments are POD, but there's really no good
reason not to do it.
---
 include/swift/AST/DiagnosticEngine.h | 126 +++++++++++++++++++++++++--
 include/swift/Parse/Parser.h         |  38 ++++----
 lib/Parse/ParseDecl.cpp              |  14 +--
 lib/Parse/ParseExpr.cpp              |   4 +-
 lib/Parse/Parser.cpp                 |  15 ++--
 lib/SIL/Parser/ParseSIL.cpp          |   8 +-
 lib/SIL/Parser/SILParser.h           |  13 ++-
 7 files changed, 166 insertions(+), 52 deletions(-)

diff --git a/include/swift/AST/DiagnosticEngine.h b/include/swift/AST/DiagnosticEngine.h
index 6151f0dc8f47a..5dedbbf5bba63 100644
--- a/include/swift/AST/DiagnosticEngine.h
+++ b/include/swift/AST/DiagnosticEngine.h
@@ -88,6 +88,10 @@ namespace swift {
     };
   }
 
+  template <class... ArgTypes>
+  using DiagArgTuple =
+    std::tuple<typename detail::PassArgument<ArgTypes>::type...>;
+
   /// A family of wrapper types for compiler data types that forces its
   /// underlying data to be formatted with full qualification.
   ///
@@ -476,20 +480,28 @@ namespace swift {
     friend DiagnosticEngine;
     friend class InFlightDiagnostic;
 
+    Diagnostic(DiagID ID) : ID(ID) {}
+
   public:
     // All constructors are intentionally implicit.
     template<typename ...ArgTypes>
     Diagnostic(Diag<ArgTypes...> ID,
                typename detail::PassArgument<ArgTypes>::type... VArgs)
-      : ID(ID.ID) {
-      DiagnosticArgument DiagArgs[] = {
-        DiagnosticArgument(0), std::move(VArgs)... 
-      };
-      Args.append(DiagArgs + 1, DiagArgs + 1 + sizeof...(VArgs));
+        : Diagnostic(ID.ID) {
+      Args.reserve(sizeof...(ArgTypes));
+      gatherArgs(VArgs...);
     }
 
     /*implicit*/Diagnostic(DiagID ID, ArrayRef<DiagnosticArgument> Args)
       : ID(ID), Args(Args.begin(), Args.end()) {}
+
+    template <class... ArgTypes>
+    static Diagnostic fromTuple(Diag<ArgTypes...> id,
+                                const DiagArgTuple<ArgTypes...> &tuple) {
+      Diagnostic result(id.ID);
+      result.gatherArgsFromTuple<DiagArgTuple<ArgTypes...>, 0, ArgTypes...>(tuple);
+      return result;
+    }
     
     // Accessors.
     DiagID getID() const { return ID; }
@@ -528,6 +540,37 @@ namespace swift {
 
     void addChildNote(Diagnostic &&D);
     void insertChildNote(unsigned beforeIndex, Diagnostic &&D);
+
+  private:
+    // gatherArgs could just be `Args.emplace_back(args)...;` if C++
+    // allowed pack expansions in statement context.
+
+    // Base case.
+    void gatherArgs() {}
+
+    // Pull one off the pack.
+    template <class ArgType, class... RemainingArgTypes>
+    void gatherArgs(ArgType arg, RemainingArgTypes... remainingArgs) {
+      Args.emplace_back(arg);
+      gatherArgs(remainingArgs...);
+    }
+
+    // gatherArgsFromTuple could just be
+    // `Args.emplace_back(std::get<packIndexOf<ArgTypes>>(tuple))...;`
+    // in a better world.
+
+    // Base case.
+    template <class Tuple, size_t Index>
+    void gatherArgsFromTuple(const Tuple &tuple) {}
+
+    // Pull one off the pack.
+    template <class Tuple, size_t Index,
+              class ArgType, class... RemainingArgTypes>
+    void gatherArgsFromTuple(const Tuple &tuple) {
+      Args.emplace_back(std::move(std::get<Index>(tuple)));
+      gatherArgsFromTuple<Tuple, Index + 1, RemainingArgTypes...>(
+        std::move(tuple));
+    }
   };
 
   /// A diagnostic that has no input arguments, so it is trivially-destructable.
@@ -866,6 +909,73 @@ namespace swift {
     DiagnosticState &operator=(DiagnosticState &&) = default;
   };
 
+  /// A lightweight reference to a diagnostic that's been fully applied to
+  /// its arguments.  This allows a general routine (in the parser, say) to
+  /// be customized to emit an arbitrary diagnostic without needing to
+  /// eagerly construct a full Diagnostic.  Like ArrayRef and function_ref,
+  /// this stores a reference to what's likely to be a temporary, so it
+  /// should only be used as a function parameter.  If you need to persist
+  /// the diagnostic, you'll have to call createDiagnostic().
+  ///
+  /// You can initialize a DiagRef parameter in one of two ways:
+  /// - passing a Diag<> as the argument, e.g.
+  ///      diag::circular_reference
+  ///   or
+  /// - constructing it with a Diag and its arguments, e.g.
+  ///      {diag::circular_protocol_def, {proto->getName()}}
+  ///
+  /// It'd be nice to let people write `{diag::my_error, arg0, arg1}`
+  /// instead of `{diag::my_error, {arg0, arg1}}`, but we can't: the
+  /// temporary needs to be created in the calling context.
+  class DiagRef {
+    DiagID id;
+
+    /// If this is null, then id is a Diag<> and there are no arguments.
+    Diagnostic (*createFn)(DiagID id, const void *opaqueStorage);
+    const void *opaqueStorage;
+
+  public:
+    /// Construct a diagnostic from a diagnostic ID that's known to not take
+    /// arguments.
+    DiagRef(Diag<> id)
+      : id(id.ID), createFn(nullptr), opaqueStorage(nullptr) {}
+
+    /// Construct a diagnostic from a diagnostic ID and its arguments.
+    template <class... ArgTypes>
+    DiagRef(Diag<ArgTypes...> id, const DiagArgTuple<ArgTypes...> &tuple)
+      : id(id.ID),
+        createFn(&createFromTuple<ArgTypes...>),
+        opaqueStorage(&tuple) {}
+
+    // A specialization of the general constructor above for diagnostics
+    // with no arguments; this is a useful optimization when a DiagRef
+    // is constructed generically.
+    DiagRef(Diag<> id, const DiagArgTuple<> &tuple)
+      : DiagRef(id) {}
+
+    /// Return the diagnostic ID that this will emit.
+    DiagID getID() const {
+      return id;
+    }
+
+    /// Create a full Diagnostic.  It's safe to do this multiple times on
+    /// a single DiagRef.
+    Diagnostic createDiagnostic() {
+      if (!createFn) {
+        return Diagnostic(Diag<> {id});
+      } else {
+        return createFn(id, opaqueStorage);
+      }
+    }
+
+  private:
+    template <class... ArgTypes>
+    static Diagnostic createFromTuple(DiagID id, const void *opaqueStorage) {
+      auto tuple = static_cast<const DiagArgTuple<ArgTypes...> *>(opaqueStorage);
+      return Diagnostic::fromTuple(Diag<ArgTypes...> {id}, *tuple);
+    }
+  };
+
   /// Class responsible for formatting diagnostics and presenting them
   /// to the user.
   class DiagnosticEngine {
@@ -1113,6 +1223,12 @@ namespace swift {
       return diagnose(Loc, Diagnostic(ID, std::move(Args)...));
     }
 
+    /// Emit the given lazily-applied diagnostic at the specified
+    /// source location.
+    InFlightDiagnostic diagnose(SourceLoc loc, DiagRef diag) {
+      return diagnose(loc, diag.createDiagnostic());
+    }
+
     /// Delete an API that may lead clients to avoid specifying source location.
     template<typename ...ArgTypes>
     InFlightDiagnostic
diff --git a/include/swift/Parse/Parser.h b/include/swift/Parse/Parser.h
index c588e11018fc4..39a2671335f63 100644
--- a/include/swift/Parse/Parser.h
+++ b/include/swift/Parse/Parser.h
@@ -729,28 +729,28 @@ class Parser {
   }
 
 public:
-  InFlightDiagnostic diagnose(SourceLoc Loc, Diagnostic Diag) {
+  InFlightDiagnostic diagnose(SourceLoc Loc, DiagRef Diag) {
     if (Diags.isDiagnosticPointsToFirstBadToken(Diag.getID()) &&
         Loc == Tok.getLoc() && Tok.isAtStartOfLine())
       Loc = getEndOfPreviousLoc();
     return Diags.diagnose(Loc, Diag);
   }
 
-  InFlightDiagnostic diagnose(Token Tok, Diagnostic Diag) {
+  InFlightDiagnostic diagnose(Token Tok, DiagRef Diag) {
     return diagnose(Tok.getLoc(), Diag);
   }
 
   template<typename ...DiagArgTypes, typename ...ArgTypes>
   InFlightDiagnostic diagnose(SourceLoc Loc, Diag<DiagArgTypes...> DiagID,
                               ArgTypes &&...Args) {
-    return diagnose(Loc, Diagnostic(DiagID, std::forward<ArgTypes>(Args)...));
+    return diagnose(Loc, {DiagID, {std::forward<ArgTypes>(Args)...}});
   }
 
   template<typename ...DiagArgTypes, typename ...ArgTypes>
   InFlightDiagnostic diagnose(Token Tok, Diag<DiagArgTypes...> DiagID,
                               ArgTypes &&...Args) {
     return diagnose(Tok.getLoc(),
-                    Diagnostic(DiagID, std::forward<ArgTypes>(Args)...));
+                    {DiagID, {std::forward<ArgTypes>(Args)...}});
   }
     
   /// Add a fix-it to remove the space in consecutive identifiers.
@@ -809,19 +809,19 @@ class Parser {
   /// its name in \p Result.  Otherwise, emit an error.
   ///
   /// \returns false on success, true on error.
-  bool parseIdentifier(Identifier &Result, SourceLoc &Loc, const Diagnostic &D,
+  bool parseIdentifier(Identifier &Result, SourceLoc &Loc, DiagRef D,
                        bool diagnoseDollarPrefix);
 
   /// Consume an identifier with a specific expected name.  This is useful for
   /// contextually sensitive keywords that must always be present.
   bool parseSpecificIdentifier(StringRef expected, SourceLoc &Loc,
-                               const Diagnostic &D);
+                               DiagRef D);
 
   template<typename ...DiagArgTypes, typename ...ArgTypes>
   bool parseIdentifier(Identifier &Result, SourceLoc &L,
                        bool diagnoseDollarPrefix, Diag<DiagArgTypes...> ID,
                        ArgTypes... Args) {
-    return parseIdentifier(Result, L, Diagnostic(ID, Args...),
+    return parseIdentifier(Result, L, {ID, {Args...}},
                            diagnoseDollarPrefix);
   }
   
@@ -829,19 +829,19 @@ class Parser {
   bool parseSpecificIdentifier(StringRef expected,
                                Diag<DiagArgTypes...> ID, ArgTypes... Args) {
     SourceLoc L;
-    return parseSpecificIdentifier(expected, L, Diagnostic(ID, Args...));
+    return parseSpecificIdentifier(expected, L, {ID, {Args...}});
   }
 
   /// Consume an identifier or operator if present and return its name
   /// in \p Result.  Otherwise, emit an error and return true.
   bool parseAnyIdentifier(Identifier &Result, SourceLoc &Loc,
-                          const Diagnostic &D, bool diagnoseDollarPrefix);
+                          DiagRef D, bool diagnoseDollarPrefix);
 
   template<typename ...DiagArgTypes, typename ...ArgTypes>
   bool parseAnyIdentifier(Identifier &Result, bool diagnoseDollarPrefix,
                           Diag<DiagArgTypes...> ID, ArgTypes... Args) {
     SourceLoc L;
-    return parseAnyIdentifier(Result, L, Diagnostic(ID, Args...),
+    return parseAnyIdentifier(Result, L, {ID, {Args...}},
                               diagnoseDollarPrefix);
   }
 
@@ -849,28 +849,28 @@ class Parser {
   /// emit the specified error diagnostic, and a note at the specified note
   /// location.
   bool parseUnsignedInteger(unsigned &Result, SourceLoc &Loc,
-                            const Diagnostic &D);
+                            DiagRef D);
 
   /// The parser expects that \p K is next token in the input.  If so,
   /// it is consumed and false is returned.
   ///
   /// If the input is malformed, this emits the specified error diagnostic.
-  bool parseToken(tok K, SourceLoc &TokLoc, const Diagnostic &D);
+  bool parseToken(tok K, SourceLoc &TokLoc, DiagRef D);
   
   template<typename ...DiagArgTypes, typename ...ArgTypes>
   bool parseToken(tok K, Diag<DiagArgTypes...> ID, ArgTypes... Args) {
     SourceLoc L;
-    return parseToken(K, L, Diagnostic(ID, Args...));
+    return parseToken(K, L, {ID, {Args...}});
   }
   template<typename ...DiagArgTypes, typename ...ArgTypes>
   bool parseToken(tok K, SourceLoc &L,
                   Diag<DiagArgTypes...> ID, ArgTypes... Args) {
-    return parseToken(K, L, Diagnostic(ID, Args...));
+    return parseToken(K, L, {ID, {Args...}});
   }
   
   /// Parse the specified expected token and return its location on success.  On failure, emit the specified
   /// error diagnostic,  a note at the specified note location, and return the location of the previous token.
-  bool parseMatchingToken(tok K, SourceLoc &TokLoc, Diagnostic ErrorDiag,
+  bool parseMatchingToken(tok K, SourceLoc &TokLoc, DiagRef ErrorDiag,
                           SourceLoc OtherLoc);
 
   /// Returns the proper location for a missing right brace, parenthesis, etc.
@@ -903,7 +903,7 @@ class Parser {
 
   /// Parse a comma separated list of some elements.
   ParserStatus parseList(tok RightK, SourceLoc LeftLoc, SourceLoc &RightLoc,
-                         bool AllowSepAfterLast, Diag<> ErrorDiag,
+                         bool AllowSepAfterLast, DiagRef RightErrorDiag,
                          llvm::function_ref<ParserStatus()> callback);
 
   void consumeTopLevelDecl(ParserPosition BeginParserPosition,
@@ -1184,7 +1184,7 @@ class Parser {
   /// Parse a version tuple of the form x[.y[.z]]. Returns true if there was
   /// an error parsing.
   bool parseVersionTuple(llvm::VersionTuple &Version, SourceRange &Range,
-                         const Diagnostic &D);
+                         DiagRef D);
 
   bool isParameterSpecifier() {
     if (Tok.is(tok::kw_inout)) return true;
@@ -1832,7 +1832,7 @@ class Parser {
   ///   unqualified-decl-name:
   ///     unqualified-decl-base-name
   ///     unqualified-decl-base-name '(' ((identifier | '_') ':') + ')'
-  DeclNameRef parseDeclNameRef(DeclNameLoc &loc, const Diagnostic &diag,
+  DeclNameRef parseDeclNameRef(DeclNameLoc &loc, DiagRef diag,
                                DeclNameOptions flags);
 
   /// Parse macro expansion.
@@ -1843,7 +1843,7 @@ class Parser {
       SourceLoc &poundLoc, DeclNameLoc &macroNameLoc, DeclNameRef &macroNameRef,
       SourceLoc &leftAngleLoc, SmallVectorImpl<TypeRepr *> &genericArgs,
       SourceLoc &rightAngleLoc, ArgumentList *&argList, bool isExprBasic,
-      const Diagnostic &diag);
+      DiagRef diag);
 
   ParserResult<Expr> parseExprIdentifier(bool allowKeyword);
   Expr *parseExprEditorPlaceholder(Token PlaceholderTok,
diff --git a/lib/Parse/ParseDecl.cpp b/lib/Parse/ParseDecl.cpp
index f026b10c0375f..f6884aa9ad175 100644
--- a/lib/Parse/ParseDecl.cpp
+++ b/lib/Parse/ParseDecl.cpp
@@ -629,7 +629,7 @@ ParserResult<AvailableAttr> Parser::parseExtendedAvailabilitySpecList(
       bool VerArgWasEmpty = VerArg.empty();
       if (parseVersionTuple(
               VerArg.Version, VerArg.Range,
-              Diagnostic(diag::attr_availability_expected_version, AttrName))) {
+              {diag::attr_availability_expected_version, {AttrName}})) {
         AnyArgumentInvalid = true;
         if (peekToken().isAny(tok::r_paren, tok::comma))
           consumeToken();
@@ -2251,7 +2251,7 @@ ParserStatus Parser::parsePlatformVersionInList(StringRef AttrName,
   llvm::VersionTuple VerTuple;
   SourceRange VersionRange;
   if (parseVersionTuple(VerTuple, VersionRange,
-      Diagnostic(diag::attr_availability_expected_version, AttrName))) {
+        {diag::attr_availability_expected_version, {AttrName}})) {
     return makeParserError();
   }
 
@@ -2745,7 +2745,7 @@ Parser::parseMacroRoleAttribute(
 ///          omitted; the identifier written by the user otherwise.
 static std::optional<Identifier> parseSingleAttrOptionImpl(
     Parser &P, SourceLoc Loc, SourceRange &AttrRange, StringRef AttrName,
-    DeclAttrKind DK, bool allowOmitted, Diagnostic nonIdentifierDiagnostic) {
+    DeclAttrKind DK, bool allowOmitted, DiagRef nonIdentifierDiagnostic) {
   SWIFT_DEFER {
     AttrRange = SourceRange(Loc, P.PreviousLoc);
   };
@@ -2793,7 +2793,7 @@ parseSingleAttrOptionIdentifier(Parser &P, SourceLoc Loc,
                                 DeclAttrKind DK, bool allowOmitted = false) {
   return parseSingleAttrOptionImpl(
              P, Loc, AttrRange, AttrName, DK, allowOmitted,
-             { diag::attr_expected_option_identifier, AttrName });
+             {diag::attr_expected_option_identifier, {AttrName}});
 }
 
 /// Parses a (possibly optional) argument for an attribute containing a single identifier from a known set of
@@ -2819,8 +2819,8 @@ parseSingleAttrOption(Parser &P, SourceLoc Loc, SourceRange &AttrRange,
   auto parsedIdentifier = parseSingleAttrOptionImpl(
              P, Loc, AttrRange,AttrName, DK,
              /*allowOmitted=*/valueIfOmitted.has_value(),
-             Diagnostic(diag::attr_expected_option_such_as, AttrName,
-                        options.front().first.str()));
+             {diag::attr_expected_option_such_as,
+              {AttrName, options.front().first.str()}});
   if (!parsedIdentifier)
     return std::nullopt;
 
@@ -4043,7 +4043,7 @@ ParserStatus Parser::parseNewDeclAttribute(DeclAttributes &Attributes,
 
 bool Parser::parseVersionTuple(llvm::VersionTuple &Version,
                                SourceRange &Range,
-                               const Diagnostic &D) {
+                               DiagRef D) {
   // A version number is either an integer (8), a float (8.1), or a
   // float followed by a dot and an integer (8.1.0).
   if (!Tok.isAny(tok::integer_literal, tok::floating_literal)) {
diff --git a/lib/Parse/ParseExpr.cpp b/lib/Parse/ParseExpr.cpp
index 7efe1e452acc0..091071a3ba38e 100644
--- a/lib/Parse/ParseExpr.cpp
+++ b/lib/Parse/ParseExpr.cpp
@@ -2275,7 +2275,7 @@ static bool tryParseArgLabelList(Parser &P, Parser::DeclNameOptions flags,
 }
 
 DeclNameRef Parser::parseDeclNameRef(DeclNameLoc &loc,
-                                     const Diagnostic &diag,
+                                     DiagRef diag,
                                      DeclNameOptions flags) {
   // Consume the base name.
   DeclBaseName baseName;
@@ -2338,7 +2338,7 @@ ParserStatus Parser::parseFreestandingMacroExpansion(
     SourceLoc &poundLoc, DeclNameLoc &macroNameLoc, DeclNameRef &macroNameRef,
     SourceLoc &leftAngleLoc, SmallVectorImpl<TypeRepr *> &genericArgs,
     SourceLoc &rightAngleLoc, ArgumentList *&argList, bool isExprBasic,
-    const Diagnostic &diag) {
+    DiagRef diag) {
   SourceLoc poundEndLoc = Tok.getRange().getEnd();
   poundLoc = consumeToken(tok::pound);
 
diff --git a/lib/Parse/Parser.cpp b/lib/Parse/Parser.cpp
index ad25e38005d27..6590dfe17f458 100644
--- a/lib/Parse/Parser.cpp
+++ b/lib/Parse/Parser.cpp
@@ -868,7 +868,7 @@ Parser::StructureMarkerRAII::StructureMarkerRAII(Parser &parser,
 //===----------------------------------------------------------------------===//
 
 bool Parser::parseIdentifier(Identifier &Result, SourceLoc &Loc,
-                             const Diagnostic &D, bool diagnoseDollarPrefix) {
+                             DiagRef D, bool diagnoseDollarPrefix) {
   switch (Tok.getKind()) {
   case tok::kw_self:
   case tok::kw_Self:
@@ -883,7 +883,7 @@ bool Parser::parseIdentifier(Identifier &Result, SourceLoc &Loc,
 }
 
 bool Parser::parseSpecificIdentifier(StringRef expected, SourceLoc &loc,
-                                     const Diagnostic &D) {
+                                     DiagRef D) {
   if (Tok.getText() != expected) {
     diagnose(Tok, D);
     return true;
@@ -895,8 +895,7 @@ bool Parser::parseSpecificIdentifier(StringRef expected, SourceLoc &loc,
 /// parseAnyIdentifier - Consume an identifier or operator if present and return
 /// its name in Result.  Otherwise, emit an error and return true.
 bool Parser::parseAnyIdentifier(Identifier &Result, SourceLoc &Loc,
-                                const Diagnostic &D,
-                                bool diagnoseDollarPrefix) {
+                                DiagRef D, bool diagnoseDollarPrefix) {
   if (Tok.is(tok::identifier)) {
     Loc = consumeIdentifier(Result, diagnoseDollarPrefix);
     return false;
@@ -935,7 +934,7 @@ bool Parser::parseAnyIdentifier(Identifier &Result, SourceLoc &Loc,
 /// consumed and false is returned.
 ///
 /// If the input is malformed, this emits the specified error diagnostic.
-bool Parser::parseToken(tok K, SourceLoc &TokLoc, const Diagnostic &D) {
+bool Parser::parseToken(tok K, SourceLoc &TokLoc, DiagRef D) {
   if (Tok.is(K)) {
     TokLoc = consumeToken(K);
     return false;
@@ -946,7 +945,7 @@ bool Parser::parseToken(tok K, SourceLoc &TokLoc, const Diagnostic &D) {
   return true;
 }
 
-bool Parser::parseMatchingToken(tok K, SourceLoc &TokLoc, Diagnostic ErrorDiag,
+bool Parser::parseMatchingToken(tok K, SourceLoc &TokLoc, DiagRef ErrorDiag,
                                 SourceLoc OtherLoc) {
   Diag<> OtherNote;
   switch (K) {
@@ -966,7 +965,7 @@ bool Parser::parseMatchingToken(tok K, SourceLoc &TokLoc, Diagnostic ErrorDiag,
 }
 
 bool Parser::parseUnsignedInteger(unsigned &Result, SourceLoc &Loc,
-                                  const Diagnostic &D) {
+                                  DiagRef D) {
   auto IntTok = Tok;
   if (parseToken(tok::integer_literal, Loc, D))
     return true;
@@ -1058,7 +1057,7 @@ Parser::parseListItem(ParserStatus &Status, tok RightK, SourceLoc LeftLoc,
 
 ParserStatus
 Parser::parseList(tok RightK, SourceLoc LeftLoc, SourceLoc &RightLoc,
-                  bool AllowSepAfterLast, Diag<> ErrorDiag,
+                  bool AllowSepAfterLast, DiagRef ErrorDiag,
                   llvm::function_ref<ParserStatus()> callback) {
   if (Tok.is(RightK)) {
     RightLoc = consumeToken(RightK);
diff --git a/lib/SIL/Parser/ParseSIL.cpp b/lib/SIL/Parser/ParseSIL.cpp
index a760239b3a0ce..372966dc54c23 100644
--- a/lib/SIL/Parser/ParseSIL.cpp
+++ b/lib/SIL/Parser/ParseSIL.cpp
@@ -138,7 +138,7 @@ ParseSILModuleRequest::evaluate(Evaluator &evaluator,
 //===----------------------------------------------------------------------===//
 
 bool SILParser::parseSILIdentifier(Identifier &Result, SourceLoc &Loc,
-                                   const Diagnostic &D) {
+                                   DiagRef D) {
   switch (P.Tok.getKind()) {
   case tok::identifier:
   case tok::dollarident:
@@ -602,7 +602,7 @@ bool SILParser::parseSILQualifier(
   }
   result = parseName(Str);
   if (!result) {
-    P.diagnose(loc, Diagnostic(diag::unrecognized_sil_qualifier));
+    P.diagnose(loc, diag::unrecognized_sil_qualifier);
     return true;
   }
   return false;
@@ -2051,7 +2051,7 @@ parseAssignOrInitAssignments(llvm::SmallVectorImpl<unsigned> &assignments,
 // Returns true on error.
 static bool parseIndexList(Parser &P, StringRef label,
                            SmallVectorImpl<unsigned> &indices,
-                           const Diagnostic &parseIndexDiag) {
+                           DiagRef parseIndexDiag) {
   SourceLoc loc;
   // Parse `[<label> <integer_literal>...]`.
   if (P.parseToken(tok::l_square, diag::sil_autodiff_expected_lsquare,
@@ -7761,7 +7761,7 @@ parseRootProtocolConformance(Parser &P, SILParser &SP, Type ConformingTy,
 
   if (P.parseSpecificIdentifier(
           ModuleKeyword, KeywordLoc,
-          Diagnostic(diag::expected_tok_in_sil_instr, ModuleKeyword)) ||
+          {diag::expected_tok_in_sil_instr, {ModuleKeyword}}) ||
       SP.parseSILIdentifier(ModuleName, Loc, diag::expected_sil_value_name))
     return ProtocolConformanceRef();
 
diff --git a/lib/SIL/Parser/SILParser.h b/lib/SIL/Parser/SILParser.h
index 2af09ab88ad5d..9c9ee5ca1f8d9 100644
--- a/lib/SIL/Parser/SILParser.h
+++ b/lib/SIL/Parser/SILParser.h
@@ -141,14 +141,13 @@ class SILParser {
   /// \verbatim
   ///   sil-identifier ::= [A-Za-z_0-9]+
   /// \endverbatim
-  bool parseSILIdentifier(Identifier &Result, SourceLoc &Loc,
-                          const Diagnostic &D);
+  bool parseSILIdentifier(Identifier &Result, SourceLoc &Loc, DiagRef D);
 
   template <typename... DiagArgTypes, typename... ArgTypes>
   bool parseSILIdentifier(Identifier &Result, Diag<DiagArgTypes...> ID,
                           ArgTypes... Args) {
     SourceLoc L;
-    return parseSILIdentifier(Result, L, Diagnostic(ID, Args...));
+    return parseSILIdentifier(Result, L, {ID, {Args...}});
   }
 
   template <typename T, typename... DiagArgTypes, typename... ArgTypes>
@@ -156,13 +155,13 @@ class SILParser {
                                 Diag<DiagArgTypes...> ID, ArgTypes... Args) {
     Identifier TmpResult;
     SourceLoc L;
-    if (parseSILIdentifier(TmpResult, L, Diagnostic(ID, Args...))) {
+    if (parseSILIdentifier(TmpResult, L, {ID, {Args...}})) {
       return true;
     }
 
     auto Iter = std::find(Strings.begin(), Strings.end(), TmpResult.str());
     if (Iter == Strings.end()) {
-      P.diagnose(P.Tok, Diagnostic(ID, Args...));
+      P.diagnose(P.Tok, ID, Args...);
       return true;
     }
 
@@ -173,7 +172,7 @@ class SILParser {
   template <typename... DiagArgTypes, typename... ArgTypes>
   bool parseSILIdentifier(Identifier &Result, SourceLoc &L,
                           Diag<DiagArgTypes...> ID, ArgTypes... Args) {
-    return parseSILIdentifier(Result, L, Diagnostic(ID, Args...));
+    return parseSILIdentifier(Result, L, {ID, {Args...}});
   }
 
   template <typename T>
@@ -184,7 +183,7 @@ class SILParser {
   bool parseVerbatim(StringRef identifier);
 
   template <typename T>
-  bool parseInteger(T &Result, const Diagnostic &D) {
+  bool parseInteger(T &Result, DiagRef D) {
     if (!P.Tok.is(tok::integer_literal)) {
       P.diagnose(P.Tok, D);
       return true;

From 1437acdf22ecb997b43a23a89c9249ca0b0ac676 Mon Sep 17 00:00:00 2001
From: John McCall <rjmccall@apple.com>
Date: Fri, 1 Mar 2024 01:39:35 -0500
Subject: [PATCH 2/3] Implement conditional feature suppression.

Our standard conception of suppressible features assumes we should
always suppress the feature if the compiler doesn't support it.
This presumes that there's no harm in suppressing the feature, and
that's a fine assumption for features that are just adding information
or suppressing new diagnostics.  Features that are semantically
relevant, maybe even ABI-breaking, are not a good fit for this,
and so instead of reprinting the decl with the feature suppressed,
we just have to hide the decl entirely.  The missing middle here
is that it's sometimes useful to be able to adopt a type change
to an existing declaration, and we'd like older compilers to be
able to use the older version of the declaration.  Making a type
change this way is, of course, only really acceptable for
@_alwaysEmitIntoClient declarations; but those represent quite a
few declarations that we'd like to be able to refine the types of.

Rather than trying to come up with heuristics based on
@_alwaysEmitIntoClient or other sources of information, this design
just requires the declaration to opt in with a new attribute,
@_allowFeatureSuppress.  When a declaration opts in to suppression
for a conditionally-suppressible feature, the printer uses the
suppression serially-print-with-downgraded-options approach;
otherwise it uses the print-only-if-feature-is-available approach.
---
 include/swift/AST/ASTBridging.h           |  8 ++
 include/swift/AST/Attr.h                  | 31 ++++++++
 include/swift/AST/DeclAttr.def            |  5 +-
 include/swift/AST/DiagnosticsParse.def    |  4 +
 include/swift/AST/PrintOptions.h          |  4 +-
 include/swift/Basic/Feature.h             |  3 -
 include/swift/Basic/Features.def          | 45 +++++++++++-
 include/swift/Parse/Parser.h              | 11 +++
 lib/AST/ASTBridging.cpp                   | 12 +++
 lib/AST/ASTPrinter.cpp                    |  2 +
 lib/AST/Attr.cpp                          | 22 ++++++
 lib/AST/FeatureSet.cpp                    | 21 +++++-
 lib/ASTGen/Sources/ASTGen/DeclAttrs.swift | 28 +++++++
 lib/Basic/LangOptions.cpp                 | 13 ----
 lib/Parse/ParseDecl.cpp                   | 89 ++++++++++++++++-------
 lib/Sema/TypeCheckAttr.cpp                |  1 +
 lib/Sema/TypeCheckDeclOverride.cpp        |  1 +
 lib/Serialization/Deserialization.cpp     | 16 ++++
 lib/Serialization/ModuleFormat.h          |  8 +-
 lib/Serialization/Serialization.cpp       | 14 ++++
 20 files changed, 289 insertions(+), 49 deletions(-)

diff --git a/include/swift/AST/ASTBridging.h b/include/swift/AST/ASTBridging.h
index c7bd27f089f97..d65098a7ee483 100644
--- a/include/swift/AST/ASTBridging.h
+++ b/include/swift/AST/ASTBridging.h
@@ -481,6 +481,14 @@ BridgedAlignmentAttr_createParsed(BridgedASTContext cContext,
                                   BridgedSourceLoc cAtLoc,
                                   BridgedSourceRange cRange, size_t cValue);
 
+SWIFT_NAME("BridgedAllowFeatureSuppressionAttr.createParsed(_:atLoc:range:features:)")
+BridgedAllowFeatureSuppressionAttr
+BridgedAllowFeatureSuppressionAttr_createParsed(
+                                  BridgedASTContext cContext,
+                                  BridgedSourceLoc cAtLoc,
+                                  BridgedSourceRange cRange,
+                                  BridgedArrayRef cFeatures);
+
 SWIFT_NAME("BridgedCDeclAttr.createParsed(_:atLoc:range:name:)")
 BridgedCDeclAttr BridgedCDeclAttr_createParsed(BridgedASTContext cContext,
                                                BridgedSourceLoc cAtLoc,
diff --git a/include/swift/AST/Attr.h b/include/swift/AST/Attr.h
index e714d5c729336..1119c156042de 100644
--- a/include/swift/AST/Attr.h
+++ b/include/swift/AST/Attr.h
@@ -191,6 +191,11 @@ class DeclAttribute : public AttributeBase {
     SWIFT_INLINE_BITFIELD(NonisolatedAttr, DeclAttribute, 1,
       isUnsafe : 1
     );
+
+    SWIFT_INLINE_BITFIELD_FULL(AllowFeatureSuppressionAttr, DeclAttribute, 32,
+      : NumPadBits,
+      NumFeatures : 32
+    );
   } Bits;
   // clang-format on
 
@@ -2636,6 +2641,32 @@ template <typename ATTR, bool AllowInvalid> struct ToAttributeKind {
   }
 };
 
+/// The @_allowFeatureSuppression(Foo, Bar) attribute.  The feature
+/// names are intentionally not validated, and the attribute itself is
+/// not printed when rendering a module interface.
+class AllowFeatureSuppressionAttr final
+    : public DeclAttribute,
+      private llvm::TrailingObjects<AllowFeatureSuppressionAttr, Identifier> {
+  friend TrailingObjects;
+
+  /// Create an implicit @objc attribute with the given (optional) name.
+  AllowFeatureSuppressionAttr(SourceLoc atLoc, SourceRange range,
+                              bool implicit, ArrayRef<Identifier> features);
+public:
+  static AllowFeatureSuppressionAttr *create(ASTContext &ctx, SourceLoc atLoc,
+                                             SourceRange range, bool implicit,
+                                             ArrayRef<Identifier> features);
+
+  ArrayRef<Identifier> getSuppressedFeatures() const {
+    return {getTrailingObjects<Identifier>(),
+            Bits.AllowFeatureSuppressionAttr.NumFeatures};
+  }
+
+  static bool classof(const DeclAttribute *DA) {
+    return DA->getKind() == DeclAttrKind::AllowFeatureSuppression;
+  }
+};
+
 /// Attributes that may be applied to declarations.
 class DeclAttributes {
   /// Linked list of declaration attributes.
diff --git a/include/swift/AST/DeclAttr.def b/include/swift/AST/DeclAttr.def
index bdb2a27cbee4c..66cb7135fc318 100644
--- a/include/swift/AST/DeclAttr.def
+++ b/include/swift/AST/DeclAttr.def
@@ -488,7 +488,10 @@ SIMPLE_DECL_ATTR(_noObjCBridging, NoObjCBridging,
 DECL_ATTR(_distributedThunkTarget, DistributedThunkTarget,
   OnAbstractFunction | UserInaccessible | ABIStableToAdd | ABIBreakingToRemove | APIStableToAdd | APIStableToRemove,
   156)
-LAST_DECL_ATTR(DistributedThunkTarget)
+DECL_ATTR(_allowFeatureSuppression, AllowFeatureSuppression,
+  OnAnyDecl | UserInaccessible | NotSerialized | ABIStableToAdd | APIStableToAdd | ABIStableToRemove | APIStableToRemove,
+  157)
+LAST_DECL_ATTR(AllowFeatureSuppression)
 
 #undef DECL_ATTR_ALIAS
 #undef CONTEXTUAL_DECL_ATTR_ALIAS
diff --git a/include/swift/AST/DiagnosticsParse.def b/include/swift/AST/DiagnosticsParse.def
index d923f2f67af0c..cd79633991b5f 100644
--- a/include/swift/AST/DiagnosticsParse.def
+++ b/include/swift/AST/DiagnosticsParse.def
@@ -1889,6 +1889,10 @@ ERROR(attr_rawlayout_expected_params,none,
 
 ERROR(attr_extern_expected_label,none,
       "expected %0 argument to @_extern attribute", (StringRef))
+
+ERROR(attr_expected_feature_name,none,
+      "expected feature name in @%0 attribute", (StringRef))
+
 //------------------------------------------------------------------------------
 // MARK: Generics parsing diagnostics
 //------------------------------------------------------------------------------
diff --git a/include/swift/AST/PrintOptions.h b/include/swift/AST/PrintOptions.h
index 83fba72c74f15..c9d3651d8c851 100644
--- a/include/swift/AST/PrintOptions.h
+++ b/include/swift/AST/PrintOptions.h
@@ -379,7 +379,9 @@ struct PrintOptions {
   /// List of attribute kinds that should not be printed.
   std::vector<AnyAttrKind> ExcludeAttrList = {
       DeclAttrKind::Transparent, DeclAttrKind::Effects,
-      DeclAttrKind::FixedLayout, DeclAttrKind::ShowInInterface};
+      DeclAttrKind::FixedLayout, DeclAttrKind::ShowInInterface,
+      DeclAttrKind::AllowFeatureSuppression
+  };
 
   /// List of attribute kinds that should be printed exclusively.
   /// Empty means allow all.
diff --git a/include/swift/Basic/Feature.h b/include/swift/Basic/Feature.h
index 0b2af2b969309..a2f2d45361ae3 100644
--- a/include/swift/Basic/Feature.h
+++ b/include/swift/Basic/Feature.h
@@ -35,9 +35,6 @@ constexpr unsigned numFeatures() {
   return NumFeatures;
 }
 
-/// Determine whether the given feature is suppressible.
-bool isSuppressibleFeature(Feature feature);
-
 /// Check whether the given feature is available in production compilers.
 bool isFeatureAvailableInProduction(Feature feature);
 
diff --git a/include/swift/Basic/Features.def b/include/swift/Basic/Features.def
index a4d3538913713..610e04a2c42d0 100644
--- a/include/swift/Basic/Features.def
+++ b/include/swift/Basic/Features.def
@@ -34,6 +34,11 @@
 // imply the existence of earlier features.  (This only needs to apply to
 // suppressible features.)
 //
+// If suppressing a feature in general is problematic, but it's okay to
+// suppress it for specific declarations, the feature can be made
+// conditionally suppressible.  Declarations opt in to suppression with
+// the @_allowFeatureSuppression attribute.
+//
 // BASELINE_LANGUAGE_FEATURE is the same as LANGUAGE_FEATURE, but is used
 // for features that can be assumed to be available in any Swift compiler that
 // will be used to process the textual interface files produced by this
@@ -44,11 +49,46 @@
 #  error define LANGUAGE_FEATURE before including Features.def
 #endif
 
+// A feature that's both suppressible and experimental.
+// Delegates to whichever the includer defines.
+#ifndef SUPPRESSIBLE_EXPERIMENTAL_FEATURE
+#  if defined(SUPPRESSIBLE_LANGUAGE_FEATURE) && \
+      defined(EXPERIMENTAL_FEATURE)
+#    error ambiguous defines when including Features.def
+#  elif defined(SUPPRESSIBLE_LANGUAGE_FEATURE)
+#    define SUPPRESSIBLE_EXPERIMENTAL_FEATURE(FeatureName, AvailableInProd) \
+       SUPPRESSIBLE_LANGUAGE_FEATURE(FeatureName, 0, #FeatureName)
+#  else
+#    define SUPPRESSIBLE_EXPERIMENTAL_FEATURE(FeatureName, AvailableInProd) \
+       EXPERIMENTAL_FEATURE(FeatureName, AvailableInProd)
+#  endif
+#endif
+
 #ifndef SUPPRESSIBLE_LANGUAGE_FEATURE
-#define SUPPRESSIBLE_LANGUAGE_FEATURE(FeatureName, SENumber, Description) \
+#  define SUPPRESSIBLE_LANGUAGE_FEATURE(FeatureName, SENumber, Description) \
   LANGUAGE_FEATURE(FeatureName, SENumber, Description)
 #endif
 
+// A feature that's both conditionally-suppressible and experimental.
+// Delegates to whichever the includer defines.
+#ifndef CONDITIONALLY_SUPPRESSIBLE_EXPERIMENTAL_FEATURE
+#  if defined(CONDITIONALLY_SUPPRESSIBLE_LANGUAGE_FEATURE) && \
+      defined(EXPERIMENTAL_FEATURE)
+#    error ambiguous defines when including Features.def
+#  elif defined(CONDITIONALLY_SUPPRESSIBLE_LANGUAGE_FEATURE)
+#    define CONDITIONALLY_SUPPRESSIBLE_EXPERIMENTAL_FEATURE(FeatureName, AvailableInProd) \
+       CONDITIONALLY_SUPPRESSIBLE_LANGUAGE_FEATURE(FeatureName, 0, #FeatureName)
+#  else
+#    define CONDITIONALLY_SUPPRESSIBLE_EXPERIMENTAL_FEATURE(FeatureName, AvailableInProd) \
+       EXPERIMENTAL_FEATURE(FeatureName, AvailableInProd)
+#  endif
+#endif
+
+#ifndef CONDITIONALLY_SUPPRESSIBLE_LANGUAGE_FEATURE
+#  define CONDITIONALLY_SUPPRESSIBLE_LANGUAGE_FEATURE(FeatureName, SENumber, Description) \
+     LANGUAGE_FEATURE(FeatureName, SENumber, Description)
+#endif
+
 #ifndef UPCOMING_FEATURE
 #  define UPCOMING_FEATURE(FeatureName, SENumber, Version)   \
      LANGUAGE_FEATURE(FeatureName, SENumber, #FeatureName)
@@ -301,5 +341,8 @@ EXPERIMENTAL_FEATURE(IsolatedAny, true)
 #undef EXPERIMENTAL_FEATURE
 #undef UPCOMING_FEATURE
 #undef BASELINE_LANGUAGE_FEATURE
+#undef CONDITIONALLY_SUPPRESSIBLE_EXPERIMENTAL_FEATURE
+#undef CONDITIONALLY_SUPPRESSIBLE_LANGUAGE_FEATURE
+#undef SUPPRESSIBLE_EXPERIMENTAL_FEATURE
 #undef SUPPRESSIBLE_LANGUAGE_FEATURE
 #undef LANGUAGE_FEATURE
diff --git a/include/swift/Parse/Parser.h b/include/swift/Parse/Parser.h
index 39a2671335f63..a7d2fe0d5565d 100644
--- a/include/swift/Parse/Parser.h
+++ b/include/swift/Parse/Parser.h
@@ -1072,6 +1072,14 @@ class Parser {
                          SmallVector<SourceLoc, 4> &NameLocs,
                          bool &IsNullarySelector);
 
+  /// Parse a parenthesized and comma-separated list of attribute arguments.
+  ///
+  /// \returns false on success, true on error.
+  ParserStatus
+  parseAttributeArguments(SourceLoc attrLoc, StringRef attrName,
+                          bool isAttrModifier, SourceRange &parensRange,
+                          llvm::function_ref<ParserStatus()> parseAttr);
+
   /// Parse the @_specialize attribute.
   /// \p closingBrace is the expected closing brace, which can be either ) or ]
   /// \p Attr is where to store the parsed attribute
@@ -1151,6 +1159,9 @@ class Parser {
   parseDocumentationAttributeArgument(std::optional<StringRef> &Metadata,
                                       std::optional<AccessLevel> &Visibility);
 
+  ParserResult<AllowFeatureSuppressionAttr>
+  parseAllowFeatureSuppressionAttribute(SourceLoc atLoc, SourceLoc loc);
+
   /// Parse the @attached or @freestanding attribute that specifies a macro
   /// role.
   ParserResult<MacroRoleAttr> parseMacroRoleAttribute(
diff --git a/lib/AST/ASTBridging.cpp b/lib/AST/ASTBridging.cpp
index 452ec2fb6d6ab..b7d820e6e50d8 100644
--- a/lib/AST/ASTBridging.cpp
+++ b/lib/AST/ASTBridging.cpp
@@ -429,6 +429,18 @@ BridgedAlignmentAttr_createParsed(BridgedASTContext cContext,
       cValue, cAtLoc.unbridged(), cRange.unbridged(), /*Implicit=*/false);
 }
 
+BridgedAllowFeatureSuppressionAttr
+BridgedAllowFeatureSuppressionAttr_createParsed(BridgedASTContext cContext,
+                                                BridgedSourceLoc cAtLoc,
+                                                BridgedSourceRange cRange,
+                                                BridgedArrayRef cFeatures) {
+  SmallVector<Identifier> features;
+  for (auto elem : cFeatures.unbridged<BridgedIdentifier>())
+    features.push_back(elem.unbridged());
+  return AllowFeatureSuppressionAttr::create(cContext.unbridged(),
+      cAtLoc.unbridged(), cRange.unbridged(), /*implicit*/ false, features);
+}
+
 BridgedCDeclAttr BridgedCDeclAttr_createParsed(BridgedASTContext cContext,
                                                BridgedSourceLoc cAtLoc,
                                                BridgedSourceRange cRange,
diff --git a/lib/AST/ASTPrinter.cpp b/lib/AST/ASTPrinter.cpp
index 9ad6f9739e616..e5f7e91a0e337 100644
--- a/lib/AST/ASTPrinter.cpp
+++ b/lib/AST/ASTPrinter.cpp
@@ -3112,6 +3112,8 @@ static void suppressingFeature(PrintOptions &options, Feature feature,
   case Feature::FeatureName:                                                   \
     suppressingFeature##FeatureName(options, action);                          \
     return;
+#define CONDITIONALLY_SUPPRESSIBLE_LANGUAGE_FEATURE(FeatureName, SENumber, Description)      \
+  SUPPRESSIBLE_LANGUAGE_FEATURE(FeatureName, SENumber, Description)
 #include "swift/Basic/Features.def"
   }
   llvm_unreachable("exhaustive switch");
diff --git a/lib/AST/Attr.cpp b/lib/AST/Attr.cpp
index e002cbcd14d8a..85f12996a11de 100644
--- a/lib/AST/Attr.cpp
+++ b/lib/AST/Attr.cpp
@@ -1918,6 +1918,8 @@ StringRef DeclAttribute::getAttrName() const {
     return "_rawLayout";
   case DeclAttrKind::Extern:
     return "_extern";
+  case DeclAttrKind::AllowFeatureSuppression:
+    return "_allowFeatureSuppression";
   }
   llvm_unreachable("bad DeclAttrKind");
 }
@@ -2901,6 +2903,26 @@ StorageRestrictionsAttr::getAccessesProperties(AccessorDecl *attachedTo) const {
                            {});
 }
 
+AllowFeatureSuppressionAttr::AllowFeatureSuppressionAttr(SourceLoc atLoc,
+                                                         SourceRange range,
+                                                         bool implicit,
+                                              ArrayRef<Identifier> features)
+    : DeclAttribute(DeclAttrKind::AllowFeatureSuppression,
+                    atLoc, range, implicit) {
+  Bits.AllowFeatureSuppressionAttr.NumFeatures = features.size();
+  std::uninitialized_copy(features.begin(), features.end(),
+                          getTrailingObjects<Identifier>());
+}
+
+AllowFeatureSuppressionAttr *
+AllowFeatureSuppressionAttr::create(ASTContext &ctx, SourceLoc atLoc,
+                                    SourceRange range, bool implicit,
+                                    ArrayRef<Identifier> features) {
+  unsigned size = totalSizeToAlloc<Identifier>(features.size());
+  auto *mem = ctx.Allocate(size, alignof(AllowFeatureSuppressionAttr));
+  return new (mem) AllowFeatureSuppressionAttr(atLoc, range, implicit, features);
+}
+
 void swift::simple_display(llvm::raw_ostream &out, const DeclAttribute *attr) {
   if (attr)
     attr->print(out);
diff --git a/lib/AST/FeatureSet.cpp b/lib/AST/FeatureSet.cpp
index b5ce3dde95a79..1981a307d2dfa 100644
--- a/lib/AST/FeatureSet.cpp
+++ b/lib/AST/FeatureSet.cpp
@@ -677,17 +677,27 @@ static bool usesFeatureIsolatedAny(Decl *decl) {
 
 void FeatureSet::collectRequiredFeature(Feature feature,
                                         InsertOrRemove operation) {
-  assert(!isSuppressibleFeature(feature));
   required.insertOrRemove(feature, operation == Insert);
 }
 
 void FeatureSet::collectSuppressibleFeature(Feature feature,
                                             InsertOrRemove operation) {
-  assert(isSuppressibleFeature(feature));
   suppressible.insertOrRemove(numFeatures() - size_t(feature),
                               operation == Insert);
 }
 
+static bool shouldSuppressFeature(StringRef featureName, Decl *decl) {
+  auto attr = decl->getAttrs().getAttribute<AllowFeatureSuppressionAttr>();
+  if (!attr) return false;
+
+  for (auto suppressedFeature : attr->getSuppressedFeatures()) {
+    if (suppressedFeature.is(featureName))
+      return true;
+  }
+
+  return false;
+}
+
 /// Go through all the features used by the given declaration and
 /// either add or remove them to this set.
 void FeatureSet::collectFeaturesUsed(Decl *decl, InsertOrRemove operation) {
@@ -699,6 +709,13 @@ void FeatureSet::collectFeaturesUsed(Decl *decl, InsertOrRemove operation) {
 #define SUPPRESSIBLE_LANGUAGE_FEATURE(FeatureName, SENumber, Description)      \
   if (usesFeature##FeatureName(decl))                                          \
     collectSuppressibleFeature(Feature::FeatureName, operation);
+#define CONDITIONALLY_SUPPRESSIBLE_LANGUAGE_FEATURE(FeatureName, SENumber, Description)      \
+  if (usesFeature##FeatureName(decl)) {                                        \
+    if (shouldSuppressFeature(#FeatureName, decl))                             \
+      collectSuppressibleFeature(Feature::FeatureName, operation);             \
+    else                                                                       \
+      collectRequiredFeature(Feature::FeatureName, operation);                 \
+  }
 #include "swift/Basic/Features.def"
 }
 
diff --git a/lib/ASTGen/Sources/ASTGen/DeclAttrs.swift b/lib/ASTGen/Sources/ASTGen/DeclAttrs.swift
index 96affe46b21db..237e810500f35 100644
--- a/lib/ASTGen/Sources/ASTGen/DeclAttrs.swift
+++ b/lib/ASTGen/Sources/ASTGen/DeclAttrs.swift
@@ -185,6 +185,8 @@ extension ASTGenVisitor {
         fatalError("unimplemented")
       case .unavailableFromAsync:
         fatalError("unimplemented")
+      case .allowFeatureSuppression:
+        return self.generateAllowFeatureSuppressionAttr(attribute: node)?.asDeclAttribute
 
       // Simple attributes.
       case .alwaysEmitConformanceMetadata,
@@ -336,6 +338,32 @@ extension ASTGenVisitor {
     )
   }
 
+  func generateAllowFeatureSuppressionAttr(attribute node: AttributeSyntax) -> BridgedAllowFeatureSuppressionAttr? {
+    guard case .argumentList(let args) = node.arguments
+    else {
+      // TODO: Diagnose.
+      return nil
+    }
+
+    let features = args.compactMap(in: self) { arg -> BridgedIdentifier? in
+      guard arg.label == nil,
+            let declNameExpr = arg.expression.as(DeclReferenceExprSyntax.self),
+            declNameExpr.argumentNames == nil
+      else {
+        // TODO: Diagnose.
+        return nil
+      }
+
+      return generateIdentifier(declNameExpr.baseName)
+    }
+
+    return .createParsed(
+      self.ctx,
+      atLoc: self.generateSourceLoc(node.atSign),
+      range: self.generateSourceRange(node),
+      features: features)
+  }
+
   func generateCDeclAttr(attribute node: AttributeSyntax) -> BridgedCDeclAttr? {
     guard
       // `@_cdecl` attribute has `.string(StringLiteralExprSyntax)` arguments.
diff --git a/lib/Basic/LangOptions.cpp b/lib/Basic/LangOptions.cpp
index 22f250dd03163..09078571139f0 100644
--- a/lib/Basic/LangOptions.cpp
+++ b/lib/Basic/LangOptions.cpp
@@ -598,19 +598,6 @@ llvm::StringRef swift::getFeatureName(Feature feature) {
   llvm_unreachable("covered switch");
 }
 
-bool swift::isSuppressibleFeature(Feature feature) {
-  switch (feature) {
-#define LANGUAGE_FEATURE(FeatureName, SENumber, Description)                   \
-  case Feature::FeatureName:                                                   \
-    return false;
-#define SUPPRESSIBLE_LANGUAGE_FEATURE(FeatureName, SENumber, Description)      \
-  case Feature::FeatureName:                                                   \
-    return true;
-#include "swift/Basic/Features.def"
-  }
-  llvm_unreachable("covered switch");
-}
-
 bool swift::isFeatureAvailableInProduction(Feature feature) {
   switch (feature) {
 #define LANGUAGE_FEATURE(FeatureName, SENumber, Description)                   \
diff --git a/lib/Parse/ParseDecl.cpp b/lib/Parse/ParseDecl.cpp
index f6884aa9ad175..f65e0934f9dcb 100644
--- a/lib/Parse/ParseDecl.cpp
+++ b/lib/Parse/ParseDecl.cpp
@@ -2419,43 +2419,69 @@ bool Parser::parseDocumentationAttributeArgument(
   return true;
 }
 
-ParserResult<DocumentationAttr>
-Parser::parseDocumentationAttribute(SourceLoc AtLoc, SourceLoc Loc) {
-  StringRef AttrName = "_documentation";
-  bool declModifier =
-      DeclAttribute::isDeclModifier(DeclAttrKind::Documentation);
-  std::optional<AccessLevel> Visibility = std::nullopt;
-  std::optional<StringRef> Metadata = std::nullopt;
-
+ParserStatus
+Parser::parseAttributeArguments(SourceLoc attrLoc, StringRef attrName,
+                                bool isModifier, SourceRange &parensRange,
+                                llvm::function_ref<ParserStatus()> parseArg) {
+  parensRange.Start = Tok.getLoc();
   if (!consumeIfAttributeLParen()) {
-    diagnose(Loc, diag::attr_expected_lparen, AttrName,
-             declModifier);
+    diagnose(attrLoc, diag::attr_expected_lparen, attrName, isModifier);
     return makeParserError();
   }
 
-  while (Tok.isNot(tok::r_paren)) {
-    if (!parseDocumentationAttributeArgument(Metadata, Visibility))
-      return makeParserError();
+  return parseList(tok::r_paren, parensRange.Start, parensRange.End,
+                   /*allow sep after last*/ true,
+                   {diag::attr_expected_rparen, {attrName, isModifier}},
+                   parseArg);
+}
 
-    if (Tok.is(tok::comma)) {
-      consumeToken();
-    } else if (Tok.isNot(tok::r_paren)) {
-      diagnose(Tok, diag::expected_separator, ",");
+ParserResult<DocumentationAttr>
+Parser::parseDocumentationAttribute(SourceLoc atLoc, SourceLoc loc) {
+  StringRef attrName = "_documentation";
+
+  std::optional<AccessLevel> visibility = std::nullopt;
+  std::optional<StringRef> metadata = std::nullopt;
+
+  SourceRange parensRange;
+  auto status = parseAttributeArguments(loc, attrName, false, parensRange,
+                                        [&] {
+    if (!parseDocumentationAttributeArgument(metadata, visibility))
       return makeParserError();
-    }
-  }
+    return makeParserSuccess();
+  });
+  if (!status.isSuccess())
+    return status;
 
-  auto range = SourceRange(Loc, Tok.getRange().getStart());
+  auto range = SourceRange(loc, parensRange.End);
+  StringRef finalMetadata = metadata.value_or("");
 
-  if (!consumeIf(tok::r_paren)) {
-    diagnose(Loc, diag::attr_expected_rparen, AttrName,
-             declModifier);
-    return makeParserError();
-  }
+  return makeParserResult(
+    new (Context) DocumentationAttr(loc, range, finalMetadata,
+                                    visibility, false));
+}
 
-  StringRef FinalMetadata = Metadata.value_or("");
+ParserResult<AllowFeatureSuppressionAttr>
+Parser::parseAllowFeatureSuppressionAttribute(SourceLoc atLoc, SourceLoc loc) {
+  StringRef attrName = "_allowFeatureSuppression";
+
+  SmallVector<Identifier, 4> features;
+  SourceRange parensRange;
+  auto status = parseAttributeArguments(loc, attrName, /*modifier*/ false,
+                                        parensRange, [&] {
+    Identifier feature;
+    if (parseAnyIdentifier(feature, /*diagnose dollars*/ true,
+                           diag::attr_expected_feature_name, attrName))
+      return makeParserError();
+    features.push_back(feature);
+    return makeParserSuccess();
+  });
+  if (!status.isSuccess())
+    return status;
 
-  return makeParserResult(new (Context) DocumentationAttr(Loc, range, FinalMetadata, Visibility, false));
+  auto range = SourceRange(loc, parensRange.End);
+  return makeParserResult(
+    AllowFeatureSuppressionAttr::create(Context, loc, range, /*implicit*/ false,
+                                        features));
 }
 
 static std::optional<MacroIntroducedDeclNameKind>
@@ -3883,6 +3909,15 @@ ParserStatus Parser::parseNewDeclAttribute(DeclAttributes &Attributes,
       return Attr;
     break;
   }
+  case DeclAttrKind::AllowFeatureSuppression: {
+    auto Attr = parseAllowFeatureSuppressionAttribute(AtLoc, Loc);
+    Status |= Attr;
+    if (Attr.isNonNull())
+      Attributes.add(Attr.get());
+    else
+      return makeParserSuccess();
+    break;
+  }
   case DeclAttrKind::RawLayout: {
     if (Tok.isNot(tok::l_paren)) {
       diagnose(Loc, diag::attr_expected_lparen, AttrName,
diff --git a/lib/Sema/TypeCheckAttr.cpp b/lib/Sema/TypeCheckAttr.cpp
index 65d4794081166..d6e511ebd2a35 100644
--- a/lib/Sema/TypeCheckAttr.cpp
+++ b/lib/Sema/TypeCheckAttr.cpp
@@ -165,6 +165,7 @@ class AttributeChecker : public AttributeVisitor<AttributeChecker> {
   IGNORED_ATTR(Documentation)
   IGNORED_ATTR(LexicalLifetimes)
   IGNORED_ATTR(DistributedThunkTarget)
+  IGNORED_ATTR(AllowFeatureSuppression)
 #undef IGNORED_ATTR
 
   void visitAlignmentAttr(AlignmentAttr *attr) {
diff --git a/lib/Sema/TypeCheckDeclOverride.cpp b/lib/Sema/TypeCheckDeclOverride.cpp
index e7bf5b5ffb262..f944b8af583f5 100644
--- a/lib/Sema/TypeCheckDeclOverride.cpp
+++ b/lib/Sema/TypeCheckDeclOverride.cpp
@@ -1559,6 +1559,7 @@ namespace  {
     UNINTERESTING_ATTR(PrivateImport)
     UNINTERESTING_ATTR(MainType)
     UNINTERESTING_ATTR(Preconcurrency)
+    UNINTERESTING_ATTR(AllowFeatureSuppression)
 
     // Differentiation-related attributes.
     UNINTERESTING_ATTR(Differentiable)
diff --git a/lib/Serialization/Deserialization.cpp b/lib/Serialization/Deserialization.cpp
index 96db52beb8f5f..a44816315d30a 100644
--- a/lib/Serialization/Deserialization.cpp
+++ b/lib/Serialization/Deserialization.cpp
@@ -6006,6 +6006,22 @@ llvm::Error DeclDeserializer::deserializeDeclCommon() {
         break;
       }
 
+      case decls_block::AllowFeatureSuppression_DECL_ATTR: {
+        bool isImplicit;
+        ArrayRef<uint64_t> featureIds;
+        serialization::decls_block::AllowFeatureSuppressionDeclAttrLayout
+                     ::readRecord(scratch, isImplicit, featureIds);
+
+        SmallVector<Identifier, 4> features;
+        for (auto id : featureIds)
+          features.push_back(MF.getIdentifier(id));
+
+        Attr = AllowFeatureSuppressionAttr::create(ctx, SourceLoc(),
+                                                   SourceRange(), isImplicit,
+                                                   features);
+        break;
+      }
+
       case decls_block::UnavailableFromAsync_DECL_ATTR: {
         bool isImplicit;
         serialization::decls_block::UnavailableFromAsyncDeclAttrLayout::
diff --git a/lib/Serialization/ModuleFormat.h b/lib/Serialization/ModuleFormat.h
index 62ab3906f6cc9..b5d54895e8105 100644
--- a/lib/Serialization/ModuleFormat.h
+++ b/lib/Serialization/ModuleFormat.h
@@ -58,7 +58,7 @@ const uint16_t SWIFTMODULE_VERSION_MAJOR = 0;
 /// describe what change you made. The content of this comment isn't important;
 /// it just ensures a conflict if two people change the module format.
 /// Don't worry about adhering to the 80-column limit for this line.
-const uint16_t SWIFTMODULE_VERSION_MINOR = 859; // channel check
+const uint16_t SWIFTMODULE_VERSION_MINOR = 860; // AllowFeatureSuppressionAttr
 
 /// A standard hash seed used for all string hashes in a serialized module.
 ///
@@ -2135,6 +2135,12 @@ namespace decls_block {
     BCArray<IdentifierIDField> // name components
   >;
 
+  using AllowFeatureSuppressionDeclAttrLayout = BCRecordLayout<
+    AllowFeatureSuppression_DECL_ATTR,
+    BCFixed<1>,   // implicit flag
+    BCArray<IdentifierIDField>  // feature names
+  >;
+
   using SPIAccessControlDeclAttrLayout = BCRecordLayout<
     SPIAccessControl_DECL_ATTR,
     BCArray<IdentifierIDField>  // SPI names
diff --git a/lib/Serialization/Serialization.cpp b/lib/Serialization/Serialization.cpp
index 609154e4a4494..3e27f554c8ba8 100644
--- a/lib/Serialization/Serialization.cpp
+++ b/lib/Serialization/Serialization.cpp
@@ -2752,6 +2752,20 @@ class Serializer::DeclSerializer : public DeclVisitor<DeclSerializer> {
       return;
     }
 
+    case DeclAttrKind::AllowFeatureSuppression: {
+      auto *theAttr = cast<AllowFeatureSuppressionAttr>(DA);
+      auto abbrCode =
+        S.DeclTypeAbbrCodes[AllowFeatureSuppressionDeclAttrLayout::Code];
+
+      SmallVector<IdentifierID> ids;
+      for (auto id : theAttr->getSuppressedFeatures())
+        ids.push_back(S.addUniquedStringRef(id.str()));
+
+      AllowFeatureSuppressionDeclAttrLayout::emitRecord(
+          S.Out, S.ScratchRecord, abbrCode, theAttr->isImplicit(), ids);
+      return;
+    }
+
     case DeclAttrKind::SPIAccessControl: {
       auto theAttr = cast<SPIAccessControlAttr>(DA);
       auto abbrCode = S.DeclTypeAbbrCodes[SPIAccessControlDeclAttrLayout::Code];

From fc538f32fb76ced6a72b599e819a038b3cac3070 Mon Sep 17 00:00:00 2001
From: John McCall <rjmccall@apple.com>
Date: Fri, 1 Mar 2024 02:39:13 -0500
Subject: [PATCH 3/3] Allow declarations to opt in to suppressing
 @isolated(any).

---
 include/swift/AST/PrintOptions.h              |  4 +++-
 include/swift/Basic/Features.def              |  2 +-
 lib/AST/ASTPrinter.cpp                        | 10 ++++++++-
 lib/AST/Attr.cpp                              | 16 ++++++++++++++
 .../isolated_any_suppression.swift            | 22 +++++++++++++++++++
 5 files changed, 51 insertions(+), 3 deletions(-)
 create mode 100644 test/ModuleInterface/isolated_any_suppression.swift

diff --git a/include/swift/AST/PrintOptions.h b/include/swift/AST/PrintOptions.h
index c9d3651d8c851..7eda6480194f1 100644
--- a/include/swift/AST/PrintOptions.h
+++ b/include/swift/AST/PrintOptions.h
@@ -376,11 +376,13 @@ struct PrintOptions {
   /// `AsyncIteratorProtocol` for backward-compatibility reasons.
   bool AsyncSequenceRethrows = false;
 
+  /// Suppress the @isolated(any) attribute.
+  bool SuppressIsolatedAny = false;
+
   /// List of attribute kinds that should not be printed.
   std::vector<AnyAttrKind> ExcludeAttrList = {
       DeclAttrKind::Transparent, DeclAttrKind::Effects,
       DeclAttrKind::FixedLayout, DeclAttrKind::ShowInInterface,
-      DeclAttrKind::AllowFeatureSuppression
   };
 
   /// List of attribute kinds that should be printed exclusively.
diff --git a/include/swift/Basic/Features.def b/include/swift/Basic/Features.def
index 610e04a2c42d0..3f3a253b80a35 100644
--- a/include/swift/Basic/Features.def
+++ b/include/swift/Basic/Features.def
@@ -335,7 +335,7 @@ EXPERIMENTAL_FEATURE(DynamicActorIsolation, false)
 EXPERIMENTAL_FEATURE(BorrowingSwitch, true)
 
 // Enable isolated(any) attribute on function types.
-EXPERIMENTAL_FEATURE(IsolatedAny, true)
+CONDITIONALLY_SUPPRESSIBLE_EXPERIMENTAL_FEATURE(IsolatedAny, true)
 
 #undef EXPERIMENTAL_FEATURE_EXCLUDED_FROM_MODULE_INTERFACE
 #undef EXPERIMENTAL_FEATURE
diff --git a/lib/AST/ASTPrinter.cpp b/lib/AST/ASTPrinter.cpp
index e5f7e91a0e337..b219d7e1ab1f7 100644
--- a/lib/AST/ASTPrinter.cpp
+++ b/lib/AST/ASTPrinter.cpp
@@ -366,6 +366,7 @@ PrintOptions PrintOptions::printSwiftInterfaceFile(ModuleDecl *ModuleToPrint,
       DeclAttrKind::StaticInitializeObjCMetadata,
       DeclAttrKind::RestatedObjCConformance,
       DeclAttrKind::NonSendable,
+      DeclAttrKind::AllowFeatureSuppression,
   };
 
   return result;
@@ -3101,6 +3102,12 @@ static void suppressingFeatureExtern(PrintOptions &options,
   options.ExcludeAttrList.resize(originalExcludeAttrCount);
 }
 
+static void suppressingFeatureIsolatedAny(PrintOptions &options,
+                                          llvm::function_ref<void()> action) {
+  llvm::SaveAndRestore<bool> scope(options.SuppressIsolatedAny, true);
+  action();
+}
+
 /// Suppress the printing of a particular feature.
 static void suppressingFeature(PrintOptions &options, Feature feature,
                                llvm::function_ref<void()> action) {
@@ -6315,7 +6322,8 @@ class TypePrinter : public TypeVisitor<TypePrinter> {
       break;
 
     case FunctionTypeIsolation::Kind::Erased:
-      Printer << "@isolated(any) ";
+      if (!Options.SuppressIsolatedAny)
+        Printer << "@isolated(any) ";
       break;
     }
 
diff --git a/lib/AST/Attr.cpp b/lib/AST/Attr.cpp
index 85f12996a11de..5d8424e5d860a 100644
--- a/lib/AST/Attr.cpp
+++ b/lib/AST/Attr.cpp
@@ -254,6 +254,13 @@ const char *IsolatedTypeAttr::getIsolationKindName(IsolationKind kind) {
 
 void IsolatedTypeAttr::printImpl(ASTPrinter &printer,
                                  const PrintOptions &options) const {
+  // Suppress the attribute if requested.
+  switch (getIsolationKind()) {
+  case IsolationKind::Dynamic:
+    if (options.SuppressIsolatedAny) return;
+    break;
+  }
+
   printer.callPrintStructurePre(PrintStructureKind::BuiltinAttribute);
   printer.printAttrName("@isolated");
   printer << "(" << getIsolationKindName() << ")";
@@ -1254,6 +1261,15 @@ bool DeclAttribute::printImpl(ASTPrinter &Printer, const PrintOptions &Options,
     Printer << "(" << cast<AlignmentAttr>(this)->getValue() << ")";
     break;
 
+  case DeclAttrKind::AllowFeatureSuppression:
+    Printer.printAttrName("@_allowFeatureSuppression");
+    Printer << "(";
+    interleave(cast<AllowFeatureSuppressionAttr>(this)->getSuppressedFeatures(),
+               [&](Identifier ident) { Printer << ident; },
+               [&] { Printer << ", "; });
+    Printer << ")";
+    break;
+
   case DeclAttrKind::SILGenName:
     Printer.printAttrName("@_silgen_name");
     Printer << "(\"" << cast<SILGenNameAttr>(this)->Name << "\")";
diff --git a/test/ModuleInterface/isolated_any_suppression.swift b/test/ModuleInterface/isolated_any_suppression.swift
new file mode 100644
index 0000000000000..81af0fa18d201
--- /dev/null
+++ b/test/ModuleInterface/isolated_any_suppression.swift
@@ -0,0 +1,22 @@
+// RUN: %empty-directory(%t)
+
+// RUN: %target-swift-frontend -swift-version 5 -enable-library-evolution -module-name isolated_any -emit-module -o %t/isolated_any.swiftmodule -emit-module-interface-path -  -enable-experimental-feature IsolatedAny %s | %FileCheck %s
+
+// CHECK:      #if compiler(>=5.3) && $IsolatedAny
+// CHECK-NEXT: {{^}}public func test1(fn: @isolated(any) () -> ())
+// CHECK-NEXT: #endif
+public func test1(fn: @isolated(any) () -> ()) {}
+
+// CHECK-NEXT: #if compiler(>=5.3) && $IsolatedAny
+// CHECK-NEXT: {{^}}public func test2(fn: @isolated(any) () -> ())
+// CHECK-NEXT: #endif
+@_allowFeatureSuppression(XXX)
+public func test2(fn: @isolated(any) () -> ()) {}
+
+// CHECK-NEXT: #if compiler(>=5.3) && $IsolatedAny
+// CHECK-NEXT: {{^}}public func test3(fn: @isolated(any) () -> ())
+// CHECK-NEXT: #else
+// CHECK-NEXT: {{^}}public func test3(fn: () -> ())
+// CHECK-NEXT: #endif
+@_allowFeatureSuppression(IsolatedAny)
+public func test3(fn: @isolated(any) () -> ()) {}