Merge pull request #12174 from slavapestov/there-can-only-be-one-self

Fix issue with 'Self' metadata when class conforms to protocol with default implementations

include/swift/AST/DiagnosticsSema.def

@@ -1452,6 +1452,11 @@ ERROR(witness_requires_dynamic_self,none,
       "method %0 in non-final class %1 must return `Self` to conform to "
       "protocol %2",
       (DeclName, Type, Type))
+ERROR(witness_requires_class_implementation,none,
+      "method %0 in non-final class %1 cannot be implemented in a "
+      "protocol extension because it returns `Self` and has associated type "
+      "requirements",
+      (DeclName, Type))
 ERROR(witness_not_accessible_proto,none,
       "%select{initializer %1|method %1|%select{|setter for }2property %1"
       "|subscript%select{| setter}2}0 must be declared "

lib/SILOptimizer/Utils/Devirtualize.cpp

@@ -712,7 +712,8 @@ DevirtualizationResult swift::tryDevirtualizeClassMethod(FullApplySite AI,
 //===----------------------------------------------------------------------===//
 
 static SubstitutionMap
-getSubstitutionsForProtocolConformance(ProtocolConformanceRef CRef) {
+getSubstitutionsForProtocolConformance(ModuleDecl *M,
+                                       ProtocolConformanceRef CRef) {
   auto C = CRef.getConcrete();
 
   // Walk down to the base NormalProtocolConformance.
@@ -746,8 +747,7 @@ getSubstitutionsForProtocolConformance(ProtocolConformanceRef CRef) {
 
   if (Subs.empty()) {
     auto *DC = NormalC->getDeclContext();
-    return NormalC->getType()
+    return NormalC->getType()->getContextSubstitutionMap(M, DC);
-      ->getContextSubstitutionMap(DC->getParentModule(), DC);
   }
 
   return NormalC->getGenericSignature()->getSubstitutionMap(Subs);
@@ -764,17 +764,50 @@ getSubstitutionsForProtocolConformance(ProtocolConformanceRef CRef) {
 /// are written in terms of the requirement's generic signature need
 /// to be remapped to substitutions suitable for the witness signature.
 ///
+/// Supported remappings are:
+///
+/// - (Concrete witness thunk) Original substitutions:
+///   [Self := ConcreteType, R0 := X0, R1 := X1, ...]
+/// - Requirement generic signature:
+///   <Self : P, R0, R1, ...>
+/// - Witness thunk generic signature:
+///   <W0, W1, ...>
+/// - Remapped substitutions:
+///   [W0 := X0, W1 := X1, ...]
+///
+/// - (Class witness thunk) Original substitutions:
+///   [Self := C<A0, A1>, T0 := X0, T1 := X1, ...]
+/// - Requirement generic signature:
+///   <Self : P, R0, R1, ...>
+/// - Witness thunk generic signature:
+///   <Self : C<B0, B1>, B0, B1, W0, W1, ...>
+/// - Remapped substitutions:
+///   [Self := C<B0, B1>, B0 := A0, B1 := A1, W0 := X0, W1 := X1]
+///
+/// - (Default witness thunk) Original substitutions:
+///   [Self := ConcreteType, R0 := X0, R1 := X1, ...]
+/// - Requirement generic signature:
+///   <Self : P, R0, R1, ...>
+/// - Witness thunk generic signature:
+///   <Self : P, W0, W1, ...>
+/// - Remapped substitutions:
+///   [Self := ConcreteType, W0 := X0, W1 := X1, ...]
+///
 /// \param conformanceRef The (possibly-specialized) conformance
 /// \param requirementSig The generic signature of the requirement
 /// \param witnessThunkSig The generic signature of the witness method
 /// \param origSubs The substitutions from the call instruction
+/// \param isDefaultWitness True if this is a default witness method
+/// \param classWitness The ClassDecl if this is a class witness method
 static SubstitutionMap
 getWitnessMethodSubstitutions(
+    ModuleDecl *mod,
     ProtocolConformanceRef conformanceRef,
     GenericSignature *requirementSig,
     GenericSignature *witnessThunkSig,
     SubstitutionList origSubs,
-    bool isDefaultWitness) {
+    bool isDefaultWitness,
+    ClassDecl *classWitness) {
 
   if (witnessThunkSig == nullptr)
     return SubstitutionMap();
@@ -789,7 +822,7 @@ getWitnessMethodSubstitutions(
 
   // If `Self` maps to a bound generic type, this gives us the
   // substitutions for the concrete type's generic parameters.
-  auto baseSubMap = getSubstitutionsForProtocolConformance(conformanceRef);
+  auto baseSubMap = getSubstitutionsForProtocolConformance(mod, conformanceRef);
 
   unsigned baseDepth = 0;
   auto *rootConformance = conformance->getRootNormalConformance();
@@ -798,6 +831,39 @@ getWitnessMethodSubstitutions(
 
   auto origDepth = 1;
 
+  // If the witness has a class-constrained 'Self' generic parameter,
+  // we have to build a new substitution map that shifts all generic
+  // parameters down by one.
+  if (classWitness != nullptr) {
+    baseDepth += 1;
+
+    auto &ctx = mod->getASTContext();
+    auto *proto = conformance->getProtocol();
+    auto selfType = proto->getSelfInterfaceType();
+
+    auto witnessThunkToWitnessMap = witnessThunkSig->getSubstitutionMap(
+        [&](SubstitutableType *type) -> Type {
+          if (type->isEqual(selfType))
+            return classWitness->getSelfInterfaceType();
+
+          auto *origParamTy = cast<GenericTypeParamType>(type);
+          auto *substParamTy = GenericTypeParamType::get(
+              origParamTy->getDepth() - 1,
+              origParamTy->getIndex(),
+              ctx);
+
+          return substParamTy;
+        },
+        [&](CanType origType, Type replacementType, ProtocolType *protoType)
+            -> Optional<ProtocolConformanceRef> {
+          assert(!origType->isEqual(selfType));
+
+          return ProtocolConformanceRef(protoType->getDecl());
+        });
+
+    baseSubMap = witnessThunkToWitnessMap.subst(baseSubMap);
+  }
+
   return SubstitutionMap::combineSubstitutionMaps(
       baseSubMap,
       origSubMap,
@@ -807,24 +873,40 @@ getWitnessMethodSubstitutions(
       witnessThunkSig);
 }
 
+static ClassDecl *
+getWitnessMethodClass(SILFunctionType *witnessFnTy, ModuleDecl &M) {
+  auto selfTy = witnessFnTy->getSelfInstanceType();
+  auto genericSig = witnessFnTy->getGenericSignature();
+  if (auto paramTy = dyn_cast<GenericTypeParamType>(selfTy)) {
+    auto superclass = genericSig->getSuperclassBound(paramTy, M);
+    if (superclass)
+      return superclass->getClassOrBoundGenericClass();
+  }
+
+  return nullptr;
+}
+
 static SubstitutionMap
 getWitnessMethodSubstitutions(SILModule &Module, ApplySite AI, SILFunction *F,
                               ProtocolConformanceRef CRef) {
+  auto witnessFnTy = F->getLoweredFunctionType();
+  assert(witnessFnTy->getRepresentation() ==
+         SILFunctionTypeRepresentation::WitnessMethod);
+
   auto requirementSig = AI.getOrigCalleeType()->getGenericSignature();
-  auto witnessThunkSig = F->getLoweredFunctionType()->getGenericSignature();
+  auto witnessThunkSig = witnessFnTy->getGenericSignature();
 
   SubstitutionList origSubs = AI.getSubstitutions();
 
+  auto *mod = Module.getSwiftModule();
   bool isDefaultWitness =
-    F->getLoweredFunctionType()->getRepresentation()
+    (witnessFnTy->getDefaultWitnessMethodProtocol(*mod)
-      == SILFunctionTypeRepresentation::WitnessMethod &&
+      == CRef.getRequirement());
-    F->getLoweredFunctionType()->getDefaultWitnessMethodProtocol(
+  auto *classWitness = getWitnessMethodClass(witnessFnTy, *mod);
-                                                     *Module.getSwiftModule())
-      == CRef.getRequirement();
 
   return getWitnessMethodSubstitutions(
-      CRef, requirementSig, witnessThunkSig,
+      mod, CRef, requirementSig, witnessThunkSig,
-      origSubs, isDefaultWitness);
+      origSubs, isDefaultWitness, classWitness);
 }
 
 /// Generate a new apply of a function_ref to replace an apply of a

test/Interpreter/protocol_extensions.swift

@@ -263,5 +263,55 @@ _ = sup.init()
 sup = Sub.self
 _ = sup.init()
 
+// https://bugs.swift.org/browse/SR-617
+protocol SelfMetadataTest {
+  associatedtype T = Int
+
+  func staticTypeOfSelf() -> Any
+  func staticTypeOfSelfTakesT(_: T) -> Any
+}
+
+extension SelfMetadataTest {
+  func staticTypeOfSelf() -> Any {
+    return Self.self
+  }
+
+  func staticTypeOfSelfTakesT(_: T) -> Any {
+    return Self.self
+  }
+
+  func staticTypeOfSelfNotAWitness() -> Any {
+    return Self.self
+  }
+}
+
+class SelfMetadataBase : SelfMetadataTest {}
+
+class SelfMetadataDerived : SelfMetadataBase {}
+
+func testSelfMetadata<T : SelfMetadataTest>(_ x: T, _ t: T.T) {
+  print(x.staticTypeOfSelf())
+  print(x.staticTypeOfSelfTakesT(t))
+  print(x.staticTypeOfSelfNotAWitness())
+}
+
+// CHECK: SelfMetadataBase
+// CHECK: SelfMetadataBase
+// CHECK: SelfMetadataBase
+testSelfMetadata(SelfMetadataBase(), 0)
+
+// CHECK: SelfMetadataBase
+// CHECK: SelfMetadataBase
+// CHECK: SelfMetadataBase
+testSelfMetadata(SelfMetadataDerived() as SelfMetadataBase, 0)
+
+// This is the interesting case -- make sure the static type of 'Self'
+// is correctly passed on from the call site to the extension method
+
+// CHECK: SelfMetadataDerived
+// CHECK: SelfMetadataBase
+// CHECK: SelfMetadataDerived
+testSelfMetadata(SelfMetadataDerived(), 0)
+
 // CHECK: DONE
 print("DONE")

test/SILGen/protocol_extensions.swift

@@ -70,7 +70,7 @@ class C : P1 {
 
 //   (materializeForSet test from above)
 // CHECK-LABEL: sil private [transparent] [thunk] @_T019protocol_extensions1CCAA2P1A2aDPS2icimTW
-// CHECK: bb0(%0 : $Builtin.RawPointer, %1 : $*Builtin.UnsafeValueBuffer, %2 : $Int, %3 : $*C):
+// CHECK: bb0(%0 : $Builtin.RawPointer, %1 : $*Builtin.UnsafeValueBuffer, %2 : $Int, %3 : $*τ_0_0):
 // CHECK: function_ref @_T019protocol_extensions2P1PAAES2icig
 // CHECK: return
 

test/SILGen/witnesses_class.swift

@@ -1,4 +1,4 @@
-// RUN: %target-swift-frontend -emit-silgen %s | %FileCheck %s
+// RUN: %target-swift-frontend -enable-sil-ownership -emit-silgen %s | %FileCheck %s
 
 protocol Fooable: class {
   func foo()
@@ -24,8 +24,8 @@ class Foo: Fooable {
   // CHECK:         class_method
 }
 
-// CHECK-LABEL: sil hidden @_T015witnesses_class3gen{{[_0-9a-zA-Z]*}}F
+// CHECK-LABEL: sil hidden @_T015witnesses_class3genyxAA7FooableRzlF
-// CHECK:         bb0([[SELF:%.*]] : $T)
+// CHECK:         bb0([[SELF:%.*]] : @owned $T)
 // CHECK:         [[METHOD:%.*]] = witness_method $T
 // CHECK-NOT:     copy_value [[SELF]]
 // CHECK:         [[BORROWED_SELF:%.*]] = begin_borrow [[SELF]]
@@ -40,7 +40,7 @@ func gen<T: Fooable>(_ foo: T) {
 }
 
 // CHECK-LABEL: sil hidden @_T015witnesses_class2exyAA7Fooable_pF
-// CHECK: bb0([[SELF:%[0-0]+]] : $Fooable):
+// CHECK: bb0([[SELF:%[0-0]+]] : @owned $Fooable):
 // CHECK:         [[BORROWED_SELF:%.*]] = begin_borrow [[SELF]]
 // CHECK:         [[SELF_PROJ:%.*]] = open_existential_ref [[BORROWED_SELF]]
 // CHECK:         [[METHOD:%.*]] = witness_method $[[OPENED:@opened(.*) Fooable]],
@@ -53,3 +53,58 @@ func gen<T: Fooable>(_ foo: T) {
 func ex(_ foo: Fooable) {
   foo.foo()
 }
+
+// Default implementations in a protocol extension
+protocol HasDefaults {
+  associatedtype T = Self
+
+  func hasDefault()
+
+  func hasDefaultTakesT(_: T)
+
+  func hasDefaultGeneric<U : Fooable>(_: U)
+
+  func hasDefaultGenericTakesT<U : Fooable>(_: T, _: U)
+}
+
+extension HasDefaults {
+  func hasDefault() {}
+
+  func hasDefaultTakesT(_: T) {}
+
+  func hasDefaultGeneric<U : Fooable>(_: U) {}
+
+  func hasDefaultGenericTakesT<U : Fooable>(_: T, _: U) {}
+}
+
+protocol Barable {}
+
+class UsesDefaults<X : Barable> : HasDefaults {}
+
+// Covariant Self:
+
+// CHECK-LABEL: sil private [transparent] [thunk] @_T015witnesses_class12UsesDefaultsCyxGAA03HasD0A2A7BarableRzlAaEP10hasDefaultyyFTW : $@convention(witness_method) <τ_0_0><τ_1_0 where τ_0_0 : UsesDefaults<τ_1_0>, τ_1_0 : Barable> (@in_guaranteed τ_0_0) -> ()
+// CHECK: [[FN:%.*]] = function_ref @_T015witnesses_class11HasDefaultsPAAE10hasDefaultyyF : $@convention(method) <τ_0_0 where τ_0_0 : HasDefaults> (@in_guaranteed τ_0_0) -> ()
+// CHECK: apply [[FN]]<τ_0_0>(
+// CHECK: return
+
+// Invariant Self, since type signature contains an associated type:
+
+// CHECK-LABEL: sil private [transparent] [thunk] @_T015witnesses_class12UsesDefaultsCyxGAA03HasD0A2A7BarableRzlAaEP16hasDefaultTakesTy1TQzFTW : $@convention(witness_method) <τ_0_0 where τ_0_0 : Barable> (@in UsesDefaults<τ_0_0>, @in_guaranteed UsesDefaults<τ_0_0>) -> ()
+// CHECK: [[FN:%.*]] = function_ref @_T015witnesses_class11HasDefaultsPAAE16hasDefaultTakesTy1TQzF : $@convention(method) <τ_0_0 where τ_0_0 : HasDefaults> (@in τ_0_0.T, @in_guaranteed τ_0_0) -> ()
+// CHECK: apply [[FN]]<UsesDefaults<τ_0_0>>(
+// CHECK: return
+
+// Covariant Self:
+
+// CHECK-LABEL: sil private [transparent] [thunk] @_T015witnesses_class12UsesDefaultsCyxGAA03HasD0A2A7BarableRzlAaEP17hasDefaultGenericyqd__AA7FooableRd__lFTW : $@convention(witness_method) <τ_0_0><τ_1_0 where τ_0_0 : UsesDefaults<τ_1_0>, τ_1_0 : Barable><τ_2_0 where τ_2_0 : Fooable> (@owned τ_2_0, @in_guaranteed τ_0_0) -> ()
+// CHECK: [[FN:%.*]] = function_ref @_T015witnesses_class11HasDefaultsPAAE17hasDefaultGenericyqd__AA7FooableRd__lF : $@convention(method) <τ_0_0 where τ_0_0 : HasDefaults><τ_1_0 where τ_1_0 : Fooable> (@owned τ_1_0, @in_guaranteed τ_0_0) -> ()
+// CHECK: apply [[FN]]<τ_0_0, τ_2_0>(
+// CHECK: return
+
+// Invariant Self, since type signature contains an associated type:
+
+// CHECK-LABEL: sil private [transparent] [thunk] @_T015witnesses_class12UsesDefaultsCyxGAA03HasD0A2A7BarableRzlAaEP23hasDefaultGenericTakesTy1TQz_qd__tAA7FooableRd__lFTW : $@convention(witness_method) <τ_0_0 where τ_0_0 : Barable><τ_1_0 where τ_1_0 : Fooable> (@in UsesDefaults<τ_0_0>, @owned τ_1_0, @in_guaranteed UsesDefaults<τ_0_0>) -> ()
+// CHECK: [[FN:%.*]] = function_ref @_T015witnesses_class11HasDefaultsPAAE23hasDefaultGenericTakesTy1TQz_qd__tAA7FooableRd__lF : $@convention(method) <τ_0_0 where τ_0_0 : HasDefaults><τ_1_0 where τ_1_0 : Fooable> (@in τ_0_0.T, @owned τ_1_0, @in_guaranteed τ_0_0) -> ()
+// CHECK: apply [[FN]]<UsesDefaults<τ_0_0>, τ_1_0>(
+// CHECK: return

test/SILOptimizer/devirt_class_witness_method.sil

@@ -0,0 +1,42 @@
+// RUN: %target-sil-opt -assume-parsing-unqualified-ownership-sil -enable-sil-verify-all %s -devirtualizer -sil-combine -enable-resilience | tee /tmp/xxx | %FileCheck %s
+sil_stage canonical
+
+import Builtin
+
+protocol P {
+  func f()
+}
+
+extension P {
+  func f()
+}
+
+class C<T, U> : P {}
+
+sil hidden_external [transparent] [thunk] @witness_thunk : $@convention(witness_method) <τ_0_0><τ_1_0, τ_1_1 where τ_0_0 : C<τ_1_0, τ_1_1>> (@in_guaranteed τ_0_0) -> ()
+
+// CHECK-LABEL: sil hidden @caller : $@convention(thin) <T, U> (@owned C<T, U>) -> ()
+// CHECK: [[FN:%.*]] = function_ref @witness_thunk
+// CHECK: apply [[FN]]<C<T, U>, T, U>(
+// CHECK: return
+sil hidden @caller : $@convention(thin) <T, U> (@owned C<T, U>) -> () {
+bb0(%0 : $C<T, U>):
+  strong_retain %0 : $C<T, U>
+  %4 = alloc_stack $C<T, U>
+  store %0 to %4 : $*C<T, U>
+  %6 = witness_method $C<T, U>, #P.f!1 : <Self where Self : P> (Self) -> () -> () : $@convention(witness_method) <τ_0_0 where τ_0_0 : P> (@in_guaranteed τ_0_0) -> ()
+  %7 = apply %6<C<T, U>>(%4) : $@convention(witness_method) <τ_0_0 where τ_0_0 : P> (@in_guaranteed τ_0_0) -> ()
+  dealloc_stack %4 : $*C<T, U>
+  strong_release %0 : $C<T, U>
+  %9 = tuple ()
+  return %9 : $()
+}
+
+sil_vtable C {}
+
+sil_witness_table hidden <T, U> C<T, U>: P module clsx {
+  method #P.f!1: <Self where Self : P> (Self) -> () -> () : @witness_thunk
+}
+
+sil_default_witness_table hidden P {
+}

test/decl/protocol/conforms/self.swift

@@ -0,0 +1,37 @@
+// RUN: %target-typecheck-verify-swift
+
+protocol P {
+  associatedtype T = Int
+
+  func hasDefault()
+  func returnsSelf() -> Self
+  func hasDefaultTakesT(_: T)
+  func returnsSelfTakesT(_: T) -> Self
+}
+
+extension P {
+  func hasDefault() {}
+
+  func returnsSelf() -> Self {
+    return self
+  }
+
+  func hasDefaultTakesT(_: T) {}
+
+  func returnsSelfTakesT(_: T) -> Self { // expected-error {{method 'returnsSelfTakesT' in non-final class 'Class' cannot be implemented in a protocol extension because it returns `Self` and has associated type requirements}}
+    return self
+  }
+}
+
+// This fails
+class Class : P {}
+
+// This succeeds, because the class is final
+final class FinalClass : P {}
+
+// This succeeds, because we're not using the default implementation
+class NonFinalClass : P {
+  func returnsSelfTakesT(_: T) -> Self {
+    return self
+  }
+}

validation-test/compiler_crashers_fixed/26725-llvm-smallvectorimpl-swift-diagnosticargument-operator.swift

@@ -6,6 +6,5 @@
 // See https://swift.org/CONTRIBUTORS.txt for the list of Swift project authors
 
 // RUN: not %target-swift-frontend %s -typecheck
-// This test fails in the AST verifier, which can be turned off.
+// REQUIRES: asserts
-// REQUIRES: swift_ast_verifier
 class a<T where g:d{class A{class A<T>:A{init(){T{