Merge pull request #5126 from slavapestov/concrete-constrained-extens…

…ions

Concrete constrained extensions

include/swift/AST/ArchetypeBuilder.h

@@ -305,8 +305,11 @@ class ArchetypeBuilder {
   /// Finalize the set of requirements, performing any remaining checking
   /// required before generating archetypes.
   ///
+  /// \param allowConcreteGenericParams If true, allow generic parameters to
+  /// be made concrete.
+  ///
   /// \returns true if an error occurs, false otherwise.
-  bool finalize(SourceLoc loc);
+  bool finalize(SourceLoc loc, bool allowConcreteGenericParams=false);
 
   /// \brief Resolve the given type to the potential archetype it names.
   ///

lib/AST/ArchetypeBuilder.cpp

@@ -1155,15 +1155,6 @@ bool ArchetypeBuilder::addSameTypeRequirementBetweenArchetypes(
        compareDependentTypes(&T2, &T1) < 0))
     std::swap(T1, T2);
 
-  // Don't allow two generic parameters to be equivalent, because then we
-  // don't actually have two parameters.
-  // FIXME: Should we simply allow this?
-  if (T1Depth == 0 && T2Depth == 0) {
-    Diags.diagnose(Source.getLoc(), diag::requires_generic_params_made_equal,
-                   T1->getName(), T2->getName());
-    return true;
-  }
-
   // Merge any concrete constraints.
   Type concrete1 = T1->ArchetypeOrConcreteType.getAsConcreteType();
   Type concrete2 = T2->ArchetypeOrConcreteType.getAsConcreteType();
@@ -1248,16 +1239,6 @@ bool ArchetypeBuilder::addSameTypeRequirementToConcrete(
     return false;
   }
 
-  // Don't allow a generic parameter to be equivalent to a concrete type,
-  // because then we don't actually have a parameter.
-  // FIXME: Should we simply allow this?
-  if (T->getNestingDepth() == 0) {
-    Diags.diagnose(Source.getLoc(), 
-                   diag::requires_generic_param_made_equal_to_concrete,
-                   T->getName());
-    return true;
-  }
-
   // Make sure the concrete type fulfills the requirements on the archetype.
   DenseMap<ProtocolDecl *, ProtocolConformance*> conformances;
   if (!Concrete->is<ArchetypeType>()) {
@@ -1752,8 +1733,67 @@ static Identifier typoCorrectNestedType(
   return bestMatches.front();
 }
 
-bool ArchetypeBuilder::finalize(SourceLoc loc) {
+bool
+ArchetypeBuilder::finalize(SourceLoc loc, bool allowConcreteGenericParams) {
   bool invalid = false;
+  SmallPtrSet<PotentialArchetype *, 4> visited;
+
+  // Check for generic parameters which have been made concrete or equated
+  // with each other.
+  if (!allowConcreteGenericParams) {
+    unsigned depth = 0;
+    for (const auto &pair : Impl->PotentialArchetypes) {
+      depth = std::max(depth, pair.second->getRootParam()->getDepth());
+    }
+
+    for (const auto &pair : Impl->PotentialArchetypes) {
+      auto pa = pair.second;
+      auto rep = pa->getRepresentative();
+
+      if (pa->getRootParam()->getDepth() < depth)
+        continue;
+
+      if (!visited.insert(rep).second)
+        continue;
+
+      // Don't allow a generic parameter to be equivalent to a concrete type,
+      // because then we don't actually have a parameter.
+      if (rep->ArchetypeOrConcreteType.getAsConcreteType()) {
+        auto &Source = rep->SameTypeSource;
+
+        // For auto-generated locations, we should have diagnosed the problem
+        // elsewhere already.
+        if (!Source->getLoc().isValid())
+          continue;
+
+        Diags.diagnose(Source->getLoc(),
+                       diag::requires_generic_param_made_equal_to_concrete,
+                       rep->getName());
+        invalid = true;
+        continue;
+      }
+
+      // Don't allow two generic parameters to be equivalent, because then we
+      // don't actually have two parameters.
+      for (auto other : rep->getEquivalenceClass()) {
+        if (pa != other && other->getParent() == nullptr) {
+          auto &Source = (other == rep ? pa->SameTypeSource
+                                       : other->SameTypeSource);
+
+          // For auto-generated locations, we should have diagnosed the problem
+          // elsewhere already.
+          if (!Source->getLoc().isValid())
+            continue;
+
+          Diags.diagnose(Source->getLoc(),
+                         diag::requires_generic_params_made_equal,
+                         pa->getName(), other->getName());
+          invalid = true;
+          break;
+        }
+      }
+    }
+  }
 
   // If any nested types remain unresolved, produce diagnostics.
   if (Impl->NumUnresolvedNestedTypes > 0) {

lib/SIL/SILPrinter.cpp

@@ -1817,7 +1817,8 @@ void SILFunction::print(SILPrintContext &PrintCtx) const {
     unsigned disambiguatedNameCounter = 1;
     for (auto *paramTy : sig->getGenericParams()) {
       auto *archetypeTy = mapTypeIntoContext(paramTy)->getAs<ArchetypeType>();
-      assert(archetypeTy);
+      if (!archetypeTy)
+        continue;
 
       Identifier name = archetypeTy->getName();
       while (!UsedNames.insert(name).second) {

lib/SIL/TypeLowering.cpp

@@ -1678,11 +1678,10 @@ TypeConverter::getEffectiveGenericEnvironment(AnyFunctionRef fn,
                                               CaptureInfo captureInfo) {
   auto dc = fn.getAsDeclContext();
 
-  if (dc->getParent()->isLocalContext() &&
-      !captureInfo.hasGenericParamCaptures())
-    return nullptr;
+  if (getEffectiveGenericSignature(fn, captureInfo))
+    return dc->getGenericEnvironmentOfContext();
 
-  return dc->getGenericEnvironmentOfContext();
+  return nullptr;
 }
 
 CanGenericSignature
@@ -1694,8 +1693,11 @@ TypeConverter::getEffectiveGenericSignature(AnyFunctionRef fn,
       !captureInfo.hasGenericParamCaptures())
     return nullptr;
 
-  if (auto sig = dc->getGenericSignatureOfContext())
+  if (auto sig = dc->getGenericSignatureOfContext()) {
+    if (sig->areAllParamsConcrete())
+      return nullptr;
     return sig->getCanonicalSignature();
+  }
 
   return nullptr;
 }
@@ -1990,18 +1992,33 @@ getMaterializeForSetCallbackType(AbstractStorageDecl *storage,
     }
   }
 
+  CanType canSelfType;
+  CanType canSelfMetatypeType;
+  if (genericSig) {
+    canSelfType = genericSig->getCanonicalTypeInContext(
+        selfType, *M.getSwiftModule());
+    canSelfMetatypeType = genericSig->getCanonicalTypeInContext(
+        selfMetatypeType, *M.getSwiftModule());
+  } else {
+    canSelfType = selfType->getCanonicalType();
+    canSelfMetatypeType = selfMetatypeType->getCanonicalType();
+  }
+
   // Create the SILFunctionType for the callback.
   SILParameterInfo params[] = {
     { ctx.TheRawPointerType, ParameterConvention::Direct_Unowned },
     { ctx.TheUnsafeValueBufferType, ParameterConvention::Indirect_Inout },
-    { selfType->getCanonicalType(), ParameterConvention::Indirect_Inout },
-    { selfMetatypeType->getCanonicalType(), ParameterConvention::Direct_Unowned },
+    { canSelfType, ParameterConvention::Indirect_Inout },
+    { canSelfMetatypeType, ParameterConvention::Direct_Unowned },
   };
   ArrayRef<SILResultInfo> results = {};
   auto extInfo = 
     SILFunctionType::ExtInfo()
       .withRepresentation(SILFunctionTypeRepresentation::Thin);
 
+  if (genericSig && genericSig->areAllParamsConcrete())
+    genericSig = nullptr;
+
   return SILFunctionType::get(genericSig, extInfo,
                    /*callee*/ ParameterConvention::Direct_Unowned,
                               params, results, None, ctx);

lib/Sema/ConstraintSystem.cpp

@@ -1044,20 +1044,19 @@ void ConstraintSystem::openGeneric(
        ConstraintLocatorBuilder locator,
        llvm::DenseMap<CanType, TypeVariableType *> &replacements) {
   auto locatorPtr = getConstraintLocator(locator);
-
   auto *genericEnv = innerDC->getGenericEnvironmentOfContext();
 
   // Create the type variables for the generic parameters.
   for (auto gp : params) {
     auto contextTy = genericEnv->mapTypeIntoContext(gp);
-    if (auto *archetype = contextTy->getAs<ArchetypeType>()) {
-      auto typeVar = createTypeVariable(getConstraintLocator(
-                                          locator.withPathElement(
-                                            LocatorPathElt(archetype))),
-                                        TVO_PrefersSubtypeBinding |
-                                        TVO_MustBeMaterializable);
-      replacements[gp->getCanonicalType()] = typeVar;
-    }
+    if (auto *archetype = contextTy->getAs<ArchetypeType>())
+      locatorPtr = getConstraintLocator(
+          locator.withPathElement(LocatorPathElt(archetype)));
+
+    auto typeVar = createTypeVariable(locatorPtr,
+                                      TVO_PrefersSubtypeBinding |
+                                      TVO_MustBeMaterializable);
+    replacements[gp->getCanonicalType()] = typeVar;
   }
 
   GetTypeVariable getTypeVariable{*this, locator};

lib/Sema/TypeCheckDecl.cpp

@@ -730,6 +730,7 @@ TypeChecker::handleSILGenericParams(GenericParamList *genericParams,
     auto genericParams = nestedList.rbegin()[i];
     bool invalid = false;
     auto *genericSig = validateGenericSignature(genericParams, DC, parentSig,
+                                                /*allowConcreteGenericParams=*/true,
                                                 nullptr, invalid);
     if (invalid)
       return std::make_pair(nullptr, nullptr);
@@ -7545,11 +7546,10 @@ static Type checkExtensionGenericParams(
   bool invalid = false;
   auto *parentSig = ext->getDeclContext()->getGenericSignatureOfContext();
   auto *parentEnv = ext->getDeclContext()->getGenericEnvironmentOfContext();
-  GenericSignature *sig = tc.validateGenericSignature(
-      genericParams,
-      ext->getDeclContext(),
-      parentSig,
-      inferExtendedTypeReqs, invalid);
+  auto *sig = tc.validateGenericSignature(genericParams,
+                                          ext->getDeclContext(), parentSig,
+                                          /*allowConcreteGenericParams=*/true,
+                                          inferExtendedTypeReqs, invalid);
   ext->setGenericSignature(sig);
 
   if (invalid) {

lib/Sema/TypeCheckGeneric.cpp

@@ -725,6 +725,7 @@ GenericSignature *TypeChecker::validateGenericSignature(
                     GenericParamList *genericParams,
                     DeclContext *dc,
                     GenericSignature *parentSig,
+                    bool allowConcreteGenericParams,
                     std::function<bool(ArchetypeBuilder &)> inferRequirements,
                     bool &invalid) {
   assert(genericParams && "Missing generic parameters?");
@@ -747,7 +748,8 @@ GenericSignature *TypeChecker::validateGenericSignature(
   }
 
   // Finalize the generic requirements.
-  (void)builder.finalize(genericParams->getSourceRange().Start);
+  (void)builder.finalize(genericParams->getSourceRange().Start,
+                         allowConcreteGenericParams);
 
   // The archetype builder now has all of the requirements, although there might
   // still be errors that have not yet been diagnosed. Revert the signature
@@ -920,6 +922,7 @@ bool TypeChecker::validateGenericTypeSignature(GenericTypeDecl *typeDecl) {
   }
 
   auto *sig = validateGenericSignature(gp, dc, dc->getGenericSignatureOfContext(),
+                                       /*allowConcreteGenericParams=*/false,
                                        nullptr, invalid);
   assert(sig->getInnermostGenericParams().size()
            == typeDecl->getGenericParams()->size());

lib/Sema/TypeChecker.h

@@ -1056,6 +1056,7 @@ class TypeChecker final : public LazyResolver {
                       GenericParamList *genericParams,
                       DeclContext *dc,
                       GenericSignature *outerSignature,
+                      bool allowConcreteGenericParams,
                       std::function<bool(ArchetypeBuilder &)> inferRequirements,
                       bool &invalid);
 

test/Constraints/same_types.swift

@@ -111,7 +111,7 @@ func testAssocTypeEquivalence<T: Fooable>(_ fooable: T) -> X.Type
 }
 
 func fail6<T>(_ t: T) -> Int where T == Int { // expected-error{{same-type requirement makes generic parameter 'T' non-generic}}
-  return t // expected-error{{cannot convert return expression of type 'T' to return type 'Int'}}
+  return t
 }
 
 func test8<T: Barrable, U: Barrable>(_ t: T, u: U) -> (Y, Y, X, X)
@@ -142,3 +142,66 @@ struct BadFooable : Fooable {
 func bogusInOutError(d: inout Brunch<BadFooable>) {}
 // expected-error@-1{{'Brunch' requires the types '<<error type>>' and 'X' be equivalent}}
 
+// Some interesting invalid cases that used to crash
+protocol P {
+  associatedtype A
+  associatedtype B
+}
+
+struct Q : P {
+  typealias A = Int
+  typealias B = Int
+}
+
+struct S1<T : P> {
+  func foo<X, Y>(x: X, y: Y) where X == T.A, Y == T.B {
+    print(X.self)
+    print(Y.self)
+    print(x)
+    print(y)
+  }
+}
+S1<Q>().foo(x: 1, y: 2)
+
+struct S2<T : P> where T.A == T.B {
+  // expected-error@+1 {{same-type requirement makes generic parameters 'X' and 'Y' equivalent}}
+  func foo<X, Y>(x: X, y: Y) where X == T.A, Y == T.B {
+    print(X.self)
+    print(Y.self)
+    print(x)
+    print(y)
+  }
+}
+S2<Q>().foo(x: 1, y: 2)
+
+struct S3<T : P> {
+  // expected-error@+1 {{same-type requirement makes generic parameters 'X' and 'Y' equivalent}}
+  func foo<X, Y>(x: X, y: Y) where X == T.A, Y == T.A {}
+}
+S3<Q>().foo(x: 1, y: 2)
+
+// Secondaries can be equated OK, even if we're imposing
+// new conformances onto an outer secondary
+
+protocol PPP {}
+
+protocol PP {
+  associatedtype A : PPP
+}
+
+struct SSS : PPP {}
+struct SS : PP { typealias A = SSS }
+
+struct QQ : P {
+  typealias A = SSS
+  typealias B = Int
+}
+
+struct S4<T : P> {
+  func foo<X : PP>(x: X) where X.A == T.A {
+    print(x)
+    print(X.self)
+  }
+}
+
+S4<QQ>().foo(x: SS())

test/Generics/function_defs.swift

@@ -294,5 +294,5 @@ func badTypeConformance1<T>(_: T) where Int : EqualComparable {} // expected-err
 
 func badTypeConformance2<T>(_: T) where T.Blarg : EqualComparable { } // expected-error{{'Blarg' is not a member type of 'T'}}
 
-func badSameType<T, U : GeneratesAnElement, V>(_ : T) // expected-error{{generic parameter 'V' is not used in function signature}}
+func badSameType<T, U : GeneratesAnElement, V>(_ : T)
   where T == U.Element, U.Element == V {} // expected-error{{same-type requirement makes generic parameters 'T' and 'V' equivalent}}