Merge pull request swiftlang#80843 from xedin/fix-printing-of-specifiers-and-attributes

[AST] ASTPrinter: Improvements to specifier and attribute printing

Author: xedin

include/swift/AST/Decl.h

@@ -6902,6 +6902,9 @@ class ParamDecl : public VarDecl {
 
     /// Whether or not this parameter is 'sending'.
     IsSending = 1 << 4,
+
+    /// Whether or not this parameter is isolated to a caller.
+    IsCallerIsolated = 1 << 5,
   };
 
   /// The type repr and 3 bits used for flags.
@@ -7190,6 +7193,18 @@ class ParamDecl : public VarDecl {
       removeFlag(Flag::IsSending);
   }
 
+  /// Whether or not this parameter is marked with 'nonisolated(nonsending)'.
+  bool isCallerIsolated() const {
+    return getOptions().contains(Flag::IsCallerIsolated);
+  }
+
+  void setCallerIsolated(bool value = true) {
+    if (value)
+      addFlag(Flag::IsCallerIsolated);
+    else
+      removeFlag(Flag::IsCallerIsolated);
+  }
+
   /// Whether or not this parameter is marked with '@_addressable'.
   bool isAddressable() const {
     return getOptions().contains(Flag::IsAddressable);

lib/AST/ASTPrinter.cpp

@@ -3848,13 +3848,13 @@ static void printParameterFlags(ASTPrinter &printer,
                                 const PrintOptions &options,
                                 const ParamDecl *param,
                                 ParameterTypeFlags flags,
-                                bool escaping) {
-  if (!options.excludeAttrKind(TypeAttrKind::Autoclosure) &&
-      flags.isAutoClosure())
-    printer.printAttrName("@autoclosure ");
-  if (!options.excludeAttrKind(TypeAttrKind::NoDerivative) &&
-      flags.isNoDerivative())
-    printer.printAttrName("@noDerivative ");
+                                bool escaping,
+                                bool isIsolatedToCaller = false) {
+  // Always print `nonisolated(nonsending)` specifier on a parameter
+  // first, to avoid any issues with ordering.
+  if (isIsolatedToCaller) {
+    printer.printKeyword("nonisolated(nonsending)", options, " ");
+  }
 
   switch (flags.getOwnershipSpecifier()) {
   case ParamSpecifier::Default:
@@ -3883,7 +3883,7 @@ static void printParameterFlags(ASTPrinter &printer,
 
   if (flags.isSending()) {
     if (!options.SuppressSendingArgsAndResults) {
-      printer.printAttrName("sending ");
+      printer.printKeyword("sending", options, " ");
     } else if (flags.getOwnershipSpecifier() ==
                ParamSpecifier::ImplicitlyCopyableConsuming) {
       // Ok. We are suppressing sending. If our ownership specifier was
@@ -3901,14 +3901,24 @@ static void printParameterFlags(ASTPrinter &printer,
       printer.printKeyword("isolated", options, " ");
   }
 
-  if (!options.excludeAttrKind(TypeAttrKind::Escaping) && escaping)
-    printer.printKeyword("@escaping", options, " ");
-
   if (flags.isCompileTimeLiteral())
     printer.printKeyword("_const", options, " ");
-  
+
+  if (!options.excludeAttrKind(TypeAttrKind::Autoclosure) &&
+      flags.isAutoClosure())
+    printer.printAttrName("@autoclosure ");
+  if (!options.excludeAttrKind(TypeAttrKind::NoDerivative) &&
+      flags.isNoDerivative())
+    printer.printAttrName("@noDerivative ");
+
+  // `inout` implies `@escaping`
+  if (flags.getOwnershipSpecifier() != ParamSpecifier::InOut) {
+    if (!options.excludeAttrKind(TypeAttrKind::Escaping) && escaping)
+      printer.printAttrName("@escaping ");
+  }
+
   if (flags.isConstValue())
-    printer.printKeyword("@const", options, " ");
+    printer.printAttrName("@const ");
 }
 
 void PrintAST::visitVarDecl(VarDecl *decl) {
@@ -4024,11 +4034,24 @@ void PrintAST::printOneParameter(const ParamDecl *param,
 
   printArgName();
 
+  auto interfaceTy = param->getInterfaceType();
+
+  // If type of this parameter is isolated to a caller, let's
+  // strip the isolation from the type to avoid printing it as
+  // part of the function type because that would break ordering
+  // between specifiers and attributes.
+  if (param->isCallerIsolated()) {
+    if (auto *funcTy = dyn_cast<AnyFunctionType>(interfaceTy.getPointer())) {
+      interfaceTy =
+          funcTy->withIsolation(FunctionTypeIsolation::forNonIsolated());
+    }
+  }
+
   TypeLoc TheTypeLoc;
   if (auto *repr = param->getTypeRepr()) {
-    TheTypeLoc = TypeLoc(repr, param->getInterfaceType());
+    TheTypeLoc = TypeLoc(repr, interfaceTy);
   } else {
-    TheTypeLoc = TypeLoc::withoutLoc(param->getInterfaceType());
+    TheTypeLoc = TypeLoc::withoutLoc(interfaceTy);
   }
 
   {
@@ -4040,7 +4063,8 @@ void PrintAST::printOneParameter(const ParamDecl *param,
         !willUseTypeReprPrinting(TheTypeLoc, CurrentType, Options)) {
       auto type = TheTypeLoc.getType();
       printParameterFlags(Printer, Options, param, paramFlags,
-                          isEscaping(type));
+                          isEscaping(type),
+                          param->isCallerIsolated());
     }
 
     printTypeLocForImplicitlyUnwrappedOptional(

lib/AST/Decl.cpp

@@ -8940,6 +8940,12 @@ void ParamDecl::setTypeRepr(TypeRepr *repr) {
         continue;
       }
 
+      if (auto *callerIsolated =
+              dyn_cast<CallerIsolatedTypeRepr>(unwrappedType)) {
+        setCallerIsolated(true);
+        unwrappedType = callerIsolated->getBase();
+      }
+
       break;
     }
   }
@@ -11025,6 +11031,9 @@ AccessorDecl *AccessorDecl::createParsed(
       if (subscriptParam->isSending())
         param->setSending();
 
+      if (subscriptParam->isCallerIsolated())
+        param->setCallerIsolated();
+
       newParams.push_back(param);
     }
 

lib/Serialization/Deserialization.cpp

@@ -4110,6 +4110,7 @@ class DeclDeserializer {
     bool isIsolated;
     bool isCompileTimeLiteral, isConstValue;
     bool isSending;
+    bool isCallerIsolated;
     uint8_t rawDefaultArg;
     TypeID defaultExprType;
     uint8_t rawDefaultArgIsolation;
@@ -4122,6 +4123,7 @@ class DeclDeserializer {
                                          isCompileTimeLiteral,
                                          isConstValue,
                                          isSending,
+                                         isCallerIsolated,
                                          rawDefaultArg,
                                          defaultExprType,
                                          rawDefaultArgIsolation,
@@ -4167,6 +4169,7 @@ class DeclDeserializer {
     param->setCompileTimeLiteral(isCompileTimeLiteral);
     param->setConstValue(isConstValue);
     param->setSending(isSending);
+    param->setCallerIsolated(isCallerIsolated);
 
     // Decode the default argument kind.
     // FIXME: Default argument expression, if available.

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 = 941; // remove @execution attr
+const uint16_t SWIFTMODULE_VERSION_MINOR = 942; // `isCallerIsolated` parameter flag
 
 /// A standard hash seed used for all string hashes in a serialized module.
 ///
@@ -1728,6 +1728,7 @@ namespace decls_block {
     BCFixed<1>,              // isCompileTimeLiteral?
     BCFixed<1>,              // isConst?
     BCFixed<1>,              // isSending?
+    BCFixed<1>,              // isCallerIsolated?
     DefaultArgumentField,    // default argument kind
     TypeIDField,             // default argument type
     ActorIsolationField,     // default argument isolation

lib/Serialization/Serialization.cpp

@@ -4745,6 +4745,7 @@ class Serializer::DeclSerializer : public DeclVisitor<DeclSerializer> {
         param->isCompileTimeLiteral(),
         param->isConstVal(),
         param->isSending(),
+        param->isCallerIsolated(),
         getRawStableDefaultArgumentKind(argKind),
         S.addTypeRef(defaultExprType),
         getRawStableActorIsolationKind(isolation.getKind()),

test/Constraints/diagnostics.swift

@@ -1127,7 +1127,7 @@ func rdar17170728() {
   }
 
   let _ = [i, j, k].reduce(0 as Int?) { // expected-error {{missing argument label 'into:' in call}}
-    // expected-error@-1 {{cannot convert value of type 'Int?' to expected argument type '(inout @escaping (Bool, Bool) -> Bool?, Int?) throws -> ()'}}
+    // expected-error@-1 {{cannot convert value of type 'Int?' to expected argument type '(inout (Bool, Bool) -> Bool?, Int?) throws -> ()'}}
     $0 && $1 ? $0 + $1 : ($0 ? $0 : ($1 ? $1 : nil))
     // expected-error@-1 {{binary operator '+' cannot be applied to two 'Bool' operands}}
   }

test/ModuleInterface/attrs.swift

@@ -100,3 +100,12 @@ public enum TestExtensible {
   case a
   case b
 }
+
+public struct TestPlacementOfAttrsAndSpecifiers {
+  // CHECK: public func test1<T>(_: sending @autoclosure () -> T)
+  public func test1<T>(_: sending @autoclosure () -> T) {}
+  // CHECK: public func test2<T>(_: borrowing @autoclosure () -> T)
+  public func test2<T>(_: borrowing @autoclosure () -> T) {}
+  // CHECK: public func test3<T>(_: inout () async -> T)
+  public func test3<T>(_: inout () async -> T) {}
+}

test/ModuleInterface/execution_behavior_attrs.swift

@@ -5,6 +5,18 @@
 
 // REQUIRES: concurrency
 
+public struct TestWithAttrs {
+  // CHECK: #if compiler(>=5.3) && $AsyncExecutionBehaviorAttributes
+  // CHECK-NEXT: public func test(_: nonisolated(nonsending) @escaping () async -> Swift.Void)
+  // CHECK-NEXT: #endif
+  public func test(_: nonisolated(nonsending) @escaping () async -> Void) {}
+
+  // CHECK: #if compiler(>=5.3) && $AsyncExecutionBehaviorAttributes
+  // CHECK-NEXT: public func withInOut(fn: nonisolated(nonsending) inout () async -> Swift.Void)
+  // CHECK-NEXT: #endif
+  public func withInOut(fn: nonisolated(nonsending) inout () async -> Void) {}
+}
+
 public struct Test {
   // CHECK: #if compiler(>=5.3) && $AsyncExecutionBehaviorAttributes
   // CHECK-NEXT: nonisolated(nonsending) public init() async

test/SymbolGraph/Symbols/Mixins/DeclarationFragments/Full/Function.swift

@@ -41,12 +41,12 @@ public func foo<S>(f: @escaping () -> (), ext int: Int = 2, s: S) where S: Seque
 // FOO-NEXT:          "spelling": ": "
 // FOO-NEXT:        },
 // FOO-NEXT:        {
-// FOO-NEXT:          "kind": "keyword",
-// FOO-NEXT:          "spelling": "@escaping"
+// FOO-NEXT:          "kind": "attribute",
+// FOO-NEXT:          "spelling": "@escaping "
 // FOO-NEXT:        },
 // FOO-NEXT:        {
 // FOO-NEXT:          "kind": "text",
-// FOO-NEXT:          "spelling": " () -> (), "
+// FOO-NEXT:          "spelling": "() -> (), "
 // FOO-NEXT:        },
 // FOO-NEXT:        {
 // FOO-NEXT:          "kind": "externalParam",