Merge pull request swiftlang#80853 from hamishknight/macro-async-warning

Author: hamishknight

include/swift/AST/DiagnosticEngine.h

@@ -390,6 +390,17 @@ namespace swift {
       return limitBehaviorUntilSwiftVersion(limit, languageMode);
     }
 
+    /// Limit the diagnostic behavior to warning until the next future
+    /// language mode.
+    ///
+    /// This should be preferred over passing the next major version to
+    /// `warnUntilSwiftVersion` to make it easier to find and update clients
+    /// when a new language mode is introduced.
+    ///
+    /// This helps stage in fixes for stricter diagnostics as warnings
+    /// until the next major language version.
+    InFlightDiagnostic &warnUntilFutureSwiftVersion();
+
     /// Limit the diagnostic behavior to warning until the specified version.
     ///
     /// This helps stage in fixes for stricter diagnostics as warnings

include/swift/AST/Expr.h

@@ -267,7 +267,7 @@ class alignas(8) Expr : public ASTAllocated<Expr> {
     Kind : 2
   );
 
-  SWIFT_INLINE_BITFIELD(ClosureExpr, AbstractClosureExpr, 1+1+1+1+1+1+1,
+  SWIFT_INLINE_BITFIELD(ClosureExpr, AbstractClosureExpr, 1+1+1+1+1+1+1+1,
     /// True if closure parameters were synthesized from anonymous closure
     /// variables.
     HasAnonymousClosureVars : 1,
@@ -295,7 +295,12 @@ class alignas(8) Expr : public ASTAllocated<Expr> {
     /// isolation checks when it either specifies or inherits isolation
     /// and was passed as an argument to a function that is not fully 
     /// concurrency checked.
-    RequiresDynamicIsolationChecking : 1
+    RequiresDynamicIsolationChecking : 1,
+
+    /// Whether this closure was type-checked as an argument to a macro. This
+    /// is only populated after type-checking, and only exists for diagnostic
+    /// logic. Do not add more uses of this.
+    IsMacroArgument : 1
   );
 
   SWIFT_INLINE_BITFIELD_FULL(BindOptionalExpr, Expr, 16,
@@ -4316,6 +4321,7 @@ class ClosureExpr : public AbstractClosureExpr {
     Bits.ClosureExpr.IsPassedToSendingParameter = false;
     Bits.ClosureExpr.NoGlobalActorAttribute = false;
     Bits.ClosureExpr.RequiresDynamicIsolationChecking = false;
+    Bits.ClosureExpr.IsMacroArgument = false;
   }
 
   SourceRange getSourceRange() const;
@@ -4394,6 +4400,17 @@ class ClosureExpr : public AbstractClosureExpr {
     Bits.ClosureExpr.RequiresDynamicIsolationChecking = value;
   }
 
+  /// Whether this closure was type-checked as an argument to a macro. This
+  /// is only populated after type-checking, and only exists for diagnostic
+  /// logic. Do not add more uses of this.
+  bool isMacroArgument() const {
+    return Bits.ClosureExpr.IsMacroArgument;
+  }
+
+  void setIsMacroArgument(bool value = true) {
+    Bits.ClosureExpr.IsMacroArgument = value;
+  }
+
   /// Determine whether this closure expression has an
   /// explicitly-specified result type.
   bool hasExplicitResultType() const { return ArrowLoc.isValid(); }

include/swift/Basic/Version.h

@@ -131,6 +131,13 @@ class Version {
   /// SWIFT_VERSION_MINOR.
   static Version getCurrentLanguageVersion();
 
+  /// Returns a major version to represent the next future language mode. This
+  /// exists to make it easier to find and update clients when a new language
+  /// mode is added.
+  static constexpr unsigned getFutureMajorLanguageVersion() {
+    return 7;
+  }
+
   // List of backward-compatibility versions that we permit passing as
   // -swift-version <vers>
   static std::array<StringRef, 4> getValidEffectiveVersions() {

lib/AST/DiagnosticEngine.cpp

@@ -452,7 +452,7 @@ InFlightDiagnostic::limitBehaviorUntilSwiftVersion(
     // version. We do this before limiting the behavior, because
     // wrapIn will result in the behavior of the wrapping diagnostic.
     if (limit >= DiagnosticBehavior::Warning) {
-      if (majorVersion > 6) {
+      if (majorVersion >= version::Version::getFutureMajorLanguageVersion()) {
         wrapIn(diag::error_in_a_future_swift_lang_mode);
       } else {
         wrapIn(diag::error_in_swift_lang_mode, majorVersion);
@@ -472,6 +472,11 @@ InFlightDiagnostic::limitBehaviorUntilSwiftVersion(
   return *this;
 }
 
+InFlightDiagnostic &InFlightDiagnostic::warnUntilFutureSwiftVersion() {
+  using namespace version;
+  return warnUntilSwiftVersion(Version::getFutureMajorLanguageVersion());
+}
+
 InFlightDiagnostic &
 InFlightDiagnostic::warnUntilSwiftVersion(unsigned majorVersion) {
   return limitBehaviorUntilSwiftVersion(DiagnosticBehavior::Warning,

lib/Basic/Version.cpp

@@ -181,16 +181,19 @@ std::optional<Version> Version::getEffectiveLanguageVersion() const {
     static_assert(SWIFT_VERSION_MAJOR == 6,
                   "getCurrentLanguageVersion is no longer correct here");
     return Version::getCurrentLanguageVersion();
-  case 7:
-    // Allow version '7' in asserts compilers *only* so that we can start
-    // testing changes planned for after Swift 6. Note that it's still not
-    // listed in `Version::getValidEffectiveVersions()`.
-    // FIXME: When Swift 7 becomes real, remove 'REQUIRES: swift7' from tests
-    //        using '-swift-version 7'.
+
+  // FIXME: When Swift 7 becomes real, remove 'REQUIRES: swift7' from tests
+  //        using '-swift-version 7'.
+
+  case Version::getFutureMajorLanguageVersion():
+    // Allow the future language mode version in asserts compilers *only* so
+    // that we can start testing changes planned for after the current latest
+    // language mode. Note that it'll not be listed in
+    // `Version::getValidEffectiveVersions()`.
 #ifdef NDEBUG
     LLVM_FALLTHROUGH;
 #else
-    return Version{7};
+    return Version{Version::getFutureMajorLanguageVersion()};
 #endif
   default:
     return std::nullopt;

lib/Sema/CSApply.cpp

@@ -5612,6 +5612,18 @@ namespace {
       E->setMacroRef(macroRef);
       E->setType(expandedType);
 
+      auto fnType =
+          simplifyType(overload.adjustedOpenedType)->castTo<FunctionType>();
+
+      auto newArgs = coerceCallArguments(
+          E->getArgs(), fnType, macroRef, /*applyExpr=*/nullptr,
+          cs.getConstraintLocator(E, ConstraintLocator::ApplyArgument),
+          solution.getAppliedPropertyWrappers(E));
+      if (!newArgs)
+        return nullptr;
+
+      E->setArgs(newArgs);
+
       // FIXME: Expansion should be lazy.
       // i.e. 'ExpandMacroExpansionExprRequest' should be sinked into
       // 'getRewritten()', and performed on-demand. Unfortunately that requires
@@ -6073,28 +6085,32 @@ static bool hasCurriedSelf(ConstraintSystem &cs, ConcreteDeclRef callee,
 static void applyContextualClosureFlags(Expr *expr, bool implicitSelfCapture,
                                         bool inheritActorContext,
                                         bool isPassedToSendingParameter,
-                                        bool requiresDynamicIsolationChecking) {
+                                        bool requiresDynamicIsolationChecking,
+                                        bool isMacroArg) {
   if (auto closure = dyn_cast<ClosureExpr>(expr)) {
     closure->setAllowsImplicitSelfCapture(implicitSelfCapture);
     closure->setInheritsActorContext(inheritActorContext);
     closure->setIsPassedToSendingParameter(isPassedToSendingParameter);
     closure->setRequiresDynamicIsolationChecking(
         requiresDynamicIsolationChecking);
+    closure->setIsMacroArgument(isMacroArg);
     return;
   }
 
   if (auto captureList = dyn_cast<CaptureListExpr>(expr)) {
     applyContextualClosureFlags(captureList->getClosureBody(),
                                 implicitSelfCapture, inheritActorContext,
                                 isPassedToSendingParameter,
-                                requiresDynamicIsolationChecking);
+                                requiresDynamicIsolationChecking,
+                                isMacroArg);
   }
 
   if (auto identity = dyn_cast<IdentityExpr>(expr)) {
     applyContextualClosureFlags(identity->getSubExpr(), implicitSelfCapture,
                                 inheritActorContext,
                                 isPassedToSendingParameter,
-                                requiresDynamicIsolationChecking);
+                                requiresDynamicIsolationChecking,
+                                isMacroArg);
   }
 }
 
@@ -6219,19 +6235,22 @@ ArgumentList *ExprRewriter::coerceCallArguments(
   }();
 
   auto applyFlagsToArgument = [&paramInfo,
-                               &closuresRequireDynamicIsolationChecking](
+                               &closuresRequireDynamicIsolationChecking,
+                               &locator](
                                   unsigned paramIdx, Expr *argument) {
     if (!isClosureLiteralExpr(argument))
       return;
 
     bool isImplicitSelfCapture = paramInfo.isImplicitSelfCapture(paramIdx);
     bool inheritsActorContext = paramInfo.inheritsActorContext(paramIdx);
     bool isPassedToSendingParameter = paramInfo.isSendingParameter(paramIdx);
+    bool isMacroArg = isExpr<MacroExpansionExpr>(locator.getAnchor());
 
     applyContextualClosureFlags(argument, isImplicitSelfCapture,
                                 inheritsActorContext,
                                 isPassedToSendingParameter,
-                                closuresRequireDynamicIsolationChecking);
+                                closuresRequireDynamicIsolationChecking,
+                                isMacroArg);
   };
 
   // Quickly test if any further fix-ups for the argument types are necessary.

lib/Sema/MiscDiagnostics.cpp

@@ -2239,11 +2239,12 @@ class ImplicitSelfUsageChecker : public BaseDiagnosticWalker {
         invalidImplicitSelfShouldOnlyWarn510(base, closure)) {
       warnUntilVersion.emplace(6);
     }
-    // Prior to Swift 7, downgrade to a warning if we're in a macro to preserve
-    // compatibility with the Swift 6 diagnostic behavior where we previously
-    // skipped diagnosing.
-    if (!Ctx.isSwiftVersionAtLeast(7) && isInMacro())
-      warnUntilVersion.emplace(7);
+    // Prior to the next language mode, downgrade to a warning if we're in a
+    // macro to preserve compatibility with the Swift 6 diagnostic behavior
+    // where we previously skipped diagnosing.
+    auto futureVersion = version::Version::getFutureMajorLanguageVersion();
+    if (!Ctx.isSwiftVersionAtLeast(futureVersion) && isInMacro())
+      warnUntilVersion.emplace(futureVersion);
 
     auto diag = Ctx.Diags.diagnose(loc, ID, std::move(Args)...);
     if (warnUntilVersion)

lib/Sema/TypeCheckAttr.cpp

@@ -1260,7 +1260,7 @@ void AttributeChecker::visitAccessControlAttr(AccessControlAttr *attr) {
             diagnose(attr->getLocation(),
                      diag::access_control_non_objc_open_member, VD)
                 .fixItReplace(attr->getRange(), "public")
-                .warnUntilSwiftVersion(7);
+                .warnUntilFutureSwiftVersion();
           }
         }
       }

lib/Sema/TypeCheckAvailability.cpp

@@ -276,16 +276,18 @@ static bool shouldAllowReferenceToUnavailableInSwiftDeclaration(
   return false;
 }
 
-// Utility function to help determine if noasync diagnostics are still
-// appropriate even if a `DeclContext` returns `false` from `isAsyncContext()`.
-static bool shouldTreatDeclContextAsAsyncForDiagnostics(const DeclContext *DC) {
-  if (auto *D = DC->getAsDecl())
-    if (auto *FD = dyn_cast<FuncDecl>(D))
+/// Retrieve the innermost DeclContext that should be consulted for noasync
+/// checking.
+static const DeclContext *
+getInnermostDeclContextForNoAsync(const DeclContext *DC) {
+  if (auto *D = DC->getAsDecl()) {
+    if (auto *FD = dyn_cast<FuncDecl>(D)) {
       if (FD->isDeferBody())
         // If this is a defer body, we should delegate to its parent.
-        return shouldTreatDeclContextAsAsyncForDiagnostics(DC->getParent());
-
-  return DC->isAsyncContext();
+        return getInnermostDeclContextForNoAsync(DC->getParent());
+    }
+  }
+  return DC;
 }
 
 /// A class that walks the AST to find the innermost (i.e., deepest) node that
@@ -2758,7 +2760,8 @@ static bool
 diagnoseDeclAsyncAvailability(const ValueDecl *D, SourceRange R,
                               const Expr *call, const ExportContext &Where) {
   // If we are not in an (effective) async context, don't check it
-  if (!shouldTreatDeclContextAsAsyncForDiagnostics(Where.getDeclContext()))
+  auto *noAsyncDC = getInnermostDeclContextForNoAsync(Where.getDeclContext());
+  if (!noAsyncDC->isAsyncContext())
     return false;
 
   ASTContext &ctx = Where.getDeclContext()->getASTContext();
@@ -2774,14 +2777,27 @@ diagnoseDeclAsyncAvailability(const ValueDecl *D, SourceRange R,
     }
   }
 
+  // In Swift 6 we previously didn't coerce macro arguments to parameter types,
+  // so closure arguments may be treated as async in cases where they weren't in
+  // Swift 6. As such we need to warn if the use is within a closure macro
+  // argument until the next language mode.
+  auto shouldWarnUntilFutureVersion = [&]() {
+    auto *CE = dyn_cast<ClosureExpr>(noAsyncDC);
+    return CE && CE->isMacroArgument();
+  };
+
   // @available(noasync) spelling
   if (auto attr = D->getNoAsyncAttr()) {
     SourceLoc diagLoc = call ? call->getLoc() : R.Start;
     auto diag = ctx.Diags.diagnose(diagLoc, diag::async_unavailable_decl, D,
                                    attr->getMessage());
-    diag.warnUntilSwiftVersion(6);
-    diag.limitBehaviorWithPreconcurrency(DiagnosticBehavior::Warning,
-                                         D->preconcurrency());
+    if (D->preconcurrency()) {
+      diag.limitBehavior(DiagnosticBehavior::Warning);
+    } else if (shouldWarnUntilFutureVersion()) {
+      diag.warnUntilFutureSwiftVersion();
+    } else {
+      diag.warnUntilSwiftVersion(6);
+    }
 
     if (!attr->getRename().empty()) {
       fixItAvailableAttrRename(diag, R, D, attr->getRename(), call);
@@ -2797,10 +2813,16 @@ diagnoseDeclAsyncAvailability(const ValueDecl *D, SourceRange R,
   // @_unavailableFromAsync spelling
   const UnavailableFromAsyncAttr *attr =
       D->getAttrs().getAttribute<UnavailableFromAsyncAttr>();
-  SourceLoc diagLoc = call ? call->getLoc() : R.Start;
-  ctx.Diags
-      .diagnose(diagLoc, diag::async_unavailable_decl, D, attr->Message)
-      .warnUntilSwiftVersion(6);
+  {
+    SourceLoc diagLoc = call ? call->getLoc() : R.Start;
+    auto diag = ctx.Diags.diagnose(diagLoc, diag::async_unavailable_decl, D,
+                                   attr->Message);
+    if (shouldWarnUntilFutureVersion()) {
+      diag.warnUntilFutureSwiftVersion();
+    } else {
+      diag.warnUntilSwiftVersion(6);
+    }
+  }
   D->diagnose(diag::decl_declared_here, D);
   return true;
 }

lib/Sema/TypeCheckConcurrency.cpp

@@ -2675,7 +2675,7 @@ namespace {
                     fromType, toType);
 
                 if (downgradeToWarning)
-                  diag.warnUntilSwiftVersion(7);
+                  diag.warnUntilFutureSwiftVersion();
               }
 
               for (auto type : nonSendableTypes) {