Merge pull request swiftlang#81373 from eeckstein/fix-embedded-error

EmbeddedSwiftDiagnostics: fix and improve some embedded diagnostic errors

Author: eeckstein

SwiftCompilerSources/Sources/AST/Type.swift

@@ -180,6 +180,7 @@ extension TypeProperties {
   public var hasArchetype: Bool { rawType.bridged.hasArchetype() }
   public var hasTypeParameter: Bool { rawType.bridged.hasTypeParameter() }
   public var hasLocalArchetype: Bool { rawType.bridged.hasLocalArchetype() }
+  public var hasDynamicSelf: Bool { rawType.bridged.hasDynamicSelf() }
   public var isEscapable: Bool { rawType.bridged.isEscapable() }
   public var isNoEscape: Bool { rawType.bridged.isNoEscape() }
   public var isBuiltinType: Bool { rawType.bridged.isBuiltinType() }

SwiftCompilerSources/Sources/Optimizer/ModulePasses/EmbeddedSwiftDiagnostics.swift

@@ -130,15 +130,27 @@ private struct FunctionChecker {
         throw Diagnostic(.embedded_swift_allocating_type, (instruction as! SingleValueInstruction).type,
                               at: instruction.location)
       }
-    case is BeginApplyInst:
-      throw Diagnostic(.embedded_swift_allocating_coroutine, at: instruction.location)
+
     case is ThunkInst:
-      throw Diagnostic(.embedded_swift_allocating, at: instruction.location)
+      if context.options.noAllocations {
+        throw Diagnostic(.embedded_swift_allocating, at: instruction.location)
+      }
+
+    case let ba as BeginApplyInst:
+      if context.options.noAllocations {
+        throw Diagnostic(.embedded_swift_allocating_coroutine, at: instruction.location)
+      }
+      try checkApply(apply: ba)
 
     case let pai as PartialApplyInst:
       if context.options.noAllocations && !pai.isOnStack {
         throw Diagnostic(.embedded_swift_allocating_closure, at: instruction.location)
       }
+      try checkApply(apply: pai)
+
+    // Remaining apply instructions
+    case let apply as ApplySite:
+      try checkApply(apply: apply)
 
     case let bi as BuiltinInst:
       switch bi.id {
@@ -156,39 +168,45 @@ private struct FunctionChecker {
         break
       }
 
-    case let apply as ApplySite:
-      if context.options.noAllocations && apply.isAsync {
-        throw Diagnostic(.embedded_swift_allocating_type, at: instruction.location)
-      }
+    default:
+      break
+    }
+  }
 
-      if !apply.callee.type.hasValidSignatureForEmbedded,
-         // Some runtime functions have generic parameters in SIL, which are not used in IRGen.
-         // Therefore exclude runtime functions at all.
-         !apply.callsEmbeddedRuntimeFunction
-      {
-        switch apply.callee {
-        case let cmi as ClassMethodInst:
-          throw Diagnostic(.embedded_cannot_specialize_class_method, cmi.member, at: instruction.location)
-        case let wmi as WitnessMethodInst:
-          throw Diagnostic(.embedded_cannot_specialize_witness_method, wmi.member, at: instruction.location)
-        default:
-          throw Diagnostic(.embedded_call_generic_function, at: instruction.location)
-        }
-      }
+  mutating func checkApply(apply: ApplySite) throws {
+    if context.options.noAllocations && apply.isAsync {
+      throw Diagnostic(.embedded_swift_allocating_type, at: apply.location)
+    }
 
-      // Although all (non-generic) functions are initially put into the worklist there are two reasons
-      // to call `checkFunction` recursively:
-      // * To get a better caller info in the diagnostics.
-      // * When passing an opened existential to a generic function, it's valid in Embedded swift even if the
-      //   generic is not specialized. We need to check such generic functions, too.
-      if let callee = apply.referencedFunction {
-        callStack.push(CallSite(apply: apply, callee: callee))
-        try checkFunction(callee)
-        _ = callStack.pop()
+    if !apply.callee.type.hasValidSignatureForEmbedded,
+       // Some runtime functions have generic parameters in SIL, which are not used in IRGen.
+       // Therefore exclude runtime functions at all.
+       !apply.callsEmbeddedRuntimeFunction
+    {
+      switch apply.callee {
+      case let cmi as ClassMethodInst:
+        throw Diagnostic(.embedded_cannot_specialize_class_method, cmi.member, at: apply.location)
+      case let wmi as WitnessMethodInst:
+        throw Diagnostic(.embedded_cannot_specialize_witness_method, wmi.member, at: apply.location)
+      default:
+        if apply.substitutionMap.replacementTypes.contains(where: { $0.hasDynamicSelf }),
+           apply.calleeHasGenericSelfMetatypeParameter
+        {
+          throw Diagnostic(.embedded_call_generic_function_with_dynamic_self, at: apply.location)
+        }
+        throw Diagnostic(.embedded_call_generic_function, at: apply.location)
       }
+    }
 
-    default:
-      break
+    // Although all (non-generic) functions are initially put into the worklist there are two reasons
+    // to call `checkFunction` recursively:
+    // * To get a better caller info in the diagnostics.
+    // * When passing an opened existential to a generic function, it's valid in Embedded swift even if the
+    //   generic is not specialized. We need to check such generic functions, too.
+    if let callee = apply.referencedFunction {
+      callStack.push(CallSite(apply: apply, callee: callee))
+      try checkFunction(callee)
+      _ = callStack.pop()
     }
   }
 
@@ -355,6 +373,15 @@ private extension ApplySite {
     }
     return false
   }
+
+  var calleeHasGenericSelfMetatypeParameter: Bool {
+    let convention = FunctionConvention(for: callee.type.canonicalType, in: parentFunction)
+    guard convention.hasSelfParameter, let selfParam = convention.parameters.last else {
+      return false
+    }
+    let selfParamType = selfParam.type
+    return selfParamType.isMetatype && selfParamType.instanceTypeOfMetatype.isGenericTypeParameter
+  }
 }
 
 private extension Type {

SwiftCompilerSources/Sources/Optimizer/ModulePasses/MandatoryPerformanceOptimizations.swift

@@ -258,11 +258,13 @@ private func shouldInline(apply: FullApplySite, callee: Function, alreadyInlined
     return false
   }
 
-  if !callee.canBeInlinedIntoCaller(withSerializedKind: apply.parentFunction.serializedKind) &&
-     // Even if the serialization kind doesn't match, we need to make sure to inline witness method thunks
-     // in embedded swift.
-     callee.thunkKind != .thunk
-  {
+  guard callee.canBeInlinedIntoCaller(withSerializedKind: apply.parentFunction.serializedKind) ||
+        // Even if the serialization kind doesn't match, we need to make sure to inline witness method thunks
+        // in embedded swift.
+        callee.thunkKind == .thunk ||
+        // Force inlining transparent co-routines. This might be necessary if `-enable-testing` is turned on.
+        (apply is BeginApplyInst && callee.isTransparent)
+  else {
     return false
   }
 

include/swift/AST/ASTBridging.h

@@ -3077,6 +3077,7 @@ struct BridgedASTType {
   BRIDGED_INLINE bool isGenericAtAnyLevel() const;
   BRIDGED_INLINE bool hasTypeParameter() const;
   BRIDGED_INLINE bool hasLocalArchetype() const;
+  BRIDGED_INLINE bool hasDynamicSelf() const;
   BRIDGED_INLINE bool isArchetype() const;
   BRIDGED_INLINE bool archetypeRequiresClass() const;
   BRIDGED_INLINE bool isExistentialArchetype() const;

include/swift/AST/ASTBridgingImpl.h

@@ -436,6 +436,10 @@ bool BridgedASTType::hasLocalArchetype() const {
   return unbridged()->hasLocalArchetype();
 }
 
+bool BridgedASTType::hasDynamicSelf() const {
+  return unbridged()->hasDynamicSelfType();
+}
+
 bool BridgedASTType::isArchetype() const {
   return unbridged()->is<swift::ArchetypeType>();
 }

include/swift/AST/DiagnosticsSIL.def

@@ -415,7 +415,9 @@ ERROR(embedded_swift_allocating,none,
 ERROR(embedded_capture_of_generic_value_with_deinit,none,
       "capturing generic non-copyable type with deinit in escaping closure not supported in embedded Swift", ())
 ERROR(embedded_call_generic_function,none,
-      "cannot specialize generic function in this context", ())
+      "cannot specialize generic function or default protocol method in this context", ())
+ERROR(embedded_call_generic_function_with_dynamic_self,none,
+      "cannot call an initializer or static method, which is defined as default protocol method, from a class method or initializer", ())
 NOTE(embedded_specialization_called_from,none,
       "generic specialization called here", ())
 NOTE(embedded_existential_created,none,

test/embedded/coroutines-and-testing.swift

@@ -0,0 +1,19 @@
+// Make sure this can be compiled without any errors
+
+// RUN: %target-swift-frontend %s -emit-ir -o /dev/null -enable-experimental-feature Embedded -enable-testing
+// RUN: %target-swift-frontend %s -emit-ir -o /dev/null -enable-experimental-feature Embedded -enable-testing -no-allocations
+
+
+// REQUIRES: VENDOR=apple
+// REQUIRES: OS=macosx
+// REQUIRES: swift_feature_Embedded
+
+public protocol P {
+  var storage: UInt32 { get set }
+}
+
+struct MyStruct: P {
+  var storage: UInt32
+}
+
+

test/embedded/generic-error.swift

@@ -0,0 +1,25 @@
+// RUN: %target-swift-emit-ir -parse-as-library -module-name main -verify %s -enable-experimental-feature Embedded -swift-version 5 -wmo -o /dev/null
+
+// REQUIRES: swift_in_compiler
+// REQUIRES: optimized_stdlib
+// REQUIRES: swift_feature_Embedded
+
+// We don't support calling a default witness method (which is generic) with dynamic self.
+
+public protocol P {
+  init(x: Int)
+}
+
+extension P {
+  init(y: Int) {
+    self.init(x: y)
+  }
+}
+
+public class C: P {
+  public required init(x: Int) {}
+
+  public convenience init(z: Int) {
+    self.init(y: z)  // expected-error {{cannot call an initializer or static method, which is defined as default protocol method, from a class method or initializer}}
+  }
+}