Diagnose unidentified type-of-self in impl lookup query (#6769)

The type of the query self is looked into for a witness, but that type
may be unable to be identified. For example when the query is against
`Self` inside the declaration of a named constraint. Before this PR, we
would crash when identification failed. Now we produce a diagnostic.

This makes `RequireIdentifiedFacetType` take a `ContextScope` callback
(like it used to with an `AnnotationScope` callback) since all callers
now expect to handle diagnostics, and can provide useful context.

This is a followup to #6761.

toolchain/check/handle_require.cpp

@@ -209,21 +209,16 @@ static auto ValidateRequire(Context& context, SemIR::LocId loc_id,
     return std::nullopt;
   }
 
-  auto identified_facet_type_id = SemIR::IdentifiedFacetTypeId::None;
-  {
-    Diagnostics::ContextScope diagnostic_context(
-        &context.emitter(), [&](auto& builder) {
-          CARBON_DIAGNOSTIC(
-              RequireImplsUnidentifiedFacetType, Context,
-              "facet type {0} cannot be identified in `require` declaration",
-              InstIdAsType);
-          builder.Context(constraint_inst_id, RequireImplsUnidentifiedFacetType,
-                          constraint_inst_id);
-        });
-    identified_facet_type_id = RequireIdentifiedFacetType(
-        context, SemIR::LocId(constraint_inst_id), self_constant_value_id,
-        *constraint_facet_type);
-  }
+  auto identified_facet_type_id = RequireIdentifiedFacetType(
+      context, SemIR::LocId(constraint_inst_id), self_constant_value_id,
+      *constraint_facet_type, [&](auto& builder) {
+        CARBON_DIAGNOSTIC(
+            RequireImplsUnidentifiedFacetType, Context,
+            "facet type {0} cannot be identified in `require` declaration",
+            InstIdAsType);
+        builder.Context(constraint_inst_id, RequireImplsUnidentifiedFacetType,
+                        constraint_inst_id);
+      });
   if (!identified_facet_type_id.has_value()) {
     // The constraint can't be used. A diagnostic was emitted by
     // RequireIdentifiedFacetType().

toolchain/check/impl.cpp

@@ -795,20 +795,15 @@ auto CheckConstraintIsInterface(Context& context, SemIR::InstId impl_decl_id,
     return SemIR::SpecificInterface::None;
   }
 
-  auto identified_id = SemIR::IdentifiedFacetTypeId::None;
-  {
-    Diagnostics::ContextScope diagnostic_context(
-        &context.emitter(), [&](auto& builder) {
-          CARBON_DIAGNOSTIC(ImplOfUnidentifiedFacetType, Context,
-                            "facet type {0} cannot be identified in `impl as`",
-                            InstIdAsType);
-          builder.Context(impl_decl_id, ImplOfUnidentifiedFacetType,
-                          constraint_id);
-        });
-    identified_id = RequireIdentifiedFacetType(
-        context, SemIR::LocId(constraint_id),
-        context.constant_values().Get(self_id), *facet_type);
-  }
+  auto identified_id = RequireIdentifiedFacetType(
+      context, SemIR::LocId(constraint_id),
+      context.constant_values().Get(self_id), *facet_type, [&](auto& builder) {
+        CARBON_DIAGNOSTIC(ImplOfUnidentifiedFacetType, Context,
+                          "facet type {0} cannot be identified in `impl as`",
+                          InstIdAsType);
+        builder.Context(impl_decl_id, ImplOfUnidentifiedFacetType,
+                        constraint_id);
+      });
   if (!identified_id.has_value()) {
     return SemIR::SpecificInterface::None;
   }

toolchain/check/impl_lookup.cpp

@@ -218,15 +218,14 @@ static auto GetRequiredImplsFromConstraint(
   const auto& facet_type_info =
       context.facet_types().Get(facet_type_inst.facet_type_id);
 
-  Diagnostics::ContextScope diagnostic_context(
-      &context.emitter(), [&](auto& builder) {
+  auto identified_id = RequireIdentifiedFacetType(
+      context, loc_id, query_self_const_id, facet_type_inst,
+      [&](auto& builder) {
         CARBON_DIAGNOSTIC(ImplLookupInUnidentifiedFacetType, Context,
                           "facet type {0} can not be identified", InstIdAsType);
         builder.Context(loc_id, ImplLookupInUnidentifiedFacetType,
                         facet_type_inst_id);
       });
-  auto identified_id = RequireIdentifiedFacetType(
-      context, loc_id, query_self_const_id, facet_type_inst);
   if (!identified_id.has_value()) {
     return std::nullopt;
   }
@@ -347,22 +346,22 @@ static auto LookupImplWitnessInSelfFacetValue(
   auto self_facet_value_const_id =
       context.constant_values().Get(self_facet_value_inst_id);
 
-  // TODO: Add a diagnostic context for when identification fails, instead of
-  // CHECKing, such as for the type of Self inside a named constraint
-  // definition.
-  static_cast<void>(loc_id);
-
   // The position of the interface in `required_impls()` is also the
   // position of the witness for that interface in `FacetValue`. The
   // `FacetValue` witnesses are the output of an impl lookup, which finds and
   // returns witnesses in the same order.
   auto identified_id = RequireIdentifiedFacetType(
-      context, loc_id, self_facet_value_const_id, *facet_type);
-  // This should not be possible as FacetValue is constructed by a conversion
-  // to a facet type, which performs impl lookup for that facet type, and
-  // lookup only succeeds for identified facet types.
-  CARBON_CHECK(identified_id.has_value(),
-               "FacetValue was constructed with an unidentified facet type");
+      context, loc_id, self_facet_value_const_id, *facet_type,
+      [&](auto& builder) {
+        CARBON_DIAGNOSTIC(ImplLookupInUnidentifiedFacetTypeOfQuerySelf, Context,
+                          "facet type of value {0} can not be identified",
+                          InstIdAsType);
+        builder.Context(loc_id, ImplLookupInUnidentifiedFacetTypeOfQuerySelf,
+                        self_facet_value_inst_id);
+      });
+  if (!identified_id.has_value()) {
+    return EvalImplLookupResult::MakeNone();
+  }
   auto facet_type_req_impls = llvm::enumerate(
       context.identified_facet_types().Get(identified_id).required_impls());
   auto it = llvm::find_if(facet_type_req_impls, [&](auto e) {

toolchain/check/import_ref.cpp

@@ -2657,8 +2657,11 @@ static auto TryResolveTypedInst(ImportRefResolver& resolver,
     // GetLocalConstantId gave us an attached constant.
     auto unattached_self_const_id =
         resolver.local_constant_values().GetUnattachedConstant(self_const_id);
-    RequireIdentifiedFacetType(resolver.local_context(), SemIR::LocId::None,
-                               unattached_self_const_id, *facet_type);
+    RequireIdentifiedFacetType(
+        resolver.local_context(), SemIR::LocId::None, unattached_self_const_id,
+        *facet_type, []([[maybe_unused]] auto& builder) {
+          CARBON_FATAL("Imported impl constraint can't be identified");
+        });
   }
   if (import_impl.is_complete()) {
     ImportImplDefinition(resolver, import_impl, new_impl);

toolchain/check/testdata/facet/require_constrains_self.carbon

@@ -0,0 +1,66 @@
+// 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-FILE: toolchain/testing/testdata/min_prelude/convert.carbon
+//
+// AUTOUPDATE
+// TIP: To test this file alone, run:
+// TIP:   bazel test //toolchain/testing:file_test --test_arg=--file_tests=toolchain/check/testdata/facet/require_constrains_self.carbon
+// TIP: To dump output, run:
+// TIP:   bazel run //toolchain/testing:file_test -- --dump_output --file_tests=toolchain/check/testdata/facet/require_constrains_self.carbon
+
+// --- fail_self_doesnt_meet_constraint.carbon
+library "[[@TEST_NAME]]";
+
+interface Y2 {}
+interface Y(T:! Y2) {}
+// TODO: The error should be that Self does not implement Y2, not that it can't
+// be identified. We should allow identifying `Self`.
+//
+// CHECK:STDERR: fail_self_doesnt_meet_constraint.carbon:[[@LINE+17]]:37: error: facet type of value `Self` can not be identified [ImplLookupInUnidentifiedFacetTypeOfQuerySelf]
+// CHECK:STDERR: constraint W { extend require impls Y(Self); }
+// CHECK:STDERR:                                     ^~~~~~~
+// CHECK:STDERR: fail_self_doesnt_meet_constraint.carbon:[[@LINE+14]]:1: note: constraint is currently being defined [NamedConstraintIncompleteWithinDefinition]
+// CHECK:STDERR: constraint W { extend require impls Y(Self); }
+// CHECK:STDERR: ^~~~~~~~~~~~~~
+// CHECK:STDERR: fail_self_doesnt_meet_constraint.carbon:[[@LINE-10]]:13: note: initializing generic parameter `T` declared here [InitializingGenericParam]
+// CHECK:STDERR: interface Y(T:! Y2) {}
+// CHECK:STDERR:             ^
+// CHECK:STDERR:
+// CHECK:STDERR: fail_self_doesnt_meet_constraint.carbon:[[@LINE+7]]:37: error: cannot convert type `Self` that implements `W` into type implementing `Y2` [ConversionFailureFacetToFacet]
+// CHECK:STDERR: constraint W { extend require impls Y(Self); }
+// CHECK:STDERR:                                     ^~~~~~~
+// CHECK:STDERR: fail_self_doesnt_meet_constraint.carbon:[[@LINE-17]]:13: note: initializing generic parameter `T` declared here [InitializingGenericParam]
+// CHECK:STDERR: interface Y(T:! Y2) {}
+// CHECK:STDERR:             ^
+// CHECK:STDERR:
+constraint W { extend require impls Y(Self); }
+
+// --- fail_todo_self_meets_constraint.carbon
+library "[[@TEST_NAME]]";
+
+interface Y2 {}
+interface Y(T:! Y2) {}
+constraint W {
+  // TODO: This should allow the next line to work, as Self should be known to impl Y2.
+  require impls Y2;
+  // CHECK:STDERR: fail_todo_self_meets_constraint.carbon:[[@LINE+17]]:24: error: facet type of value `Self` can not be identified [ImplLookupInUnidentifiedFacetTypeOfQuerySelf]
+  // CHECK:STDERR:   extend require impls Y(Self);
+  // CHECK:STDERR:                        ^~~~~~~
+  // CHECK:STDERR: fail_todo_self_meets_constraint.carbon:[[@LINE-6]]:1: note: constraint is currently being defined [NamedConstraintIncompleteWithinDefinition]
+  // CHECK:STDERR: constraint W {
+  // CHECK:STDERR: ^~~~~~~~~~~~~~
+  // CHECK:STDERR: fail_todo_self_meets_constraint.carbon:[[@LINE-10]]:13: note: initializing generic parameter `T` declared here [InitializingGenericParam]
+  // CHECK:STDERR: interface Y(T:! Y2) {}
+  // CHECK:STDERR:             ^
+  // CHECK:STDERR:
+  // CHECK:STDERR: fail_todo_self_meets_constraint.carbon:[[@LINE+7]]:24: error: cannot convert type `Self` that implements `W` into type implementing `Y2` [ConversionFailureFacetToFacet]
+  // CHECK:STDERR:   extend require impls Y(Self);
+  // CHECK:STDERR:                        ^~~~~~~
+  // CHECK:STDERR: fail_todo_self_meets_constraint.carbon:[[@LINE-17]]:13: note: initializing generic parameter `T` declared here [InitializingGenericParam]
+  // CHECK:STDERR: interface Y(T:! Y2) {}
+  // CHECK:STDERR:             ^
+  // CHECK:STDERR:
+  extend require impls Y(Self);
+}

toolchain/check/testdata/generic/identify_specific_facet_type.carbon

@@ -17,14 +17,17 @@ library "[[@TEST_NAME]]";
 // is invalid, so the impl as a whole is invalid.
 
 interface K(T:! type) {}
-constraint J(N:! i32) {
+constraint L(N:! i32) {
   require impls K(array(i32, N));
 }
+constraint J(N:! i32) {
+  require impls L(N);
+}
 
 // CHECK:STDERR: fail_error_in_identifying_require_facet_type.carbon:[[@LINE+7]]:1: error: facet type `J(-1)` cannot be identified in `impl as` [ImplOfUnidentifiedFacetType]
 // CHECK:STDERR: impl {} as J(-1) {}
 // CHECK:STDERR: ^~~~~~~~~~~~~~~~~~
-// CHECK:STDERR: fail_error_in_identifying_require_facet_type.carbon:[[@LINE-6]]:30: note: array bound of -1 is negative [ArrayBoundNegative]
+// CHECK:STDERR: fail_error_in_identifying_require_facet_type.carbon:[[@LINE-9]]:30: note: array bound of -1 is negative [ArrayBoundNegative]
 // CHECK:STDERR:   require impls K(array(i32, N));
 // CHECK:STDERR:                              ^
 // CHECK:STDERR:

toolchain/check/testdata/named_constraint/incomplete.carbon

@@ -0,0 +1,35 @@
+// 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-FILE: toolchain/testing/testdata/min_prelude/none.carbon
+//
+// AUTOUPDATE
+// TIP: To test this file alone, run:
+// TIP:   bazel test //toolchain/testing:file_test --test_arg=--file_tests=toolchain/check/testdata/named_constraint/incomplete.carbon
+// TIP: To dump output, run:
+// TIP:   bazel run //toolchain/testing:file_test -- --dump_output --file_tests=toolchain/check/testdata/named_constraint/incomplete.carbon
+
+// --- fail_incomplete_constraint_in_impl_lookup.carbon
+
+interface Z {}
+
+constraint N;
+fn F(T:! N) {
+  // TODO: The failed-to-identify error should be a note under the convert error
+  // instead of being a separate error. This can be done by making a top-level
+  // convert error as a Context for specific errors that happen during convert.
+  //
+  // CHECK:STDERR: fail_incomplete_constraint_in_impl_lookup.carbon:[[@LINE+11]]:3: error: facet type of value `T` can not be identified [ImplLookupInUnidentifiedFacetTypeOfQuerySelf]
+  // CHECK:STDERR:   T as Z;
+  // CHECK:STDERR:   ^~~~~~
+  // CHECK:STDERR: fail_incomplete_constraint_in_impl_lookup.carbon:[[@LINE-9]]:1: note: constraint was forward declared here [NamedConstraintForwardDeclaredHere]
+  // CHECK:STDERR: constraint N;
+  // CHECK:STDERR: ^~~~~~~~~~~~~
+  // CHECK:STDERR:
+  // CHECK:STDERR: fail_incomplete_constraint_in_impl_lookup.carbon:[[@LINE+4]]:3: error: cannot convert type `T` that implements `N` into type implementing `Z` [ConversionFailureFacetToFacet]
+  // CHECK:STDERR:   T as Z;
+  // CHECK:STDERR:   ^~~~~~
+  // CHECK:STDERR:
+  T as Z;
+}

toolchain/check/type_completion.cpp

@@ -885,8 +885,12 @@ static auto GetSelfFacetValue(Context& context, SemIR::ConstantId self_const_id)
 
 auto RequireIdentifiedFacetType(Context& context, SemIR::LocId loc_id,
                                 SemIR::ConstantId self_const_id,
-                                const SemIR::FacetType& facet_type)
+                                const SemIR::FacetType& facet_type,
+                                DiagnosticContextFn diagnostic_context)
     -> SemIR::IdentifiedFacetTypeId {
+  CARBON_CHECK(diagnostic_context);
+  Diagnostics::ContextScope scope(&context.emitter(), diagnostic_context);
+
   auto key =
       SemIR::IdentifiedFacetTypeKey{.facet_type_id = facet_type.facet_type_id,
                                     .self_const_id = self_const_id};

toolchain/check/type_completion.h

@@ -73,7 +73,8 @@ auto RequireConcreteType(Context& context, SemIR::TypeId type_id,
 // returns None if any error is found.
 auto RequireIdentifiedFacetType(Context& context, SemIR::LocId loc_id,
                                 SemIR::ConstantId self_const_id,
-                                const SemIR::FacetType& facet_type)
+                                const SemIR::FacetType& facet_type,
+                                DiagnosticContextFn diagnostic_context)
     -> SemIR::IdentifiedFacetTypeId;
 
 // Emits an error diagnostic explaining that a class is incomplete.

toolchain/diagnostics/kind.def

@@ -370,6 +370,7 @@ CARBON_DIAGNOSTIC_KIND(MissingImplInMemberAccessInContext)
 CARBON_DIAGNOSTIC_KIND(ImplLookupCycle)
 CARBON_DIAGNOSTIC_KIND(ImplLookupCycleNote)
 CARBON_DIAGNOSTIC_KIND(ImplLookupInUnidentifiedFacetType)
+CARBON_DIAGNOSTIC_KIND(ImplLookupInUnidentifiedFacetTypeOfQuerySelf)
 
 // Require checking.
 CARBON_DIAGNOSTIC_KIND(RequireImplsExtendWithExplicitSelf)