Merge pull request #84666 from hamishknight/can-opener

[CS] Consolidate placeholder handling logic into `Solution::simplifyType`

Author: hamishknight

include/swift/Sema/ConstraintSystem.h

@@ -1677,7 +1677,11 @@ class Solution {
   /// \param wantInterfaceType If true, maps the resulting type out of context,
   /// and replaces type variables for opened generic parameters with the
   /// generic parameter types. Should only be used for diagnostic logic.
-  Type simplifyType(Type type, bool wantInterfaceType = false) const;
+  /// \param forCompletion If true, will produce archetypes instead of
+  /// ErrorTypes for generic parameter originators, which is what completion
+  /// currently expects for the code completion token.
+  Type simplifyType(Type type, bool wantInterfaceType = false,
+                    bool forCompletion = false) const;
 
   // To aid code completion, we need to attempt to convert type placeholders
   // back into underlying generic parameters if possible, since type

lib/Sema/CSDiagnostics.cpp

@@ -96,42 +96,12 @@ Type FailureDiagnostic::getRawType(ASTNode node) const {
 
 Type FailureDiagnostic::resolveType(Type rawType, bool reconstituteSugar,
                                     bool wantRValue) const {
-  rawType = rawType.transformRec([&](Type type) -> std::optional<Type> {
-    if (auto *typeVar = type->getAs<TypeVariableType>()) {
-      auto resolvedType = S.simplifyType(typeVar);
-
-      if (!resolvedType->hasError())
-        return resolvedType;
-
-      // If type variable was simplified to an unresolved pack expansion
-      // type, let's examine its original pattern type because it could
-      // contain type variables replaceable with their generic parameter
-      // types.
-      if (auto *expansion = resolvedType->getAs<PackExpansionType>()) {
-        auto *locator = typeVar->getImpl().getLocator();
-        auto *openedExpansionTy =
-            locator->castLastElementTo<LocatorPathElt::PackExpansionType>()
-                .getOpenedType();
-        auto patternType = resolveType(openedExpansionTy->getPatternType());
-        return PackExpansionType::get(patternType, expansion->getCountType());
-      }
-
-      Type GP = typeVar->getImpl().getGenericParameter();
-      return resolvedType->is<ErrorType>() && GP
-          ? ErrorType::get(GP)
-          : resolvedType;
-    }
-
-    if (type->hasElementArchetype()) {
-      auto *env = getDC()->getGenericEnvironmentOfContext();
-      return env->mapElementTypeIntoPackContext(type);
-    }
-
-    if (type->isPlaceholder())
-      return ErrorType::get(type->getASTContext());
+  rawType = S.simplifyType(rawType);
 
-    return std::nullopt;
-  });
+  if (rawType->hasElementArchetype()) {
+    auto *env = getDC()->getGenericEnvironmentOfContext();
+    rawType = env->mapElementTypeIntoPackContext(rawType);
+  }
 
   if (reconstituteSugar)
     rawType = rawType->reconstituteSugar(/*recursive*/ true);

lib/Sema/ConstraintSystem.cpp

@@ -1861,7 +1861,82 @@ void Solution::recordSingleArgMatchingChoice(ConstraintLocator *locator) {
        MatchCallArgumentResult::forArity(1)});
 }
 
-Type Solution::simplifyType(Type type, bool wantInterfaceType) const {
+static GenericTypeParamType *
+getGenericParamForHoleTypeVar(TypeVariableType *tv, const Solution &S) {
+  auto getGenericParam = [](TypeVariableType *tv) -> GenericTypeParamType * {
+    auto *gp = tv->getImpl().getGenericParameter();
+    if (!gp)
+      return nullptr;
+    // Note we only consider generic parameters with underlying decls since for
+    // diagnostics we want something we can print, and for completion we want
+    // something we can extract a GenericEnvironment to produce an archetype.
+    return gp->getDecl() ? gp : nullptr;
+  };
+
+  if (auto *gp = getGenericParam(tv))
+    return gp;
+
+  // Sometimes we run into cases where the originator of a hole isn't for
+  // the generic parameter, instead it's for a member of its equivalence
+  // class. Handle this by looking through all the bindings to see if we have
+  // the same hole bound to a generic parameter type variable.
+  for (auto &binding : S.typeBindings) {
+    if (auto *hole = binding.second->getAs<PlaceholderType>()) {
+      if (hole->getOriginator().dyn_cast<TypeVariableType *>() == tv) {
+        if (auto *gp = getGenericParam(binding.first))
+          return gp;
+      }
+    }
+  }
+  return nullptr;
+}
+
+static Type replacePlaceholderType(PlaceholderType *placeholder,
+                                   const Solution &S, bool forCompletion) {
+  auto &ctx = S.getConstraintSystem().getASTContext();
+  auto origTy = [&]() -> Type {
+    auto orig = placeholder->getOriginator();
+    if (auto *tv = orig.dyn_cast<TypeVariableType *>())
+      return tv;
+    if (auto *dmt = orig.dyn_cast<DependentMemberType *>())
+      return dmt;
+    return Type();
+  }();
+  if (!origTy)
+    return ErrorType::get(ctx);
+
+  // Try replace the type variable with its original generic parameter type.
+  auto replacement = origTy.transformRec([&](Type ty) -> std::optional<Type> {
+    auto *tv = dyn_cast<TypeVariableType>(ty.getPointer());
+    if (!tv)
+      return std::nullopt;
+
+    auto *gp = getGenericParamForHoleTypeVar(tv, S);
+    if (!gp)
+      return std::nullopt;
+
+    return Type(gp);
+  });
+  if (isa<TypeVariableType>(replacement.getPointer()))
+    return ErrorType::get(ctx);
+
+  // For completion, we want to produce an archetype instead of an ErrorType
+  // for a top-level generic parameter.
+  // FIXME: This is pretty weird, we're producing a contextual type outside of
+  // the context it exists in. We ought to see if we can make the completion
+  // logic work with ErrorTypes instead.
+  if (forCompletion) {
+    if (auto *GP = replacement->getAs<GenericTypeParamType>())
+      return GP->getDecl()->getInnermostDeclContext()->mapTypeIntoContext(GP);
+  }
+  // Return an ErrorType with the replacement as the original type. Note that
+  // if we failed to replace a type variable with a generic parameter in a
+  // dependent member, `ErrorType::get` will fold it away.
+  return ErrorType::get(replacement);
+}
+
+Type Solution::simplifyType(Type type, bool wantInterfaceType,
+                            bool forCompletion) const {
   // If we've been asked for an interface type, start by mapping any archetypes
   // out of context.
   if (wantInterfaceType)
@@ -1899,7 +1974,11 @@ Type Solution::simplifyType(Type type, bool wantInterfaceType) const {
             return type;
 
           auto *typePtr = type.getPointer();
-          if (isa<TypeVariableType>(typePtr) || isa<PlaceholderType>(typePtr))
+
+          if (auto *placeholder = dyn_cast<PlaceholderType>(typePtr))
+            return replacePlaceholderType(placeholder, *this, forCompletion);
+
+          if (isa<TypeVariableType>(typePtr))
             return ErrorType::get(ctx);
 
           return std::nullopt;
@@ -1911,136 +1990,8 @@ Type Solution::simplifyType(Type type, bool wantInterfaceType) const {
 }
 
 Type Solution::simplifyTypeForCodeCompletion(Type Ty) const {
-  auto &CS = getConstraintSystem();
-
-  // First, instantiate all type variables that we know, but don't replace
-  // placeholders by unresolved types.
-  Ty = CS.simplifyTypeImpl(Ty, [this](TypeVariableType *typeVar) -> Type {
-    return getFixedType(typeVar);
-  });
-
-  // Next, replace all placeholders by type variables. We know that all type
-  // variables now in the type originate from placeholders.
-  Ty = Ty.transformRec([](Type type) -> std::optional<Type> {
-    if (auto *placeholder = type->getAs<PlaceholderType>()) {
-      if (auto *typeVar =
-              placeholder->getOriginator().dyn_cast<TypeVariableType *>()) {
-        return Type(typeVar);
-      }
-    }
-
-    return std::nullopt;
-  });
-
-  // Replace all type variables (which must come from placeholders) by their
-  // generic parameters. Because we call into simplifyTypeImpl
-  Ty = CS.simplifyTypeImpl(Ty, [&CS, this](TypeVariableType *typeVar) -> Type {
-    // Code completion depends on generic parameter type being represented in
-    // terms of `ArchetypeType` since it's easy to extract protocol requirements
-    // from it.
-    auto getTypeVarAsArchetype = [](TypeVariableType *typeVar) -> Type {
-      if (auto *GP = typeVar->getImpl().getGenericParameter()) {
-        if (auto *GPD = GP->getDecl()) {
-          return GPD->getInnermostDeclContext()->mapTypeIntoContext(GP);
-        }
-      }
-      return Type();
-    };
-
-    if (auto archetype = getTypeVarAsArchetype(typeVar)) {
-      return archetype;
-    }
-
-    // Sometimes the type variable itself doesn't have have an originator that
-    // can be replaced by an archetype but one of its equivalent type variable
-    // does.
-    // Search thorough all equivalent type variables, looking for one that can
-    // be replaced by a generic parameter.
-    std::vector<std::pair<TypeVariableType *, Type>> bindings(
-        typeBindings.begin(), typeBindings.end());
-    // Make sure we iterate the bindings in a deterministic order.
-    llvm::sort(bindings, [](const std::pair<TypeVariableType *, Type> &lhs,
-                            const std::pair<TypeVariableType *, Type> &rhs) {
-      return lhs.first->getID() < rhs.first->getID();
-    });
-    for (auto binding : bindings) {
-      if (auto placeholder = binding.second->getAs<PlaceholderType>()) {
-        if (placeholder->getOriginator().dyn_cast<TypeVariableType *>() ==
-            typeVar) {
-          if (auto archetype = getTypeVarAsArchetype(binding.first)) {
-            return archetype;
-          }
-        }
-      }
-    }
-
-    // When applying the logic below to get contextual types inside result
-    // builders, the code completion type variable is connected by a one-way
-    // constraint to a type variable in the buildBlock call, but that is not the
-    // type variable that represents the argument type. We need to find the type
-    // variable representing the argument to retrieve protocol requirements from
-    // it. Look for a ArgumentConversion constraint that allows us to retrieve
-    // the argument type var.
-    auto &cg = CS.getConstraintGraph();
-
-    // FIXME: The type variable is not going to be part of the constraint graph
-    // at this point unless it was created at the outermost decision level;
-    // otherwise it has already been rolled back! Work around this by creating
-    // an empty node if one doesn't exist.
-    cg.addTypeVariable(typeVar);
-
-    for (auto argConstraint : cg[typeVar].getConstraints()) {
-      if (argConstraint->getKind() == ConstraintKind::ArgumentConversion &&
-          argConstraint->getFirstType()->getRValueType()->isEqual(typeVar)) {
-        if (auto argTV =
-                argConstraint->getSecondType()->getAs<TypeVariableType>()) {
-          if (auto archetype = getTypeVarAsArchetype(argTV)) {
-            return archetype;
-          }
-        }
-      }
-    }
-
-    return typeVar;
-  });
-
-  // Logic to determine the contextual type inside buildBlock result builders:
-  //
-  // When completing inside a result builder, the result builder
-  //   @ViewBuilder var body: some View {
-  //     Text("Foo")
-  //     #^COMPLETE^#
-  //   }
-  // gets rewritten to
-  //   @ViewBuilder var body: some View {
-  //     let $__builder2: Text
-  //     let $__builder0 = Text("Foo")
-  //     let $__builder1 = #^COMPLETE^#
-  //     $__builder2 = ViewBuilder.buildBlock($__builder0, $__builder1)
-  //     return $__builder2
-  //   }
-  // Inside the constraint system
-  //     let $__builder1 = #^COMPLETE^#
-  // gets type checked without context, so we can't know the contextual type for
-  // the code completion token. But we know that $__builder1 (and thus the type
-  // of #^COMPLETE^#) is used as the second argument to ViewBuilder.buildBlock,
-  // so we can extract the contextual type from that call. To do this, figure
-  // out the type variable that is used for $__builder1 in the buildBlock call.
-  // This type variable is connected to the type variable of $__builder1's
-  // definition by a one-way constraint.
-  if (auto TV = Ty->getAs<TypeVariableType>()) {
-    for (auto constraint : CS.getConstraintGraph()[TV].getConstraints()) {
-      if (constraint->getKind() == ConstraintKind::OneWayEqual &&
-          constraint->getSecondType()->isEqual(TV)) {
-        return simplifyTypeForCodeCompletion(constraint->getFirstType());
-      }
-    }
-  }
-
-  // Remove any remaining type variables and placeholders
-  Ty = simplifyType(Ty);
-
-  return Ty->getRValueType();
+  return simplifyType(Ty, /*wantInterfaceType*/ false, /*forCompletion*/ true)
+      ->getRValueType();
 }
 
 template <typename T>

test/Constraints/diag_missing_arg.swift

@@ -19,11 +19,11 @@ func attributedInOutFunc(x: inout @convention(c) () -> Int32) {} // expected-not
 attributedInOutFunc() // expected-error {{missing argument for parameter 'x' in call}} {{21-21=x: &<#@convention(c) () -> Int32#>}}
 
 func genericFunc1<T>(x: T) {} // expected-note * {{here}}
-genericFunc1() // expected-error {{missing argument for parameter 'x' in call}} {{14-14=x: <#_#>}}
+genericFunc1() // expected-error {{missing argument for parameter 'x' in call}} {{14-14=x: <#T#>}}
 
 protocol P {}
 func genericFunc2<T : P>(x: T) {} // expected-note * {{here}}
-genericFunc2() // expected-error {{missing argument for parameter 'x' in call}} {{14-14=x: <#_#>}}
+genericFunc2() // expected-error {{missing argument for parameter 'x' in call}} {{14-14=x: <#T#>}}
 
 typealias MyInt = Int
 func aliasedFunc(x: MyInt) {} // expected-note * {{here}}

test/Constraints/generics.swift

@@ -1040,8 +1040,8 @@ func test_requirement_failures_in_ambiguous_context() {
 // rdar://106054263 - failed to produce a diagnostic upon generic argument mismatch
 func test_mismatches_with_dependent_member_generic_arguments() {
   struct Binding<T, U> {}
-  // expected-note@-1 {{arguments to generic parameter 'T' ('Double?' and 'Data.SomeAssociated') are expected to be equal}}
-  // expected-note@-2 {{arguments to generic parameter 'U' ('Int' and 'Data.SomeAssociated') are expected to be equal}}
+  // expected-note@-1 {{arguments to generic parameter 'T' ('Double?' and 'Data.SomeAssociated' (aka 'String')) are expected to be equal}}
+  // expected-note@-2 {{arguments to generic parameter 'U' ('Int' and 'Data.SomeAssociated' (aka 'String')) are expected to be equal}}
 
   struct Data : SomeProtocol {
     typealias SomeAssociated = String
@@ -1058,7 +1058,7 @@ func test_mismatches_with_dependent_member_generic_arguments() {
 
   test2(Optional<Int>(nil), Data())
   // expected-error@-1 {{cannot convert value of type 'Optional<Int>' to expected argument type 'Optional<Data.SomeAssociated>'}}
-  // expected-note@-2 {{arguments to generic parameter 'Wrapped' ('Int' and 'Data.SomeAssociated') are expected to be equal}}
+  // expected-note@-2 {{arguments to generic parameter 'Wrapped' ('Int' and 'Data.SomeAssociated' (aka 'String')) are expected to be equal}}
 }
 
 extension Dictionary where Value == Any { // expected-note {{where 'Value' = 'any P'}}

test/Constraints/pack_expansion_types.swift

@@ -240,10 +240,10 @@ func patternInstantiationConcreteValid() {
 
 func patternInstantiationConcreteInvalid() {
   let _: Set<Int> = patternInstantiationTupleTest1()
-  // expected-error@-1 {{cannot convert value of type '(repeat Array<_>)' to specified type 'Set<Int>'}}
+  // expected-error@-1 {{cannot convert value of type 'Array<_>' to specified type 'Set<Int>'}}
   // expected-error@-2 {{could not infer pack element #0 from context}}
 
-  let _: (Array<Int>, Set<String>) = patternInstantiationTupleTest1() // expected-error {{'(repeat Array<Int, _>)' is not convertible to '(Array<Int>, Set<String>)', tuples have a different number of elements}}
+  let _: (Array<Int>, Set<String>) = patternInstantiationTupleTest1() // expected-error {{cannot convert value of type '(Array<Int>, Array<_>)' to specified type '(Array<Int>, Set<String>)'}}
   // expected-error@-1 {{could not infer pack element #1 from context}}
 }
 
@@ -274,7 +274,7 @@ func patternInstantiationGenericInvalid<each T: Hashable>(t: repeat each T) {
   let _: (repeat Set<each T>) = patternInstantiationTupleTest1() // expected-error {{cannot convert value of type '(repeat Array<each T>)' to specified type '(repeat Set<each T>)}}
   // expected-error@-1 {{generic parameter 'each T' could not be inferred}}
 
-  let _: (repeat Array<each T>, Set<String>) = patternInstantiationTupleTest1() // expected-error {{'(repeat Array<repeat each T, _>)' is not convertible to '(repeat Array<each T>, Set<String>)', tuples have a different number of elements}}
+  let _: (repeat Array<each T>, Set<String>) = patternInstantiationTupleTest1() // expected-error {{cannot convert value of type '(repeat Array<each T>, Array<_>)' to specified type '(repeat Array<each T>, Set<String>)'}}
   // expected-error@-1 {{could not infer pack element #1 from context}}
 }
 

test/Constraints/tuple_arguments.swift

@@ -1728,7 +1728,7 @@ do {
 do {
   func f(_: Int...) {}
   let _ = [(1, 2, 3)].map(f) // expected-error {{no exact matches in call to instance method 'map'}}
-  // expected-note@-1 {{found candidate with type '(((Int, Int, Int)) -> _) -> Array<_>'}}
+  // expected-note@-1 2{{found candidate with type '(((Int, Int, Int)) -> T) -> [T]'}}
 }
 
 // rdar://problem/48443263 - cannot convert value of type '() -> Void' to expected argument type '(_) -> Void'

test/Generics/issue-55666.swift

@@ -7,16 +7,16 @@ struct W<T> {}
 struct S<C1: Collection> {
   init(){}
   // expected-note@+2 {{where 'C1.Element' = 'W<C1>', 'main.W<C2.Element>' = 'main.W<C2.Element>'}}
-  // expected-note@+1 {{where 'C1.Element' = 'W<String>', 'W<C2.Element>' = 'W<Array<Int>.Element>'}}
+  // expected-note@+1 {{where 'C1.Element' = 'W<String>', 'W<C2.Element>' = 'W<Int>'}}
   init<C2>(_ c2: W<C2>) where C2: Collection, C1.Element == W<C2.Element> {}
-  // expected-note@+1 {{where 'C1.Element' = 'W<String>', 'W<C2.Element>' = 'W<Array<Int>.Element>'}}
+  // expected-note@+1 {{where 'C1.Element' = 'W<String>', 'W<C2.Element>' = 'W<Int>'}}
   static func f<C2>(_ c2: W<C2>) where C2: Collection, C1.Element == W<C2.Element> {}
-  // expected-note@+1 {{where 'C1.Element' = 'W<String>', 'W<C2.Element>' = 'W<Array<Int>.Element>'}}
+  // expected-note@+1 {{where 'C1.Element' = 'W<String>', 'W<C2.Element>' = 'W<Int>'}}
   func instancef<C2>(_ c2: W<C2>) where C2: Collection, C1.Element == W<C2.Element> {}
 }
-let _ = S<[W<String>]>(W<[Int]>()) // expected-error{{initializer 'init(_:)' requires the types 'W<String>' and 'W<Array<Int>.Element>' be equivalent}}
-let _ = S<[W<String>]>.f(W<[Int]>()) // expected-error{{static method 'f' requires the types 'W<String>' and 'W<Array<Int>.Element>' be equivalent}}
-let _ = S<[W<String>]>().instancef(W<[Int]>()) // expected-error{{instance method 'instancef' requires the types 'W<String>' and 'W<Array<Int>.Element>' be equivalent}}
+let _ = S<[W<String>]>(W<[Int]>()) // expected-error{{initializer 'init(_:)' requires the types 'W<String>' and 'W<Int>' be equivalent}}
+let _ = S<[W<String>]>.f(W<[Int]>()) // expected-error{{static method 'f' requires the types 'W<String>' and 'W<Int>' be equivalent}}
+let _ = S<[W<String>]>().instancef(W<[Int]>()) // expected-error{{instance method 'instancef' requires the types 'W<String>' and 'W<Int>' be equivalent}}
 
 // Archetypes requirement failure
 func genericFunc<C1: Collection, C2: Collection>(_ c2: W<C2>, c1: C1.Type) where C1.Element == W<C2.Element> {