Merge pull request #77765 from hamishknight/thunk-fix

hamishknight · web-flow · commit 3e95e8191096 · 2024-11-21T19:01:53.000Z
[CS] Use adjusted type for single curry thunks
diff --git a/lib/Sema/CSApply.cpp b/lib/Sema/CSApply.cpp
@@ -759,22 +759,27 @@ namespace {
           new (ctx) DeclRefExpr(ref, loc, implicit, semantics, fullType);
       cs.cacheType(declRefExpr);
       declRefExpr->setFunctionRefKind(choice.getFunctionRefKind());
-      Expr *result = adjustTypeForDeclReference(
-          declRefExpr, fullType, adjustedFullType, locator);
-      // If we have to load, do so now.
-      if (loadImmediately)
-        result = cs.addImplicitLoadExpr(result);
 
-      result = forceUnwrapIfExpected(result, locator);
+      Expr *result = forceUnwrapIfExpected(declRefExpr, locator);
 
       if (auto *fnDecl = dyn_cast<AbstractFunctionDecl>(decl)) {
         if (AnyFunctionRef(fnDecl).hasExternalPropertyWrapperParameters() &&
             (declRefExpr->getFunctionRefKind() == FunctionRefKind::Compound ||
              declRefExpr->getFunctionRefKind() == FunctionRefKind::Unapplied)) {
-          result = buildSingleCurryThunk(result, fnDecl, locator);
+          // We don't need to do any further adjustment once we've built the
+          // curry thunk.
+          return buildSingleCurryThunk(result, fnDecl,
+                                       adjustedFullType->castTo<FunctionType>(),
+                                       locator);
         }
       }
 
+      result = adjustTypeForDeclReference(result, fullType, adjustedFullType,
+                                          locator);
+      // If we have to load, do so now.
+      if (loadImmediately)
+        result = cs.addImplicitLoadExpr(result);
+
       return result;
     }
 
@@ -1406,41 +1411,20 @@ namespace {
     /// parameters.
     /// \param declOrClosure The underlying function-like declaration or
     /// closure we're going to call.
+    /// \param thunkTy The type of the resulting thunk. This should be the
+    /// type of the \c fnExpr, with any potential adjustments for things like
+    /// concurrency.
     /// \param locator The locator pinned on the function reference carried
     /// by \p fnExpr. If the function has associated applied property wrappers,
     /// the locator is used to pull them in.
     AutoClosureExpr *buildSingleCurryThunk(Expr *fnExpr,
                                            DeclContext *declOrClosure,
+                                           FunctionType *thunkTy,
                                            ConstraintLocatorBuilder locator) {
-      auto *const thunkTy = cs.getType(fnExpr)->castTo<FunctionType>();
-
       return buildSingleCurryThunk(/*baseExpr=*/nullptr, fnExpr, declOrClosure,
                                    thunkTy, locator);
     }
 
-    /// Build a "{ args in base.fn(args) }" single-expression curry thunk.
-    ///
-    /// \param baseExpr The base expression to be captured.
-    /// \param fnExpr The expression to be called by consecutively applying
-    /// the \p baseExpr and thunk parameters.
-    /// \param declOrClosure The underlying function-like declaration or
-    /// closure we're going to call.
-    /// \param locator The locator pinned on the function reference carried
-    /// by \p fnExpr. If the function has associated applied property wrappers,
-    /// the locator is used to pull them in.
-    AutoClosureExpr *buildSingleCurryThunk(Expr *baseExpr, Expr *fnExpr,
-                                           DeclContext *declOrClosure,
-                                           ConstraintLocatorBuilder locator) {
-      assert(baseExpr);
-      auto *const thunkTy = cs.getType(fnExpr)
-                                ->castTo<FunctionType>()
-                                ->getResult()
-                                ->castTo<FunctionType>();
-
-      return buildSingleCurryThunk(baseExpr, fnExpr, declOrClosure, thunkTy,
-                                   locator);
-    }
-
     /// Build a "{ self in { args in self.fn(args) } }" nested curry thunk.
     ///
     /// \param memberRef The expression to be called in the inner thunk by
@@ -1577,8 +1561,8 @@ namespace {
                          ConstraintLocatorBuilder memberLocator, bool Implicit,
                          AccessSemantics semantics) {
       const auto &choice = overload.choice;
-      const auto openedType = overload.openedType;
-      const auto adjustedOpenedType = overload.adjustedOpenedType;
+      const auto openedType = simplifyType(overload.openedType);
+      const auto adjustedOpenedType = simplifyType(overload.adjustedOpenedType);
 
       ValueDecl *member = choice.getDecl();
 
@@ -1639,7 +1623,7 @@ namespace {
       // If we're referring to a member type, it's just a type
       // reference.
       if (auto *TD = dyn_cast<TypeDecl>(member)) {
-        Type refType = simplifyType(adjustedOpenedType);
+        Type refType = adjustedOpenedType;
         auto ref = TypeExpr::createForDecl(memberLoc, TD, dc);
         cs.setType(ref, refType);
         auto *result = new (context) DotSyntaxBaseIgnoredExpr(
@@ -1770,10 +1754,8 @@ namespace {
         ref->setImplicit(Implicit);
         // FIXME: FunctionRefKind
 
-        auto computeRefType = [&](Type openedType) {
-          // Compute the type of the reference.
-          Type refType = simplifyType(openedType);
-
+        // Compute the type of the reference.
+        auto computeRefType = [&](Type refType) {
           // If the base was an opened existential, erase the opened
           // existential.
           if (openedExistential) {
@@ -1859,7 +1841,7 @@ namespace {
         // type having 'Self' swapped for the appropriate replacement
         // type -- usually the base object type.
         if (hasDynamicSelf) {
-          const auto conversionTy = simplifyType(adjustedOpenedType);
+          const auto conversionTy = adjustedOpenedType;
           if (!containerTy->isEqual(conversionTy)) {
             result = cs.cacheType(new (context) CovariantReturnConversionExpr(
                 result, conversionTy));
@@ -1911,9 +1893,10 @@ namespace {
       // have side effects, instead of abstracting out a 'self' parameter.
       const auto isSuperPartialApplication = needsCurryThunk && isSuper;
       if (isSuperPartialApplication) {
-        ref = buildSingleCurryThunk(base, declRefExpr,
-                                    cast<AbstractFunctionDecl>(member),
-                                    memberLocator);
+        ref = buildSingleCurryThunk(
+            base, declRefExpr, cast<AbstractFunctionDecl>(member),
+            adjustedOpenedType->castTo<FunctionType>(),
+            memberLocator);
       } else if (needsCurryThunk) {
         // Another case where we want to build a single closure is when
         // we have a partial application of a static member. It is better
@@ -1929,14 +1912,13 @@ namespace {
                                              cs.getType(base));
             cs.setType(base, base->getType());
 
-            auto *closure = buildSingleCurryThunk(
-                base, declRefExpr, cast<AbstractFunctionDecl>(member),
-                memberLocator);
-
             // Skip the code below -- we're not building an extra level of
             // call by applying the metatype; instead, the closure we just
             // built is the curried reference.
-            return closure;
+            return buildSingleCurryThunk(
+                base, declRefExpr, cast<AbstractFunctionDecl>(member),
+                adjustedOpenedType->castTo<FunctionType>(),
+                memberLocator);
           } else {
             // Add a useless ".self" to avoid downstream diagnostics, in case
             // the type ref is still a TypeExpr.
@@ -1967,7 +1949,7 @@ namespace {
             
             auto *closure = buildSingleCurryThunk(
               baseRef, declRefExpr, cast<AbstractFunctionDecl>(member),
-              simplifyType(adjustedOpenedType)->castTo<FunctionType>(),
+              adjustedOpenedType->castTo<FunctionType>(),
               memberLocator);
               
             // Wrap the closure in a capture list.
@@ -1994,7 +1976,7 @@ namespace {
           // must be downcast to the opened archetype before being erased to the
           // subclass existential to cope with the expectations placed
           // on 'CovariantReturnConversionExpr'.
-          curryThunkTy = simplifyType(adjustedOpenedType)->castTo<FunctionType>();
+          curryThunkTy = adjustedOpenedType->castTo<FunctionType>();
         } else {
           curryThunkTy = adjustedRefTy->castTo<FunctionType>();
 
@@ -2060,7 +2042,7 @@ namespace {
         apply = ConstructorRefCallExpr::create(context, ref, base);
       } else if (isUnboundInstanceMember) {
         ref = adjustTypeForDeclReference(
-            ref, cs.getType(ref), cs.simplifyType(adjustedOpenedType),
+            ref, cs.getType(ref), adjustedOpenedType,
             locator);
 
         // Reference to an unbound instance method.
@@ -8876,8 +8858,10 @@ namespace {
         rewriteFunction(closure);
 
         if (AnyFunctionRef(closure).hasExternalPropertyWrapperParameters()) {
+          auto *thunkTy = Rewriter.cs.getType(closure)->castTo<FunctionType>();
           return Action::SkipNode(Rewriter.buildSingleCurryThunk(
-              closure, closure, Rewriter.cs.getConstraintLocator(closure)));
+              closure, closure, thunkTy,
+              Rewriter.cs.getConstraintLocator(closure)));
         }
 
         return Action::SkipNode(closure);
diff --git a/lib/Sema/TypeCheckConcurrency.cpp b/lib/Sema/TypeCheckConcurrency.cpp
@@ -4458,17 +4458,21 @@ namespace {
     }
 
     // Attempt to resolve the global actor type of a closure.
-    Type resolveGlobalActorType(ClosureExpr *closure) {
+    Type resolveGlobalActorType(AbstractClosureExpr *ACE) {
       // Check whether the closure's type has a global actor already.
-      if (Type closureType = getType(closure)) {
+      if (Type closureType = getType(ACE)) {
         if (auto closureFnType = closureType->getAs<FunctionType>()) {
           if (Type globalActor = closureFnType->getGlobalActor())
             return globalActor;
         }
       }
 
       // Look for an explicit attribute.
-      return getExplicitGlobalActor(closure);
+      if (auto *CE = dyn_cast<ClosureExpr>(ACE)) {
+        if (auto globalActor = getExplicitGlobalActor(CE))
+          return globalActor;
+      }
+      return Type();
     }
 
   public:
@@ -4480,14 +4484,16 @@ namespace {
         AbstractClosureExpr *closure) {
       bool preconcurrency = false;
 
-      if (auto explicitClosure = dyn_cast<ClosureExpr>(closure)) {
+      if (auto explicitClosure = dyn_cast<ClosureExpr>(closure))
         preconcurrency = explicitClosure->isIsolatedByPreconcurrency();
 
-        // If the closure specifies a global actor, use it.
-        if (Type globalActor = resolveGlobalActorType(explicitClosure))
-          return ActorIsolation::forGlobalActor(globalActor)
-              .withPreconcurrency(preconcurrency);
+      // If the closure specifies a global actor, use it.
+      if (Type globalActor = resolveGlobalActorType(closure)) {
+        return ActorIsolation::forGlobalActor(globalActor)
+          .withPreconcurrency(preconcurrency);
+      }
 
+      if (auto *explicitClosure = dyn_cast<ClosureExpr>(closure)) {
         if (auto *attr =
                 explicitClosure->getAttrs().getAttribute<NonisolatedAttr>();
             attr && ctx.LangOpts.hasFeature(Feature::ClosureIsolation)) {
diff --git a/test/Concurrency/global_actor_function_types.swift b/test/Concurrency/global_actor_function_types.swift
@@ -413,3 +413,28 @@ extension GlobalType {
     rhs: GlobalType
   ) -> Bool { true }
 }
+
+func takesMainActorFn(_ x: @MainActor () -> Int) {}
+func takesMainActorAutoclosure(_ x: @autoclosure @MainActor () -> Int) {}
+
+func nonisolatedIntFn() -> Int { 0 }
+@MainActor func mainActorIntFn() -> Int { 0 }
+
+struct HasMainActorFns {
+  @MainActor static func staticFn() -> Int { 0 }
+  @MainActor func instanceFn() -> Int { 0 }
+}
+
+func testGlobalActorAutoclosure(_ x: HasMainActorFns) {
+  takesMainActorFn(nonisolatedIntFn)
+  takesMainActorFn(mainActorIntFn)
+
+  // Make sure we respect global actors on autoclosures.
+  takesMainActorAutoclosure(nonisolatedIntFn())
+  takesMainActorAutoclosure(mainActorIntFn())
+
+  // Including autoclosure thunks.
+  takesMainActorFn(HasMainActorFns.staticFn)
+  takesMainActorFn(HasMainActorFns.instanceFn(x))
+  takesMainActorFn(x.instanceFn)
+}
diff --git a/test/Concurrency/preconcurrency_anyobject_lookup.swift b/test/Concurrency/preconcurrency_anyobject_lookup.swift
@@ -0,0 +1,17 @@
+// RUN: %target-swift-emit-silgen -verify -disable-objc-attr-requires-foundation-module %s
+// RUN: %target-swift-emit-silgen -verify -swift-version 6 -disable-objc-attr-requires-foundation-module %s
+
+// REQUIRES: objc_interop
+
+@objc class C {
+  @preconcurrency @objc func foo(_ x: Sendable) {}
+}
+
+func bar(_ fn: (Any) -> Void) {}
+func bar(_ fn: (Sendable) -> Void) {}
+
+// Make sure we can handle both the implicit unwrap and concurrency adjustment.
+func foo(_ x: AnyObject) {
+  bar(x.foo)
+  let _ = AnyObject.foo
+}
diff --git a/test/Constraints/rdar140212823.swift b/test/Constraints/rdar140212823.swift
@@ -0,0 +1,29 @@
+// RUN: %target-swift-emit-silgen %s -verify -swift-version 6
+
+// rdar://140212823 - Make sure we build curry thunks using the adjusted
+// reference type, such that the ParenExpr agrees with the type.
+
+class C: @unchecked Sendable {
+  func foo() {}
+}
+class D: C, @unchecked Sendable {
+  func bar() {
+    let _ = (super.foo)
+  }
+}
+
+struct S {
+  func instanceMethod() {}
+  func foo() {
+    let _ = (self.instanceMethod)
+  }
+  static func staticMethod() {}
+}
+
+let _ = (S.instanceMethod)
+let _ = (type(of: S()).instanceMethod)
+
+let _ = (S.staticMethod)
+let _ = (type(of: S()).staticMethod)
+
+let _: (Int, Int) -> Int = (+)
diff --git a/test/SILGen/dynamic_self.swift b/test/SILGen/dynamic_self.swift
@@ -362,7 +362,7 @@ class Base {
 
 class Derived : Base {
   // CHECK-LABEL: sil hidden [ossa] @$s12dynamic_self7DerivedC9superCallyyF : $@convention(method) (@guaranteed Derived) -> ()
-  // CHECK: convert_function %{{[0-9]+}} : $@callee_guaranteed () -> @owned Base to $@callee_guaranteed () -> @owned Derived
+  // CHECK: function_ref @$s12dynamic_self7DerivedC9superCallyyFACycfu_ : $@convention(thin) (@guaranteed Derived) -> @owned Derived
   // CHECK-COUNT-3: unchecked_ref_cast %{{[0-9]+}} : $Base to $Derived
   // CHECK-NOT: unchecked_ref_cast
   // CHECK: end sil function '$s12dynamic_self7DerivedC9superCallyyF'
@@ -372,9 +372,12 @@ class Derived : Base {
     _ = super.property
     _ = super[]
   }
+  // CHECK-LABEL: sil private [ossa] @$s12dynamic_self7DerivedC9superCallyyFACycfu_ : $@convention(thin) (@guaranteed Derived) -> @owned Derived
+  // CHECK: unchecked_ref_cast %{{[0-9]+}} : $Base to $Derived
+  // CHECK: end sil function '$s12dynamic_self7DerivedC9superCallyyFACycfu_'
 
   // CHECK-LABEL: sil hidden [ossa] @$s12dynamic_self7DerivedC15superCallStaticyyFZ : $@convention(method) (@thick Derived.Type) -> ()
-  // CHECK: convert_function %{{[0-9]+}} : $@callee_guaranteed () -> @owned Base to $@callee_guaranteed () -> @owned Derived
+  // CHECK: function_ref @$s12dynamic_self7DerivedC15superCallStaticyyFZACycfu_ : $@convention(thin) (@thick Derived.Type) -> @owned Derived
   // CHECK-COUNT-3: unchecked_ref_cast %{{[0-9]+}} : $Base to $Derived
   // CHECK-NOT: unchecked_ref_cast
   // CHECK: end sil function '$s12dynamic_self7DerivedC15superCallStaticyyFZ'
@@ -384,9 +387,12 @@ class Derived : Base {
     _ = super.staticProperty
     _ = super[]
   }
+  // CHECK-LABEL: sil private [ossa] @$s12dynamic_self7DerivedC15superCallStaticyyFZACycfu_ : $@convention(thin) (@thick Derived.Type) -> @owned Derived
+  // CHECK: unchecked_ref_cast %{{[0-9]+}} : $Base to $Derived
+  // CHECK: end sil function '$s12dynamic_self7DerivedC15superCallStaticyyFZACycfu_'
 
   // CHECK-LABEL: sil hidden [ossa] @$s12dynamic_self7DerivedC32superCallFromMethodReturningSelfACXDyF : $@convention(method) (@guaranteed Derived) -> @owned Derived
-  // CHECK: convert_function %{{[0-9]+}} : $@callee_guaranteed () -> @owned Base to $@callee_guaranteed () -> @owned Derived
+  // CHECK: function_ref @$s12dynamic_self7DerivedC32superCallFromMethodReturningSelfACXDyFACXDycfu_ : $@convention(thin) (@guaranteed Derived) -> @owned Derived
   // CHECK-COUNT-3: unchecked_ref_cast %{{[0-9]+}} : $Base to $Derived
   // CHECK-NOT: unchecked_ref_cast
   // CHECK: end sil function '$s12dynamic_self7DerivedC32superCallFromMethodReturningSelfACXDyF'
@@ -396,9 +402,12 @@ class Derived : Base {
     _ = super[]
     return super.property
   }
+  // CHECK-LABEL: sil private [ossa] @$s12dynamic_self7DerivedC32superCallFromMethodReturningSelfACXDyFACXDycfu_ : $@convention(thin) (@guaranteed Derived) -> @owned Derived
+  // CHECK: unchecked_ref_cast %{{[0-9]+}} : $Base to $Derived
+  // CHECK: end sil function '$s12dynamic_self7DerivedC32superCallFromMethodReturningSelfACXDyFACXDycfu_'
 
   // CHECK-LABEL: sil hidden [ossa] @$s12dynamic_self7DerivedC38superCallFromMethodReturningSelfStaticACXDyFZ : $@convention(method) (@thick Derived.Type) -> @owned Derived
-  // CHECK: convert_function %{{[0-9]+}} : $@callee_guaranteed () -> @owned Base to $@callee_guaranteed () -> @owned Derived
+  // CHECK: function_ref @$s12dynamic_self7DerivedC38superCallFromMethodReturningSelfStaticACXDyFZACXDycfu_ : $@convention(thin) (@thick @dynamic_self Derived.Type) -> @owned Derived
   // CHECK-COUNT-3: unchecked_ref_cast %{{[0-9]+}} : $Base to $Derived
   // CHECK-NOT: unchecked_ref_cast
   // CHECK: end sil function '$s12dynamic_self7DerivedC38superCallFromMethodReturningSelfStaticACXDyFZ'
@@ -408,6 +417,9 @@ class Derived : Base {
     _ = super[]
     return super.staticProperty
   }
+  // CHECK-LABEL: sil private [ossa] @$s12dynamic_self7DerivedC38superCallFromMethodReturningSelfStaticACXDyFZACXDycfu_ : $@convention(thin) (@thick @dynamic_self Derived.Type) -> @owned Derived
+  // CHECK: unchecked_ref_cast %{{[0-9]+}} : $Base to $Derived
+  // CHECK: end sil function '$s12dynamic_self7DerivedC38superCallFromMethodReturningSelfStaticACXDyFZACXDycfu_'
 }
 
 class Generic<T> {
diff --git a/test/SILGen/preconcurrency.swift b/test/SILGen/preconcurrency.swift
