Explorar o código

Order impl matching by type structure (#2691)

As described in [the generics design](https://github.com/carbon-language/carbon-lang/blob/trunk/docs/design/generics/details.md#type-structure-of-an-impl-declaration), `impl` declarations are prioritized by type structure. Given two `impl` declarations that match a `type as interface` query, the one that describes the longest prefix of the query without using placeholders is preferred.

We implement this by putting all impls in a total order, first by type structure equivalence classes and then by lexical order. When matching an impl, we walk this total order, and stop once we find a match and reach the end of its equivalence class.

Equivalence classes are determined by finding the locations of the "holes" (the positions where deduced parameters appear) within the type structure, viewed as a tree. Two impls are in the same equivalence class if their holes are in the same place, and equivalence classes are ordered based on a reverse lexicographical ordering of their holes.

Explorer doesn't keep the `Bindings` list for a parameterized type in any particular order, but the type structure rule requires that we consider them in lexical order. In order to support this, we now track an index on the declared parameters of each generic. This is a simple numbering of enclosing generic parameters, both on that generic and on all lexically enclosing generics.

Co-authored-by: Jon Ross-Perkins <jperkins@google.com>
Richard Smith %!s(int64=3) %!d(string=hai) anos
pai
achega
28946d4b87

+ 22 - 0
explorer/ast/pattern.h

@@ -105,6 +105,17 @@ class Pattern : public AstNode {
 auto VisitNestedPatterns(const Pattern& pattern,
                          llvm::function_ref<bool(const Pattern&)> visitor)
     -> bool;
+inline auto VisitNestedPatterns(Pattern& pattern,
+                                llvm::function_ref<bool(Pattern&)> visitor)
+    -> bool {
+  // The non-const version is is implemented in terms of the const version. The
+  // const_cast is safe because every pattern reachable through a non-const
+  // pattern is also non-const.
+  const Pattern& const_pattern = pattern;
+  return VisitNestedPatterns(const_pattern, [&](const Pattern& inner) {
+    return visitor(const_cast<Pattern&>(inner));
+  });
+}
 
 // A pattern consisting of the `auto` keyword.
 class AutoPattern : public Pattern {
@@ -252,6 +263,16 @@ class GenericBinding : public Pattern {
   auto type() const -> const Expression& { return *type_; }
   auto type() -> Expression& { return *type_; }
 
+  // The index of this binding, which is the number of bindings that are in
+  // scope at the point where this binding is declared.
+  auto index() const -> int { return *index_; }
+
+  // Set the index of this binding. Should be called only during type-checking.
+  void set_index(int index) {
+    CARBON_CHECK(!index_) << "should only set depth and index once";
+    index_ = index;
+  }
+
   auto value_category() const -> ValueCategory { return ValueCategory::Let; }
 
   auto constant_value() const -> std::optional<Nonnull<const Value*>> {
@@ -299,6 +320,7 @@ class GenericBinding : public Pattern {
  private:
   std::string name_;
   Nonnull<Expression*> type_;
+  std::optional<int> index_;
   std::optional<Nonnull<const Value*>> symbolic_identity_;
   std::optional<Nonnull<const ImplBinding*>> impl_binding_;
   std::optional<Nonnull<const GenericBinding*>> original_;

+ 17 - 0
explorer/interpreter/BUILD

@@ -214,6 +214,7 @@ cc_library(
         ":interpreter",
         ":pattern_analysis",
         ":trace_stream",
+        ":type_structure",
         "//common:check",
         "//common:enum_base",
         "//common:error",
@@ -227,6 +228,22 @@ cc_library(
     ],
 )
 
+cc_library(
+    name = "type_structure",
+    srcs = [
+        "type_structure.cpp",
+    ],
+    hdrs = [
+        "type_structure.h",
+    ],
+    deps = [
+        "//common:ostream",
+        "//explorer/ast",
+        "//explorer/common:nonnull",
+        "@llvm-project//llvm:Support",
+    ],
+)
+
 cc_library(
     name = "resolve_unformed",
     srcs = [

+ 111 - 44
explorer/interpreter/impl_scope.cpp

@@ -11,6 +11,7 @@
 
 using llvm::cast;
 using llvm::dyn_cast;
+using llvm::isa;
 
 namespace Carbon {
 
@@ -25,8 +26,11 @@ void ImplScope::Add(Nonnull<const Value*> iface,
                     Nonnull<const Value*> type,
                     llvm::ArrayRef<Nonnull<const ImplBinding*>> impl_bindings,
                     Nonnull<const Witness*> witness,
-                    const TypeChecker& type_checker) {
+                    const TypeChecker& type_checker,
+                    std::optional<TypeStructureSortKey> sort_key) {
   if (const auto* constraint = dyn_cast<ConstraintType>(iface)) {
+    CARBON_CHECK(!sort_key)
+        << "should only be given a sort key for an impl of an interface";
     // The caller should have substituted `.Self` for `type` already.
     Add(constraint->impl_constraints(), deduced, impl_bindings, witness,
         type_checker);
@@ -42,11 +46,21 @@ void ImplScope::Add(Nonnull<const Value*> iface,
     return;
   }
 
-  impls_.push_back({.interface = cast<InterfaceType>(iface),
-                    .deduced = deduced,
-                    .type = type,
-                    .impl_bindings = impl_bindings,
-                    .witness = witness});
+  Impl new_impl = {.interface = cast<InterfaceType>(iface),
+                   .deduced = deduced,
+                   .type = type,
+                   .impl_bindings = impl_bindings,
+                   .witness = witness,
+                   .sort_key = std::move(sort_key)};
+
+  // Find the first impl that's more specific than this one, and place this
+  // impl right before it. This keeps the impls with the same type structure
+  // sorted in lexical order, which is important for `match_first` semantics.
+  auto insert_pos = std::upper_bound(
+      impls_.begin(), impls_.end(), new_impl,
+      [](const Impl& a, const Impl& b) { return a.sort_key < b.sort_key; });
+
+  impls_.insert(insert_pos, std::move(new_impl));
 }
 
 void ImplScope::Add(llvm::ArrayRef<ImplConstraint> impls,
@@ -234,14 +248,39 @@ auto ImplScope::TryResolveInterface(Nonnull<const InterfaceType*> iface_type,
                                     bool diagnose_missing_impl) const
     -> ErrorOr<std::optional<Nonnull<const Witness*>>> {
   CARBON_ASSIGN_OR_RETURN(
-      std::optional<Nonnull<const Witness*>> result,
+      std::optional<ResolveResult> result,
       TryResolveInterfaceRecursively(iface_type, type, source_loc, *this,
                                      type_checker));
   if (!result.has_value() && diagnose_missing_impl) {
     return ProgramError(source_loc) << "could not find implementation of "
                                     << *iface_type << " for " << *type;
   }
-  return result;
+  return result ? std::optional(result->witness) : std::nullopt;
+}
+
+// Do these two witnesses refer to `impl` declarations in the same
+// `match_first` block?
+static auto InSameMatchFirst(Nonnull<const Witness*> a,
+                             Nonnull<const Witness*> b) -> bool {
+  const auto* impl_a = dyn_cast<ImplWitness>(a);
+  const auto* impl_b = dyn_cast<ImplWitness>(b);
+  if (!impl_b || !impl_b) {
+    return false;
+  }
+
+  // TODO: Once we support an impl being declared more than once, we will need
+  // to check this more carefully.
+  return impl_a->declaration().match_first() &&
+         impl_a->declaration().match_first() ==
+             impl_b->declaration().match_first();
+}
+
+// Determine whether this result is definitely right -- that there can be no
+// specialization that would give a better match.
+static auto IsEffectivelyFinal(ImplScope::ResolveResult result) -> bool {
+  // TODO: Once we support 'final', check whether this is a final impl
+  // declaration if it's parameterized.
+  return result.impl->deduced.empty();
 }
 
 // Combines the results of two impl lookups. In the event of a tie, arbitrarily
@@ -249,9 +288,9 @@ auto ImplScope::TryResolveInterface(Nonnull<const InterfaceType*> iface_type,
 static auto CombineResults(Nonnull<const InterfaceType*> iface_type,
                            Nonnull<const Value*> type,
                            SourceLocation source_loc,
-                           std::optional<Nonnull<const Witness*>> a,
-                           std::optional<Nonnull<const Witness*>> b)
-    -> ErrorOr<std::optional<Nonnull<const Witness*>>> {
+                           std::optional<ImplScope::ResolveResult> a,
+                           std::optional<ImplScope::ResolveResult> b)
+    -> ErrorOr<std::optional<ImplScope::ResolveResult>> {
   // If only one lookup succeeded, return that.
   if (!b) {
     return a;
@@ -260,17 +299,37 @@ static auto CombineResults(Nonnull<const InterfaceType*> iface_type,
     return b;
   }
 
-  // If either of them was a symbolic result, then they'll end up being
-  // equivalent. In that case, pick `a`.
-  const auto* impl_a = dyn_cast<ImplWitness>(*a);
-  const auto* impl_b = dyn_cast<ImplWitness>(*b);
-  if (!impl_b) {
+  // If exactly one of them is effectively final, prefer that result.
+  bool a_is_final = IsEffectivelyFinal(*a);
+  bool b_is_final = IsEffectivelyFinal(*b);
+  if (a_is_final && !b_is_final) {
     return a;
-  }
-  if (!impl_a) {
+  } else if (b_is_final && !a_is_final) {
     return b;
   }
 
+  const auto* impl_a = dyn_cast<ImplWitness>(a->witness);
+  const auto* impl_b = dyn_cast<ImplWitness>(b->witness);
+
+  // If both are effectively final, prefer an impl declaration over a
+  // symbolic ImplBinding, because we get more information from the impl
+  // declaration. If they're both symbolic, arbitrarily pick a.
+  if (a_is_final && b_is_final) {
+    if (!impl_b) {
+      return a;
+    }
+    if (!impl_a) {
+      return b;
+    }
+  }
+  CARBON_CHECK(impl_a && impl_b) << "non-final impl should not be symbolic";
+
+  // At this point, we're comparing two `impl` declarations, and either they're
+  // both final or neither of them is.
+  // TODO: We should reject the case where both are final when checking their
+  // declarations, but we don't do so yet, so for now we report it as an
+  // ambiguity.
+  //
   // If they refer to the same `impl` declaration, it doesn't matter which one
   // we pick, so we pick `a`.
   // TODO: Compare the identities of the `impl`s, not the declarations.
@@ -278,24 +337,6 @@ static auto CombineResults(Nonnull<const InterfaceType*> iface_type,
     return a;
   }
 
-  // TODO: Order the `impl`s based on type structure.
-
-  // If the declarations appear in the same `match_first` block, whichever
-  // appears first wins.
-  // TODO: Once we support an impl being declared more than once, we will need
-  // to check this more carefully.
-  if (impl_a->declaration().match_first() &&
-      impl_a->declaration().match_first() ==
-          impl_b->declaration().match_first()) {
-    for (const auto* impl : (*impl_a->declaration().match_first())->impls()) {
-      if (impl == &impl_a->declaration()) {
-        return a;
-      }
-      if (impl == &impl_b->declaration()) {
-        return b;
-      }
-    }
-  }
   return ProgramError(source_loc)
          << "ambiguous implementations of " << *iface_type << " for " << *type;
 }
@@ -304,14 +345,14 @@ auto ImplScope::TryResolveInterfaceRecursively(
     Nonnull<const InterfaceType*> iface_type, Nonnull<const Value*> type,
     SourceLocation source_loc, const ImplScope& original_scope,
     const TypeChecker& type_checker) const
-    -> ErrorOr<std::optional<Nonnull<const Witness*>>> {
+    -> ErrorOr<std::optional<ResolveResult>> {
   CARBON_ASSIGN_OR_RETURN(
-      std::optional<Nonnull<const Witness*>> result,
+      std::optional<ResolveResult> result,
       TryResolveInterfaceHere(iface_type, type, source_loc, original_scope,
                               type_checker));
   if (parent_scope_) {
     CARBON_ASSIGN_OR_RETURN(
-        std::optional<Nonnull<const Witness*>> parent_result,
+        std::optional<ResolveResult> parent_result,
         (*parent_scope_)
             ->TryResolveInterfaceRecursively(iface_type, type, source_loc,
                                              original_scope, type_checker));
@@ -325,14 +366,37 @@ auto ImplScope::TryResolveInterfaceHere(
     Nonnull<const InterfaceType*> iface_type, Nonnull<const Value*> impl_type,
     SourceLocation source_loc, const ImplScope& original_scope,
     const TypeChecker& type_checker) const
-    -> ErrorOr<std::optional<Nonnull<const Witness*>>> {
-  std::optional<Nonnull<const Witness*>> result = std::nullopt;
+    -> ErrorOr<std::optional<ResolveResult>> {
+  std::optional<ResolveResult> result = std::nullopt;
   for (const Impl& impl : impls_) {
-    CARBON_ASSIGN_OR_RETURN(std::optional<Nonnull<const Witness*>> m,
+    // If we've passed the final impl with a sort key matching our best impl,
+    // all further impls are worse and don't need to be checked.
+    if (result && result->impl->sort_key < impl.sort_key) {
+      break;
+    }
+
+    // If this impl appears later in the same match_first block as our best
+    // result, we should not consider it.
+    //
+    // TODO: This should apply transitively: if we have
+    //   match_first { impl a; impl b; }
+    //   match_first { impl b; impl c; }
+    // then we should not consider c once we match a. For now, because each
+    // impl is only declared once, this is not a problem.
+    if (result && InSameMatchFirst(result->impl->witness, impl.witness)) {
+      continue;
+    }
+
+    // Try matching this impl against our query.
+    CARBON_ASSIGN_OR_RETURN(std::optional<Nonnull<const Witness*>> witness,
                             type_checker.MatchImpl(*iface_type, impl_type, impl,
                                                    original_scope, source_loc));
-    CARBON_ASSIGN_OR_RETURN(
-        result, CombineResults(iface_type, impl_type, source_loc, result, m));
+    if (witness) {
+      CARBON_ASSIGN_OR_RETURN(
+          result,
+          CombineResults(iface_type, impl_type, source_loc, result,
+                         ResolveResult{.impl = &impl, .witness = *witness}));
+    }
   }
   return result;
 }
@@ -343,6 +407,9 @@ void ImplScope::Print(llvm::raw_ostream& out) const {
   llvm::ListSeparator sep;
   for (const Impl& impl : impls_) {
     out << sep << *(impl.type) << " as " << *(impl.interface);
+    if (impl.sort_key) {
+      out << " " << *impl.sort_key;
+    }
   }
   for (Nonnull<const EqualityConstraint*> eq : equalities_) {
     out << sep;

+ 32 - 19
explorer/interpreter/impl_scope.h

@@ -7,6 +7,7 @@
 
 #include "explorer/ast/declaration.h"
 #include "explorer/ast/value.h"
+#include "explorer/interpreter/type_structure.h"
 
 namespace Carbon {
 
@@ -42,6 +43,33 @@ class TypeChecker;
 // scope.
 class ImplScope {
  public:
+  // The `Impl` struct is a key-value pair where the key is the
+  // combination of a type and an interface, e.g., `List` and `Container`,
+  // and the value is the result of statically resolving to the `impl`
+  // for `List` as `Container`, which is an `Expression` that produces
+  // the witness for that `impl`.
+  //
+  // When the `impl` is parameterized, `deduced` and `impl_bindings`
+  // are non-empty. The former contains the type parameters and the
+  // later are impl bindings, that is, parameters for witnesses. In this case,
+  // `sort_key` indicates the order in which this impl should be considered
+  // relative to other matching impls.
+  struct Impl {
+    Nonnull<const InterfaceType*> interface;
+    std::vector<Nonnull<const GenericBinding*>> deduced;
+    Nonnull<const Value*> type;
+    std::vector<Nonnull<const ImplBinding*>> impl_bindings;
+    Nonnull<const Witness*> witness;
+    std::optional<TypeStructureSortKey> sort_key;
+  };
+
+  // Internal type used to represent the result of resolving a lookup in a
+  // particular impl scope.
+  struct ResolveResult {
+    Nonnull<const Impl*> impl;
+    Nonnull<const Witness*> witness;
+  };
+
   explicit ImplScope() {}
   explicit ImplScope(Nonnull<const ImplScope*> parent)
       : parent_scope_(parent) {}
@@ -59,7 +87,8 @@ class ImplScope {
            llvm::ArrayRef<Nonnull<const GenericBinding*>> deduced,
            Nonnull<const Value*> type,
            llvm::ArrayRef<Nonnull<const ImplBinding*>> impl_bindings,
-           Nonnull<const Witness*> witness, const TypeChecker& type_checker);
+           Nonnull<const Witness*> witness, const TypeChecker& type_checker,
+           std::optional<TypeStructureSortKey> sort_key = std::nullopt);
   // Adds a list of impl constraints from a constraint type into scope. Any
   // references to `.Self` are expected to have already been substituted for
   // the type implementing the constraint.
@@ -109,22 +138,6 @@ class ImplScope {
 
   void Print(llvm::raw_ostream& out) const;
 
-  // The `Impl` struct is a key-value pair where the key is the
-  // combination of a type and an interface, e.g., `List` and `Container`,
-  // and the value is the result of statically resolving to the `impl`
-  // for `List` as `Container`, which is an `Expression` that produces
-  // the witness for that `impl`.
-  // When the `impl` is parameterized, `deduced` and `impl_bindings`
-  // are non-empty. The former contains the type parameters and the
-  // later are impl bindings, that is, parameters for witnesses.
-  struct Impl {
-    Nonnull<const InterfaceType*> interface;
-    std::vector<Nonnull<const GenericBinding*>> deduced;
-    Nonnull<const Value*> type;
-    std::vector<Nonnull<const ImplBinding*>> impl_bindings;
-    Nonnull<const Witness*> witness;
-  };
-
  private:
   // Returns the associated impl for the given `iface` and `type` in
   // the ancestor graph of this scope. Reports a compilation error
@@ -148,7 +161,7 @@ class ImplScope {
                                       SourceLocation source_loc,
                                       const ImplScope& original_scope,
                                       const TypeChecker& type_checker) const
-      -> ErrorOr<std::optional<Nonnull<const Witness*>>>;
+      -> ErrorOr<std::optional<ResolveResult>>;
 
   // Returns the associated impl for the given `iface` and `type` in
   // this scope, returns std::nullopt if there is none, or reports
@@ -161,7 +174,7 @@ class ImplScope {
                                SourceLocation source_loc,
                                const ImplScope& original_scope,
                                const TypeChecker& type_checker) const
-      -> ErrorOr<std::optional<Nonnull<const Witness*>>>;
+      -> ErrorOr<std::optional<ResolveResult>>;
 
   std::vector<Impl> impls_;
   std::vector<Nonnull<const EqualityConstraint*>> equalities_;

+ 37 - 13
explorer/interpreter/type_checker.cpp

@@ -30,6 +30,7 @@
 #include "explorer/interpreter/impl_scope.h"
 #include "explorer/interpreter/interpreter.h"
 #include "explorer/interpreter/pattern_analysis.h"
+#include "explorer/interpreter/type_structure.h"
 #include "llvm/ADT/DenseSet.h"
 #include "llvm/ADT/STLExtras.h"
 #include "llvm/ADT/ScopeExit.h"
@@ -3811,11 +3812,12 @@ auto TypeChecker::TypeCheckExp(Nonnull<Expression*> e,
   }
 }
 
-void TypeChecker::CollectGenericBindingsInPattern(
-    Nonnull<const Pattern*> p,
+void TypeChecker::CollectAndNumberGenericBindingsInPattern(
+    Nonnull<Pattern*> p,
     std::vector<Nonnull<const GenericBinding*>>& generic_bindings) {
-  VisitNestedPatterns(*p, [&](const Pattern& pattern) {
-    if (const auto* binding = dyn_cast<GenericBinding>(&pattern)) {
+  VisitNestedPatterns(*p, [&](Pattern& pattern) {
+    if (auto* binding = dyn_cast<GenericBinding>(&pattern)) {
+      binding->set_index(generic_bindings.size());
       generic_bindings.push_back(binding);
     }
     return true;
@@ -4555,20 +4557,21 @@ auto TypeChecker::DeclareCallableDeclaration(Nonnull<CallableDeclaration*> f,
     *trace_stream_ << "** declaring function " << *name << "\n";
   }
   ImplScope function_scope(scope_info.innermost_scope);
-  std::vector<Nonnull<const GenericBinding*>> deduced_bindings;
+  std::vector<Nonnull<const GenericBinding*>> all_bindings =
+      scope_info.bindings;
   std::vector<Nonnull<const ImplBinding*>> impl_bindings;
   // Bring the deduced parameters into scope.
   for (Nonnull<GenericBinding*> deduced : f->deduced_parameters()) {
     CARBON_RETURN_IF_ERROR(TypeCheckPattern(
         deduced, std::nullopt, function_scope, ValueCategory::Let));
-    CollectGenericBindingsInPattern(deduced, deduced_bindings);
+    CollectAndNumberGenericBindingsInPattern(deduced, all_bindings);
     CollectImplBindingsInPattern(deduced, impl_bindings);
   }
   // Type check the receiver pattern.
   if (f->is_method()) {
     CARBON_RETURN_IF_ERROR(TypeCheckPattern(
         &f->self_pattern(), std::nullopt, function_scope, ValueCategory::Let));
-    CollectGenericBindingsInPattern(&f->self_pattern(), deduced_bindings);
+    CollectAndNumberGenericBindingsInPattern(&f->self_pattern(), all_bindings);
     CollectImplBindingsInPattern(&f->self_pattern(), impl_bindings);
   }
   // Type check the parameter pattern.
@@ -4576,15 +4579,25 @@ auto TypeChecker::DeclareCallableDeclaration(Nonnull<CallableDeclaration*> f,
                                           function_scope, ValueCategory::Let));
   CollectImplBindingsInPattern(&f->param_pattern(), impl_bindings);
 
+  // All bindings we've seen so far in this scope are our deduced bindings.
+  std::vector<Nonnull<const GenericBinding*>> deduced_bindings(
+      all_bindings.begin() + scope_info.bindings.size(), all_bindings.end());
+
   // Keep track of any generic parameters and nested generic bindings in the
   // parameter pattern.
   std::vector<FunctionType::GenericParameter> generic_parameters;
   for (size_t i = 0; i != f->param_pattern().fields().size(); ++i) {
-    const Pattern* param_pattern = f->param_pattern().fields()[i];
+    Pattern* param_pattern = f->param_pattern().fields()[i];
+
+    size_t old_size = all_bindings.size();
+    CollectAndNumberGenericBindingsInPattern(param_pattern, all_bindings);
+
     if (const auto* binding = dyn_cast<GenericBinding>(param_pattern)) {
       generic_parameters.push_back({i, binding});
     } else {
-      CollectGenericBindingsInPattern(param_pattern, deduced_bindings);
+      deduced_bindings.insert(deduced_bindings.end(),
+                              all_bindings.begin() + old_size,
+                              all_bindings.end());
     }
   }
 
@@ -4725,7 +4738,7 @@ auto TypeChecker::DeclareClassDeclaration(Nonnull<ClassDeclaration*> class_decl,
     Nonnull<TuplePattern*> type_params = *class_decl->type_params();
     CARBON_RETURN_IF_ERROR(TypeCheckPattern(type_params, std::nullopt,
                                             class_scope, ValueCategory::Let));
-    CollectGenericBindingsInPattern(type_params, bindings);
+    CollectAndNumberGenericBindingsInPattern(type_params, bindings);
     if (trace_stream_->is_enabled()) {
       *trace_stream_ << class_scope;
     }
@@ -4993,7 +5006,8 @@ auto TypeChecker::DeclareConstraintTypeDeclaration(
     if (trace_stream_->is_enabled()) {
       *trace_stream_ << constraint_scope;
     }
-    CollectGenericBindingsInPattern(*constraint_decl->params(), bindings);
+    CollectAndNumberGenericBindingsInPattern(*constraint_decl->params(),
+                                             bindings);
   }
 
   // Form the full symbolic type of the interface or named constraint. This is
@@ -5301,12 +5315,22 @@ auto TypeChecker::CheckAndAddImplBindings(
                                                  impl_type, self_witness,
                                                  iface_witness, iface_scope));
 
+      std::optional<TypeStructureSortKey> sort_key;
+      if (deduced_bindings.size()) {
+        sort_key = TypeStructureSortKey::ForImpl(impl_type, iface_type);
+        if (trace_stream_->is_enabled()) {
+          *trace_stream_ << "type structure sort key for `impl " << *impl_type
+                         << " as " << *iface_type << "` is " << sort_key
+                         << "\n";
+        }
+      }
+
       // TODO: We should do this either before checking any interface or after
       // checking all of them, so that the order of lookup contexts doesn't
       // matter.
       scope_info.innermost_non_class_scope->Add(
           iface_type, deduced_bindings, impl_type, impl_decl->impl_bindings(),
-          self_witness, *this);
+          self_witness, *this, sort_key);
     } else if (isa<NamedConstraintType>(lookup.context)) {
       // Nothing to check here, since a named constraint can't introduce any
       // associated entities.
@@ -5517,7 +5541,7 @@ auto TypeChecker::DeclareChoiceDeclaration(Nonnull<ChoiceDeclaration*> choice,
     Nonnull<TuplePattern*> type_params = *choice->type_params();
     CARBON_RETURN_IF_ERROR(TypeCheckPattern(type_params, std::nullopt,
                                             choice_scope, ValueCategory::Let));
-    CollectGenericBindingsInPattern(type_params, bindings);
+    CollectAndNumberGenericBindingsInPattern(type_params, bindings);
     if (trace_stream_->is_enabled()) {
       *trace_stream_ << choice_scope;
     }

+ 4 - 3
explorer/interpreter/type_checker.h

@@ -273,9 +273,10 @@ class TypeChecker {
   auto DeclareAliasDeclaration(Nonnull<AliasDeclaration*> alias,
                                const ScopeInfo& scope_info) -> ErrorOr<Success>;
 
-  // Find all of the GenericBindings in the given pattern.
-  void CollectGenericBindingsInPattern(
-      Nonnull<const Pattern*> p,
+  // Find all of the GenericBindings in the given pattern, and assign them
+  // index numbers based on their position in the resulting vector of bindings.
+  void CollectAndNumberGenericBindingsInPattern(
+      Nonnull<Pattern*> p,
       std::vector<Nonnull<const GenericBinding*>>& generic_bindings);
 
   // Find all of the ImplBindings in the given pattern. The pattern is required

+ 141 - 0
explorer/interpreter/type_structure.cpp

@@ -0,0 +1,141 @@
+// Part of the Carbon Language project, under the Apache License v2.0 with LLVM
+// Exceptions. See /LICENSE for license information.
+// SPDX-License-Identifier: Apache-2.0 WITH LLVM-exception
+
+#include "explorer/interpreter/type_structure.h"
+
+#include <limits>
+
+#include "explorer/ast/declaration.h"
+#include "explorer/ast/value.h"
+#include "llvm/ADT/StringExtras.h"
+
+namespace Carbon {
+
+namespace {
+struct TypeStructureBuilder {
+  // Visit a child of the current value at the given index.
+  template <typename T>
+  void VisitChild(int index, const T& child) {
+    path.push_back(index);
+    Visit(child);
+    path.pop_back();
+  }
+
+  // Visit an instance of a type derived from Value. By default we do this by
+  // decomposing the value and walking each piece in turn.
+  template <typename T>
+  void VisitValue(Nonnull<const T*> value) {
+    value->Decompose([&](const auto&... parts) {
+      int inner_index = 0;
+      (VisitChild(inner_index++, parts), ...);
+    });
+  }
+
+  // A variable type is a hole.
+  void VisitValue(Nonnull<const VariableType*>) { AddHole(); }
+
+  // Visit a value by visiting its derived type.
+  void Visit(Nonnull<const Value*> value) {
+    value->Visit<void>([&](auto* derived_value) { VisitValue(derived_value); });
+  }
+
+  // Visit all of the arguments in a list of bindings.
+  void Visit(Nonnull<const Bindings*> bindings) {
+    // Reconstruct the lexical ordering of the parameters.
+    // TODO: Store bindings as an array indexed by binding index, not as a map.
+    std::vector<Nonnull<const GenericBinding*>> params;
+    for (auto [param, value] : bindings->args()) {
+      params.push_back(param);
+    }
+    std::sort(params.begin(), params.end(), [](auto* param_1, auto* param_2) {
+      return param_1->index() < param_2->index();
+    });
+
+    for (int i = 0; i != static_cast<int>(params.size()); ++i) {
+      VisitChild(i, bindings->args().find(params[i])->second);
+    }
+  }
+
+  template <typename T>
+  void Visit(const std::optional<T>& opt) {
+    if (opt) {
+      Visit(*opt);
+    }
+  }
+
+  template <typename T>
+  void Visit(const std::vector<T>& seq) {
+    for (int i = 0; i != static_cast<int>(seq.size()); ++i) {
+      VisitChild(i, seq[i]);
+    }
+  }
+
+  void Visit(const NamedValue& value) { Visit(value.value); }
+
+  // Ignore values that can't contain holes.
+  void Visit(int) {}
+  void Visit(std::string_view) {}
+  void Visit(Nonnull<const AstNode*>) {}
+  void Visit(const ValueNodeView&) {}
+  void Visit(const Address&) {}
+  void Visit(const VTable&) {}
+  void Visit(const FunctionType::GenericParameter&) {}
+  void Visit(const NamedElement&) {}
+  void Visit(Nonnull<const ContinuationValue::Representation*>) {}
+
+  // Constraint types can contain mentions of VariableTypes, but they aren't
+  // deducible so it's not important to look for them.
+  void Visit(const ImplConstraint&) {}
+  void Visit(const IntrinsicConstraint&) {}
+  void Visit(const EqualityConstraint&) {}
+  void Visit(const RewriteConstraint&) {}
+  void Visit(const LookupContext&) {}
+
+  // TODO: Find a way to remove the derived-most pointer from NominalClassValue.
+  void Visit(Nonnull<const NominalClassValue**>) {}
+
+  void AddHole() {
+    if (!result.empty()) {
+      result.push_back(-1);
+    }
+    result.insert(result.end(), path.begin(), path.end());
+  }
+
+  std::vector<int> path;
+  std::vector<int> result;
+};
+}  // namespace
+
+auto TypeStructureSortKey::ForImpl(Nonnull<const Value*> type,
+                                   Nonnull<const Value*> interface)
+    -> TypeStructureSortKey {
+  TypeStructureBuilder builder;
+  builder.VisitChild(0, type);
+  builder.VisitChild(1, interface);
+
+  TypeStructureSortKey result;
+  result.holes_ = std::move(builder.result);
+  result.holes_.push_back(std::numeric_limits<int>::max());
+  return result;
+}
+
+void TypeStructureSortKey::Print(llvm::raw_ostream& out) const {
+  out << "[";
+  llvm::ListSeparator sep;
+  for (int i : holes_) {
+    if (i == -1) {
+      out << "; ";
+      // Reinitialize `sep` to suppress the next separator.
+      sep = llvm::ListSeparator();
+    } else if (i == std::numeric_limits<int>::max()) {
+      out << "]";
+    } else {
+      out << sep << i;
+    }
+  }
+}
+
+void TypeStructureSortKey::Dump() const { Print(llvm::errs()); }
+
+}  // namespace Carbon

+ 85 - 0
explorer/interpreter/type_structure.h

@@ -0,0 +1,85 @@
+// Part of the Carbon Language project, under the Apache License v2.0 with LLVM
+// Exceptions. See /LICENSE for license information.
+// SPDX-License-Identifier: Apache-2.0 WITH LLVM-exception
+
+#ifndef CARBON_EXPLORER_INTERPRETER_TYPE_STRUCTURE_H_
+#define CARBON_EXPLORER_INTERPRETER_TYPE_STRUCTURE_H_
+
+#include <vector>
+
+#include "common/ostream.h"
+#include "explorer/common/nonnull.h"
+#include "llvm/Support/Compiler.h"
+
+namespace Carbon {
+
+class Value;
+
+// A type structure sort key represents the information needed to order `impl`
+// declarations by their type structures.
+//
+// The type structure for an `impl` declaration is the `type as interface`
+// portion, with all references to the enclosing generic parameters replaced by
+// `?`s. Two type structures that match the same type and interface are ordered
+// based on the lexical position of the first `?` that is in one but not the
+// other. The type structure with the earlier `?` has the worse match.
+//
+// This class extracts the relevant information needed to order type
+// structures, and provides a weak ordering over type structures in which type
+// structures that provide a better match compare earlier.
+class TypeStructureSortKey {
+ public:
+  // Compute the sort key for `impl type as interface`.
+  static auto ForImpl(Nonnull<const Value*> type,
+                      Nonnull<const Value*> interface) -> TypeStructureSortKey;
+
+  // Order by sort key. Smaller keys represent better matches.
+  friend bool operator<(const TypeStructureSortKey& a,
+                        const TypeStructureSortKey& b) {
+    return a.holes_ > b.holes_;
+  }
+  friend bool operator<=(const TypeStructureSortKey& a,
+                         const TypeStructureSortKey& b) {
+    return a.holes_ >= b.holes_;
+  }
+  friend bool operator>(const TypeStructureSortKey& a,
+                        const TypeStructureSortKey& b) {
+    return a.holes_ < b.holes_;
+  }
+  friend bool operator>=(const TypeStructureSortKey& a,
+                         const TypeStructureSortKey& b) {
+    return a.holes_ <= b.holes_;
+  }
+
+  // Determine whether two sort keys are in the same equivalence class. If so,
+  // the sort keys represent type structures with `?`s in the same positions.
+  // This does not imply that the remainder of the type structure is the same.
+  // For example, the sort key for `Optional(?) as Hash` is the same as the
+  // sort key for `Vector(?) as Ordered`.
+  friend bool operator==(const TypeStructureSortKey& a,
+                         const TypeStructureSortKey& b) {
+    return a.holes_ == b.holes_;
+  }
+  friend bool operator!=(const TypeStructureSortKey& a,
+                         const TypeStructureSortKey& b) {
+    return a.holes_ != b.holes_;
+  }
+
+  void Print(llvm::raw_ostream& out) const;
+
+  LLVM_DUMP_METHOD void Dump() const;
+
+ private:
+  // Positions of holes (`?`s) in the structure. Each hole is described as a
+  // path of indexes from the root of the type tree to the position of the `?`.
+  // Holes are listed in appearance order, separated by -1s, and the vector is
+  // terminated by std::numeric_limits<int>::max().
+  //
+  // This representation is chosen so that better matches are lexicographically
+  // larger than worse matches.
+  std::vector<int> holes_;
+};
+
+}  // namespace Carbon
+
+#endif  // CARBON_EXPLORER_INTERPRETER_TYPE_STRUCTURE_H_

+ 65 - 0
explorer/testdata/assoc_const/bug_multi_impl_scoping.carbon

@@ -0,0 +1,65 @@
+// Part of the Carbon Language project, under the Apache License v2.0 with LLVM
+// Exceptions. See /LICENSE for license information.
+// SPDX-License-Identifier: Apache-2.0 WITH LLVM-exception
+//
+// AUTOUPDATE
+// RUN: %{explorer-run}
+// RUN: %{explorer-run-trace}
+// CHECK:STDOUT: (C(i32) as A).FA()
+// CHECK:STDOUT: 6
+// CHECK:STDOUT: (C(i32) as A).FA()
+// CHECK:STDOUT: 7
+// CHECK:STDOUT: (C(T) as B).FB()
+// CHECK:STDOUT: (C(T) as A).FA()
+// CHECK:STDOUT: 3
+// CHECK:STDOUT: result: 0
+
+package ExplorerTest api;
+
+interface A {
+  let TA:! type;
+  fn FA() -> TA;
+}
+interface B {
+  let TB:! type;
+  fn FB() -> TB;
+}
+
+class C(T:! type) {
+  impl as A & B where .TA = i32 and .TB = i32 {
+    fn FA() -> i32 {
+      Print("(C(T) as A).FA()");
+      // OK, know that TA is i32 here.
+      let v: Self.(A.TA) = 1;
+      let w: i32 = v;
+      return w;
+    }
+    fn FB() -> i32 {
+      Print("(C(T) as B).FB()");
+      // OK, know that TB is i32 here.
+      let v: Self.(B.TB) = 2;
+      // Don't know that TA is i32; it could be specialized.
+      // TODO: We should not accept this.
+      let w: Self.(A.TA) = Self.(A.FA)();
+      return v + w;
+    }
+  }
+}
+
+external impl C(i32) as A where .TA = (i32, i32) {
+  fn FA() -> (i32, i32) {
+    Print("(C(i32) as A).FA()");
+    return (6, 7);
+  }
+}
+
+fn Main() -> i32 {
+  Print("{0}", C(i32).(A.FA)()[0]);
+  Print("{0}", C(i32).(A.FA)()[1]);
+  // TODO: The implementation of C(T) as B eagerly picked the (non-final)
+  // implementation of C(T) as A, so this ends up using a different
+  // implementation of C(i32) as A than the one we just used, violating
+  // coherence.
+  Print("{0}", C(i32).(B.FB)());
+  return 0;
+}

+ 0 - 44
explorer/testdata/assoc_const/fail_multi_impl_scoping.carbon

@@ -1,44 +0,0 @@
-// Part of the Carbon Language project, under the Apache License v2.0 with LLVM
-// Exceptions. See /LICENSE for license information.
-// SPDX-License-Identifier: Apache-2.0 WITH LLVM-exception
-//
-// AUTOUPDATE
-// RUN: %{not} %{explorer-run}
-// RUN: %{not} %{explorer-run-trace}
-
-package ExplorerTest api;
-
-interface A {
-  let TA:! type;
-  fn FA() -> TA;
-}
-interface B {
-  let TB:! type;
-  fn FB() -> TB;
-}
-
-class C(T:! type) {
-  impl as A & B where .TA = i32 and .TB = i32 {
-    fn FA() -> i32 {
-      // OK, know that TA is i32 here.
-      let v: Self.(A.TA) = 1;
-      let w: i32 = v;
-      return w;
-    }
-    fn FB() -> i32 {
-      // OK, know that TB is i32 here.
-      let v: Self.(B.TB) = 2;
-      // Don't know that TA is i32; it could be specialized.
-      // TODO: We should not accept this.
-      let w: Self.(A.TA) = 3;
-      return v + w;
-    }
-  }
-}
-
-external impl C(i32) as B where .TB == () {
-  fn FB() -> () { return (); }
-// CHECK:STDERR: COMPILATION ERROR: {{.*}}/explorer/testdata/assoc_const/fail_multi_impl_scoping.carbon:[[@LINE+1]]: ambiguous implementations of interface B for class C(T = i32)
-}
-
-fn Main() -> i32 { return C(i32).FB(); }

+ 0 - 28
explorer/testdata/impl/fail_ambiguous_impl_generic.carbon

@@ -1,28 +0,0 @@
-// Part of the Carbon Language project, under the Apache License v2.0 with LLVM
-// Exceptions. See /LICENSE for license information.
-// SPDX-License-Identifier: Apache-2.0 WITH LLVM-exception
-//
-// AUTOUPDATE
-// RUN: %{not} %{explorer-run}
-// RUN: %{not} %{explorer-run-trace}
-
-package ExplorerTest api;
-
-class A(T:! type) {}
-interface B(T:! type) {}
-
-external impl forall [T:! type] A(T) as B(i32) {}
-external impl forall [T:! type] A(i32) as B(T) {}
-
-fn F[T:! B(i32)](x: T) {}
-fn G[T:! B(bool)](x: T) {}
-
-fn Main() -> i32 {
-  let a: A(bool) = {};
-  let b: A(i32) = {};
-  F(a);
-  G(b);
-  // CHECK:STDERR: COMPILATION ERROR: {{.*}}/explorer/testdata/impl/fail_ambiguous_impl_generic.carbon:[[@LINE+1]]: ambiguous implementations of interface B(T = i32) for class A(T = i32)
-  F(b);
-  return 0;
-}

+ 0 - 36
explorer/testdata/impl/fail_ambiguous_impl_match_first.carbon

@@ -1,36 +0,0 @@
-// Part of the Carbon Language project, under the Apache License v2.0 with LLVM
-// Exceptions. See /LICENSE for license information.
-// SPDX-License-Identifier: Apache-2.0 WITH LLVM-exception
-//
-// AUTOUPDATE
-// RUN: %{not} %{explorer-run}
-// RUN: %{not} %{explorer-run-trace}
-
-package ExplorerTest api;
-
-class A(T:! type) {}
-interface B(T:! type) {}
-
-// This match_first doesn't affect the ambiguity between
-// these impls and the one below.
-// TODO: Once we order by type structure, the second impl
-// in this block should win.
-__match_first {
-  external impl A(i32) as B(i32) {}
-  external impl forall [T:! type] A(i32) as B(T) {}
-}
-
-external impl forall [T:! type] A(T) as B(i32) {}
-
-fn F[T:! B(i32)](x: T) {}
-fn G[T:! B(bool)](x: T) {}
-
-fn Main() -> i32 {
-  let a: A(bool) = {};
-  let b: A(i32) = {};
-  F(a);
-  G(b);
-  // CHECK:STDERR: COMPILATION ERROR: {{.*}}/explorer/testdata/impl/fail_ambiguous_impl_match_first.carbon:[[@LINE+1]]: ambiguous implementations of interface B(T = i32) for class A(T = i32)
-  F(b);
-  return 0;
-}

+ 1 - 1
explorer/testdata/impl/fail_ambiguous_impl.carbon → explorer/testdata/impl/fail_impl_redefinition.carbon

@@ -35,7 +35,7 @@ external impl Point as Vector {
   fn Scale[self: Point](v: i32) -> Point {
       return {.x = self.x * v, .y = self.y * v};
   }
-// CHECK:STDERR: COMPILATION ERROR: {{.*}}/explorer/testdata/impl/fail_ambiguous_impl.carbon:[[@LINE+1]]: ambiguous implementations of interface Vector for class Point
+// CHECK:STDERR: COMPILATION ERROR: {{.*}}/explorer/testdata/impl/fail_impl_redefinition.carbon:[[@LINE+1]]: ambiguous implementations of interface Vector for class Point
 }
 
 fn AddAndScaleGeneric[T:! Vector](a: T, b: T, s: i32) -> T {

+ 0 - 27
explorer/testdata/impl_match/fail_self_recurse_with_match_first.carbon

@@ -1,27 +0,0 @@
-// Part of the Carbon Language project, under the Apache License v2.0 with LLVM
-// Exceptions. See /LICENSE for license information.
-// SPDX-License-Identifier: Apache-2.0 WITH LLVM-exception
-//
-// AUTOUPDATE
-// RUN: %{not} %{explorer-run}
-// RUN: %{not} %{explorer-run-trace}
-
-package ExplorerTest api;
-
-interface Foo {}
-
-__match_first {
-  impl i32* as Foo {}
-  impl forall [T:! type where .Self* is Foo] T as Foo {}
-}
-
-fn F[T:! Foo](x: T) {}
-
-fn Main() -> i32 {
-  // TODO: We should decide whether we want to accept this.
-  // CHECK:STDERR: COMPILATION ERROR: {{.*}}/explorer/testdata/impl_match/fail_self_recurse_with_match_first.carbon:[[@LINE+3]]: impl matching recursively performed a more complex match using the same impl
-  // CHECK:STDERR:   outer match: i32 as interface Foo
-  // CHECK:STDERR:   inner match: i32* as interface Foo
-  F(0);
-  return 0;
-}

+ 5 - 2
explorer/testdata/impl_match/fail_self_recurse_with_match.carbon → explorer/testdata/impl_match/fail_self_recurse_without_match_first.carbon

@@ -10,13 +10,16 @@ package ExplorerTest api;
 
 interface Foo {}
 
-impl i32* as Foo {}
+impl forall [T:! type where .Self == i32*] T as Foo {}
 impl forall [T:! type where .Self* is Foo] T as Foo {}
 
 fn F[T:! Foo](x: T) {}
 
 fn Main() -> i32 {
-  // CHECK:STDERR: COMPILATION ERROR: {{.*}}/explorer/testdata/impl_match/fail_self_recurse_with_match.carbon:[[@LINE+3]]: impl matching recursively performed a more complex match using the same impl
+  // When checking the second impl, we recursively look for a match for
+  // i32* as Foo, which considers the second impl again, even though the first
+  // impl would match.
+  // CHECK:STDERR: COMPILATION ERROR: {{.*}}/explorer/testdata/impl_match/fail_self_recurse_without_match_first.carbon:[[@LINE+3]]: impl matching recursively performed a more complex match using the same impl
   // CHECK:STDERR:   outer match: i32 as interface Foo
   // CHECK:STDERR:   inner match: i32* as interface Foo
   F(0);

+ 0 - 0
explorer/testdata/impl/match_first.carbon → explorer/testdata/impl_match/match_first.carbon


+ 24 - 0
explorer/testdata/impl_match/self_recurse_with_match.carbon

@@ -0,0 +1,24 @@
+// Part of the Carbon Language project, under the Apache License v2.0 with LLVM
+// Exceptions. See /LICENSE for license information.
+// SPDX-License-Identifier: Apache-2.0 WITH LLVM-exception
+//
+// AUTOUPDATE
+// RUN: %{explorer-run}
+// RUN: %{explorer-run-trace}
+// CHECK:STDOUT: result: 0
+
+package ExplorerTest api;
+
+interface Foo {}
+
+impl i32* as Foo {}
+impl forall [T:! type where .Self* is Foo] T as Foo {}
+
+fn F[T:! Foo](x: T) {}
+
+fn Main() -> i32 {
+  // This is OK: we don't consider the second impl when recursively checking
+  // for `impl i32* as Foo` because the first impl is a better match.
+  F(0);
+  return 0;
+}

+ 26 - 0
explorer/testdata/impl_match/self_recurse_with_match_first.carbon

@@ -0,0 +1,26 @@
+// Part of the Carbon Language project, under the Apache License v2.0 with LLVM
+// Exceptions. See /LICENSE for license information.
+// SPDX-License-Identifier: Apache-2.0 WITH LLVM-exception
+//
+// AUTOUPDATE
+// RUN: %{explorer-run}
+// RUN: %{explorer-run-trace}
+// CHECK:STDOUT: result: 0
+
+package ExplorerTest api;
+
+interface Foo {}
+
+__match_first {
+  impl forall [T:! type where .Self == i32*] T as Foo {}
+  impl forall [T:! type where .Self* is Foo] T as Foo {}
+}
+
+fn F[T:! Foo](x: T) {}
+
+fn Main() -> i32 {
+  // This is OK: we don't recursively consider the second `impl`, because the
+  // first one matches.
+  F(0);
+  return 0;
+}

+ 3 - 3
explorer/testdata/impl/fail_unambiguous_impl_generic.carbon → explorer/testdata/impl_match/specialization.carbon

@@ -3,8 +3,9 @@
 // SPDX-License-Identifier: Apache-2.0 WITH LLVM-exception
 //
 // AUTOUPDATE
-// RUN: %{not} %{explorer-run}
-// RUN: %{not} %{explorer-run-trace}
+// RUN: %{explorer-run}
+// RUN: %{explorer-run-trace}
+// CHECK:STDOUT: result: 0
 
 package ExplorerTest api;
 
@@ -44,7 +45,6 @@ fn Main() -> i32 {
   var a: Point(i32) = {.x = 1, .y = 1};
   var b: Point(i32) = {.x = 2, .y = 3};
   // TODO: This shouldn't be considered ambiguous.
-  // CHECK:STDERR: COMPILATION ERROR: {{.*}}/explorer/testdata/impl/fail_unambiguous_impl_generic.carbon:[[@LINE+1]]: ambiguous implementations of interface Vector(T = i32) for class Point(T = i32)
   var p: Point(i32) = AddAndScaleGeneric(a, b, 5);
   return p.x - 15;
 }

+ 57 - 0
explorer/testdata/impl_match/type_structure_bindings.carbon

@@ -0,0 +1,57 @@
+// Part of the Carbon Language project, under the Apache License v2.0 with LLVM
+// Exceptions. See /LICENSE for license information.
+// SPDX-License-Identifier: Apache-2.0 WITH LLVM-exception
+//
+// AUTOUPDATE
+// RUN: %{explorer-run}
+// RUN: %{explorer-run-trace}
+// CHECK:STDOUT: 1
+// CHECK:STDOUT: 2
+// CHECK:STDOUT: 3
+// CHECK:STDOUT: 4
+// CHECK:STDOUT: 4
+// CHECK:STDOUT: 6
+// CHECK:STDOUT: 5
+// CHECK:STDOUT: 6
+// CHECK:STDOUT: result: 0
+
+package ExplorerTest api;
+
+class A(T:! type, U:! type) {}
+interface B(T:! type, U:! type) { fn F(); }
+
+external impl forall [T:! type] A(T, i32) as B(i32, i32) {
+  fn F() { Print("1"); }
+}
+external impl forall [T:! type] A(i32, T) as B(i32, i32) {
+  fn F() { Print("2"); }
+}
+// Intentionally out of order so that explorer can't get the right answer by
+// chance, by just picking the first or last matching impl.
+external impl forall [T:! type] A(i32, i32) as B(i32, T) {
+  fn F() { Print("4"); }
+}
+external impl forall [T:! type] A(i32, i32) as B(T, i32) {
+  fn F() { Print("3"); }
+}
+
+external impl forall [T:! type, U:! type] A(T, i32*) as B(i32*, U) {
+  fn F() { Print("5"); }
+}
+external impl forall [T:! type, U:! type] A(i32*, T) as B(U, i32*) {
+  fn F() { Print("6"); }
+}
+
+fn Main() -> i32 {
+  A((), i32).(B(i32, i32).F)();
+  A(i32, ()).(B(i32, i32).F)();
+  A(i32, i32).(B((), i32).F)();
+  A(i32, i32).(B(i32, ()).F)();
+
+  A(i32, i32).(B(i32, i32).F)();
+
+  A(i32*, i32*).(B(i32, i32*).F)();
+  A(i32*, i32*).(B(i32*, i32).F)();
+  A(i32*, i32*).(B(i32*, i32*).F)();
+  return 0;
+}

+ 46 - 0
explorer/testdata/impl_match/type_structure_order.carbon

@@ -0,0 +1,46 @@
+// Part of the Carbon Language project, under the Apache License v2.0 with LLVM
+// Exceptions. See /LICENSE for license information.
+// SPDX-License-Identifier: Apache-2.0 WITH LLVM-exception
+//
+// AUTOUPDATE
+// RUN: %{explorer-run}
+// RUN: %{explorer-run-trace}
+// CHECK:STDOUT: A(T) as B(i32)
+// CHECK:STDOUT: A(T) as B(U)
+// CHECK:STDOUT: A(i32) as B(T)
+// CHECK:STDOUT: A(i32) as B(T)
+// CHECK:STDOUT: result: 0
+
+package ExplorerTest api;
+
+class A(T:! type) {}
+interface B(T:! type) {
+  fn F();
+}
+
+external impl forall [T:! type] A(T) as B(i32) {
+  fn F() { Print("A(T) as B(i32)"); }
+}
+external impl forall [T:! type] A(i32) as B(T) {
+  fn F() { Print("A(i32) as B(T)"); }
+}
+external impl forall [T:! type, U:! type] A(T) as B(U) {
+  fn F() { Print("A(T) as B(U)"); }
+}
+
+fn F[T:! B(i32)](x: T) {
+  T.F();
+}
+fn G[T:! B(bool)](x: T) {
+  T.F();
+}
+
+fn Main() -> i32 {
+  let a: A(bool) = {};
+  let b: A(i32) = {};
+  F(a);
+  G(a);
+  F(b);
+  G(b);
+  return 0;
+}

+ 57 - 0
explorer/testdata/impl_match/type_structure_vs_match_first.carbon

@@ -0,0 +1,57 @@
+// Part of the Carbon Language project, under the Apache License v2.0 with LLVM
+// Exceptions. See /LICENSE for license information.
+// SPDX-License-Identifier: Apache-2.0 WITH LLVM-exception
+//
+// AUTOUPDATE
+// RUN: %{explorer-run}
+// RUN: %{explorer-run-trace}
+// CHECK:STDOUT: A(T) as B(i32)
+// CHECK:STDOUT: A(T) as B(U)
+// CHECK:STDOUT: A(i32) as B(T)
+// CHECK:STDOUT: A(i32) as B(T)
+// CHECK:STDOUT: result: 0
+
+package ExplorerTest api;
+
+class A(T:! type) {}
+interface B(T:! type) {
+  fn F();
+}
+
+// Note: this match_first has no effect, because it only orders impls with the
+// same type structure.
+__match_first {
+  external impl forall [T:! type] A(T) as B(i32) {
+    fn F() {
+      Print("A(T) as B(i32)");
+    }
+  }
+  external impl forall [T:! type] A(i32) as B(T) {
+    fn F() {
+      Print("A(i32) as B(T)");
+    }
+  }
+}
+
+external impl forall [T:! type, U:! type] A(T) as B(U) {
+  fn F() {
+    Print("A(T) as B(U)");
+  }
+}
+
+fn F[T:! B(i32)](x: T) {
+  T.F();
+}
+fn G[T:! B(bool)](x: T) {
+  T.F();
+}
+
+fn Main() -> i32 {
+  let a: A(bool) = {};
+  let b: A(i32) = {};
+  F(a);
+  G(a);
+  F(b);
+  G(b);
+  return 0;
+}