Merge pull request #4821 from swiftwasm/release/5.7

[pull] swiftwasm-release/5.7 from release/5.7

Author: kateinoigakukun

include/swift/AST/DiagnosticsSema.def

@@ -4590,6 +4590,10 @@ ERROR(distributed_actor_isolated_non_self_reference,none,
 ERROR(distributed_actor_needs_explicit_distributed_import,none,
       "'Distributed' module not imported, required for 'distributed actor'",
       ())
+ERROR(distributed_func_cannot_overload_on_async_only,none,
+      "ambiguous distributed func declaration %0, cannot overload distributed methods on effect only", (DeclName))
+NOTE(distributed_func_other_ambiguous_overload_here,none,
+      "ambiguous distributed func %0 declared here", (DeclName))
 ERROR(actor_isolated_inout_state,none,
       "actor-isolated %0 %1 cannot be passed 'inout' to"
       "%select{| implicitly}2 'async' function call",

lib/IRGen/GenDecl.cpp

@@ -3437,9 +3437,9 @@ IRGenModule::getAddrOfLLVMVariable(LinkEntity entity,
     ? overrideDeclType
     : entity.getDefaultDeclarationType(*this);
 
-  auto &entry = GlobalVars[entity];
-  if (entry) {
-    auto existing = cast<llvm::GlobalValue>(entry);
+  auto existingGlobal = GlobalVars[entity];
+  if (existingGlobal) {
+    auto existing = cast<llvm::GlobalValue>(existingGlobal);
 
     // If we're looking to define something, we may need to replace a
     // forward declaration.
@@ -3459,12 +3459,12 @@ IRGenModule::getAddrOfLLVMVariable(LinkEntity entity,
 
       // Fall out to the case below, clearing the name so that
       // createVariable doesn't detect a collision.
-      entry->setName("");
+      existingGlobal->setName("");
 
     // Otherwise, we have a previous declaration or definition which
     // we need to ensure has the right type.
     } else {
-      return getElementBitCast(entry, defaultType);
+      return getElementBitCast(existingGlobal, defaultType);
     }
   }
 
@@ -3508,11 +3508,17 @@ IRGenModule::getAddrOfLLVMVariable(LinkEntity entity,
     lazyInitializer->Create(var);
   }
 
+  if (lazyInitializer) {
+    // Protect against self-references that might've been created during
+    // the lazy emission.
+    existingGlobal = GlobalVars[entity];
+  }
+
   // If we have an existing entry, destroy it, replacing it with the
-  // new variable.
-  if (entry) {
-    auto existing = cast<llvm::GlobalValue>(entry);
-    auto castVar = llvm::ConstantExpr::getBitCast(var, entry->getType());
+  // new variable.  We only really have to do 
+  if (existingGlobal) {
+    auto existing = cast<llvm::GlobalValue>(existingGlobal);
+    auto castVar = llvm::ConstantExpr::getBitCast(var, existing->getType());
     existing->replaceAllUsesWith(castVar);
     existing->eraseFromParent();
   }
@@ -3535,7 +3541,7 @@ IRGenModule::getAddrOfLLVMVariable(LinkEntity entity,
   }
 
   // Cache and return.
-  entry = var;
+  GlobalVars[entity] = var;
   return var;
 }
 

lib/Sema/CSSimplify.cpp

@@ -3856,6 +3856,13 @@ ConstraintSystem::matchTypesBindTypeVar(
       // let's ignore this mismatch and mark affected type variable as a hole
       // because something else has to be fixed already for this to happen.
       if (type->is<DependentMemberType>() && !type->hasTypeVariable()) {
+        // Since the binding couldn't be performed, the type variable is a
+        // hole regardless whether it would be bound later to some other
+        // type or not. If this is not reflected in constraint system
+        // it would let the solver to form a _valid_ solution as if the
+        // constraint between the type variable and the unresolved dependent
+        // member type never existed.
+        increaseScore(SK_Hole);
         recordPotentialHole(typeVar);
         return getTypeMatchSuccess();
       }

lib/Sema/TypeCheckDistributed.cpp

@@ -677,10 +677,21 @@ void TypeChecker::checkDistributedActor(SourceFile *SF, NominalTypeDecl *nominal
   // If applicable, this will create the default 'init(transport:)' initializer
   (void)nominal->getDefaultInitializer();
 
+  // We check decls for ambiguity more strictly than normal nominal types,
+  // because we want to record distributed accessors the same if function they
+  // point at (in a remote process) is async or not, as that has no effect on
+  // a caller from a different process, so we want to make the remoteCall target
+  // identifiers, less fragile against such refactorings.
+  //
+  // To achieve this, we ban overloads on just "effects" of functions,
+  // which are useful in local settings, but really should not be relied
+  // on as differenciators in remote calls - the call will always be "async"
+  // since it will go through a thunk, and then be asynchronously transferred
+  // to the called process.
+  llvm::SmallDenseSet<DeclName, 2> diagnosedAmbiguity;
 
   for (auto member : nominal->getMembers()) {
-    // A distributed computed property needs to have a thunk for
-    // its getter accessor.
+    // --- Ensure 'distributed func' all thunks
     if (auto *var = dyn_cast<VarDecl>(member)) {
       if (!var->isDistributed())
         continue;
@@ -691,7 +702,7 @@ void TypeChecker::checkDistributedActor(SourceFile *SF, NominalTypeDecl *nominal
       continue;
     }
 
-    // --- Ensure all thunks
+    // --- Ensure 'distributed func' all thunks
     if (auto func = dyn_cast<AbstractFunctionDecl>(member)) {
       if (!func->isDistributed())
         continue;
@@ -704,6 +715,48 @@ void TypeChecker::checkDistributedActor(SourceFile *SF, NominalTypeDecl *nominal
               nominal->getName());
           return;
         }
+
+        // Check there's no async/no-async overloads, since those are more
+        // fragile in distribution than we'd want distributed calls to be.
+        // A remote call is always 'async throws', and we can always record
+        // an async throws "accessor" (see AccessibleFunction.cpp) as such.
+        // This means, if we allowed async/no-async overloads of functions,
+        // we'd have to store the precise "it was not throwing" information,
+        // but we'll _never_ make use of such because all remote calls are
+        // necessarily going to async to the actor in the recipient process,
+        // and for the remote caller, they are always as-if-async.
+        //
+        // By banning such overloads, which may be useful in local APIs,
+        // but too fragile in distributed APIs, we allow a remote 'v2' version
+        // of an implementation to add or remove `async` to their implementation
+        // without breaking calls which were made on previous 'v1' versions of
+        // the same interface; Callers are never broken this way, and rollouts
+        // are simpler.
+        //
+        // The restriction on overloads is not a problem for distributed calls,
+        // as we don't have a vast swab of APIs which must compatibly get async
+        // versions, as that is what the async overloading aimed to address.
+        //
+        // Note also, that overloading on throws is already illegal anyway.
+        if (!diagnosedAmbiguity.contains(func->getName())) {
+          auto candidates = nominal->lookupDirect(func->getName());
+          if (candidates.size() > 1) {
+            auto firstDecl = dyn_cast<AbstractFunctionDecl>(candidates.back());
+            for (auto decl : candidates) {
+              if (decl == firstDecl) {
+                decl->diagnose(
+                    diag::distributed_func_cannot_overload_on_async_only,
+                    decl->getName());
+              } else {
+                decl->diagnose(
+                    diag::distributed_func_other_ambiguous_overload_here,
+                    decl->getName());
+              }
+            }
+
+            diagnosedAmbiguity.insert(func->getName());
+          }
+        }
       }
 
       if (auto thunk = func->getDistributedThunk()) {

test/Distributed/Runtime/distributed_actor_func_calls_remoteCall_remoteChanged_asyncness.swift

@@ -0,0 +1,100 @@
+// RUN: %empty-directory(%t)
+// RUN: %target-swift-frontend-emit-module -emit-module-path %t/FakeDistributedActorSystems.swiftmodule -module-name FakeDistributedActorSystems -disable-availability-checking %S/../Inputs/FakeDistributedActorSystems.swift
+// RUN: %target-build-swift -module-name main  -Xfrontend -disable-availability-checking -j2 -parse-as-library -I %t %s %S/../Inputs/FakeDistributedActorSystems.swift -o %t/a.out
+// RUN: %target-run %t/a.out | %FileCheck %s --color --dump-input=always
+
+// REQUIRES: executable_test
+// REQUIRES: concurrency
+// REQUIRES: distributed
+
+// rdar://76038845
+// UNSUPPORTED: use_os_stdlib
+// UNSUPPORTED: back_deployment_runtime
+
+// FIXME(distributed): Distributed actors currently have some issues on windows, isRemote always returns false. rdar://82593574
+// UNSUPPORTED: OS=windows-msvc
+
+import Distributed
+import FakeDistributedActorSystems
+
+typealias DefaultDistributedActorSystem = FakeRoundtripActorSystem
+
+distributed actor Greeter {
+  distributed func usedToHaveAsyncBytNotAnymore()
+    /*previously had async, and remote called invoked such async method */ -> String {
+    return #function
+  }
+}
+
+distributed actor Helloer {
+  distributed func usedToBeSyncButNowIsAsync()
+    /* The remote caller thinks this method was not async, but actually it is */
+    async -> String {
+    return #function
+  }
+}
+
+func test_usedToBeAsync_but_remoteImplIsNotAnymore() async throws {
+  let system = DefaultDistributedActorSystem()
+
+  let local = Greeter(actorSystem: system)
+  let remote = try Greeter.resolve(id: local.id, using: system)
+
+  var invocation = FakeInvocationEncoder()
+  // let reply = try await remote.usedToHaveAsyncBytNotAnymore()
+  let reply: String = try await system.remoteCall(
+    on: remote,
+    // Important: notice the mangling has a YaK here, since we mangled
+    // based on the distributed thunk, which is 'async throws' (YaK).
+    //
+    // This test is about ensuring, that even if a remote recipient process,
+    // changed their implementation from async to not async, we're still able
+    // to invoke them. From the perspective of the remote caller it truly does
+    // not matter how the recipient implements this call: is it async or not,
+    // and we should not tie ability to invoke a method to it's asyncness.
+    //
+    // Note also that a remote call is always async and throws as well,
+    // so mangling based on the 'async throws' thunk does not introduce
+    // unexpected effects in remote calls.
+    //
+    // Design limitation by choice: this means we cannot
+    target: RemoteCallTarget("$s4main7GreeterC28usedToHaveAsyncBytNotAnymoreSSyYaKFTE"),
+    invocation: &invocation,
+    throwing: Never.self,
+    returning: String.self
+  )
+
+  // CHECK: >> remoteCall: on:main.Greeter, target:main.Greeter.usedToHaveAsyncBytNotAnymore(), invocation:FakeInvocationEncoder(genericSubs: [], arguments: [], returnType: nil, errorType: nil), throwing:Swift.Never, returning:Swift.String
+  // CHECK: << onReturn: usedToHaveAsyncBytNotAnymore()
+
+  print("reply: \(reply)") // CHECK: reply: usedToHaveAsyncBytNotAnymore()
+}
+
+func test_usedToBeSync_but_remoteImplIsAsyncNow() async throws {
+  let system = DefaultDistributedActorSystem()
+
+  let local = Helloer(actorSystem: system)
+  let remote = try Helloer.resolve(id: local.id, using: system)
+
+  var invocation = FakeInvocationEncoder()
+  //  let reply: String = try await remote.usedToBeSyncButNowIsAsync()
+  let reply: String = try await system.remoteCall(
+    on: remote,
+    target: RemoteCallTarget("$s4main7HelloerC25usedToBeSyncButNowIsAsyncSSyYaKFTE"),
+    invocation: &invocation,
+    throwing: Never.self,
+    returning: String.self
+  )
+
+  // CHECK: >> remoteCall: on:main.Helloer, target:main.Helloer.usedToBeSyncButNowIsAsync(), invocation:FakeInvocationEncoder(genericSubs: [], arguments: [], returnType: nil, errorType: nil), throwing:Swift.Never, returning:Swift.String
+  // CHECK: << onReturn: usedToBeSyncButNowIsAsync()
+
+  print("reply: \(reply)") // CHECK: reply: usedToBeSyncButNowIsAsync()
+}
+
+@main struct Main {
+  static func main() async {
+    try! await test_usedToBeAsync_but_remoteImplIsNotAnymore()
+    try! await test_usedToBeSync_but_remoteImplIsAsyncNow()
+  }
+}

test/Distributed/distributed_func_overloads.swift

@@ -0,0 +1,40 @@
+// RUN: %empty-directory(%t)
+// RUN: %target-swift-frontend-emit-module -emit-module-path %t/FakeDistributedActorSystems.swiftmodule -module-name FakeDistributedActorSystems -disable-availability-checking %S/Inputs/FakeDistributedActorSystems.swift
+// RUN: %target-swift-frontend -typecheck -verify -disable-availability-checking -I %t 2>&1 %s
+// REQUIRES: concurrency
+// REQUIRES: distributed
+
+import Distributed
+import FakeDistributedActorSystems
+
+typealias DefaultDistributedActorSystem = FakeActorSystem
+
+// ==== ------------------------------------------------------------------------
+
+distributed actor Overloader {
+
+  func overloaded() {}
+  func overloaded() async {}
+
+  distributed func overloadedDistA() {} // expected-note{{ambiguous distributed func 'overloadedDistA()' declared here}}
+  distributed func overloadedDistA() async {} // expected-error{{ambiguous distributed func declaration 'overloadedDistA()', cannot overload distributed methods on effect only}}
+
+  distributed func overloadedDistT() throws {} // expected-note{{ambiguous distributed func 'overloadedDistT()' declared here}}
+  distributed func overloadedDistT() async throws {} // expected-error{{ambiguous distributed func declaration 'overloadedDistT()', cannot overload distributed methods on effect only}}
+
+  // Throws overloads are not legal anyway, but let's check for them here too:
+  distributed func overloadedDistThrows() {}
+  // expected-note@-1{{ambiguous distributed func 'overloadedDistThrows()' declared here}}
+  // expected-note@-2{{'overloadedDistThrows()' previously declared here}}
+  distributed func overloadedDistThrows() throws {}
+  // expected-error@-1{{ambiguous distributed func declaration 'overloadedDistThrows()', cannot overload distributed methods on effect only}}
+  // expected-error@-2{{invalid redeclaration of 'overloadedDistThrows()'}}
+
+  distributed func overloadedDistAsync() async {}
+  // expected-note@-1{{ambiguous distributed func 'overloadedDistAsync()' declared here}}
+  // expected-note@-2{{'overloadedDistAsync()' previously declared here}}
+  distributed func overloadedDistAsync() async throws {}
+  // expected-error@-1{{ambiguous distributed func declaration 'overloadedDistAsync()', cannot overload distributed methods on effect only}}
+  // expected-error@-2{{invalid redeclaration of 'overloadedDistAsync()'}}
+}
+

validation-test/IRGen/98995607.swift.gyb

@@ -0,0 +1,21 @@
+// RUN: %target-run-simple-swiftgyb
+
+%for N in range(0, 100):
+
+protocol P${N}<A, B> {
+  associatedtype A
+  associatedtype B
+}
+
+%end
+
+var array : [Any.Type] = [
+%for N in range(0, 100):
+  (any P${N}<Int, Float>).self,
+%end
+%for N in range(0, 100):
+  (any P${N}<Float, String>).self,
+%end
+]
+
+print(array)

validation-test/Sema/type_checker_crashers_fixed/issue60649.swift

@@ -0,0 +1,26 @@
+// RUN: %target-typecheck-verify-swift
+
+protocol P {}
+
+protocol Key {
+  associatedtype A: P
+  // expected-note@-1 {{unable to infer associated type 'A' for protocol 'Key'}}
+  static var value: A { get }
+}
+
+struct Values {
+  subscript<K: Key>(type: K.Type) -> K.A {
+    fatalError()
+  }
+}
+
+enum MyKey: Key { // expected-error {{type 'MyKey' does not conform to protocol 'Key'}}
+  static let value = 1
+  // expected-note@-1 {{candidate would match and infer 'A' = 'Int' if 'Int' conformed to 'P'}}
+}
+
+extension Values {
+  var myValue: Int {
+    get { self[MyKey.self] }
+  }
+}