Clarify and fix diagnostic for missing `Self` in a require declaration (#6616)

If `Self` is not in the self type, then it must be an argument to every
interface required by the declaration. Specifically, this means the
interfaces in the identified facet type, and does not matter if `Self`
appears in the arguments of named constraints.

Fix the diagnostic to stop saying "constraint" incorrectly. And improve
clarity by including in the diagnostic which interface it found without
`Self` as an argument, since it may be found in some other named
constraint, rather than directly in the facet type as written.

toolchain/check/handle_require.cpp

@@ -107,7 +107,7 @@ auto HandleParseNode(Context& context, Parse::RequireTypeImplsId node_id)
 }
 
 static auto TypeStructureReferencesSelf(
-    Context& context, SemIR::TypeInstId inst_id,
+    Context& context, SemIR::LocId loc_id, SemIR::TypeInstId inst_id,
     const SemIR::IdentifiedFacetType& identified_facet_type) -> bool {
   auto find_self = [&](SemIR::TypeIterator& type_iter) -> bool {
     while (true) {
@@ -143,18 +143,35 @@ static auto TypeStructureReferencesSelf(
   }
 
   if (identified_facet_type.required_impls().empty()) {
+    CARBON_DIAGNOSTIC(
+        RequireImplsMissingSelfEmptyFacetType, Error,
+        "no `Self` reference found in `require` declaration; `Self` must "
+        "appear in the self-type or as a generic argument for each required "
+        "interface, but no interfaces were found");
+    context.emitter().Emit(loc_id, RequireImplsMissingSelfEmptyFacetType);
     return false;
   }
 
+  bool interfaces_all_reference_self = true;
   for (auto [_, specific_interface] : identified_facet_type.required_impls()) {
     SemIR::TypeIterator type_iter(&context.sem_ir());
     type_iter.Add(specific_interface);
     if (!find_self(type_iter)) {
-      return false;
+      // TODO: The IdentifiedFacetType loses the location (since it's
+      // canonical), but it would be nice to somehow point this diagnostic at
+      // the particular interface in the facet type that is missing `Self`.
+      CARBON_DIAGNOSTIC(
+          RequireImplsMissingSelf, Error,
+          "no `Self` reference found in `require` declaration; `Self` must "
+          "appear in the self-type or as a generic argument for each required "
+          "interface, but found interface `{0}` without a `Self` argument",
+          SemIR::SpecificInterface);
+      context.emitter().Emit(loc_id, RequireImplsMissingSelf,
+                             specific_interface);
+      interfaces_all_reference_self = false;
     }
   }
-
-  return true;
+  return interfaces_all_reference_self;
 }
 
 struct ValidateRequireResult {
@@ -215,12 +232,7 @@ static auto ValidateRequire(Context& context, SemIR::LocId loc_id,
   const auto& identified =
       context.identified_facet_types().Get(identified_facet_type_id);
 
-  if (!TypeStructureReferencesSelf(context, self_inst_id, identified)) {
-    CARBON_DIAGNOSTIC(RequireImplsMissingSelf, Error,
-                      "no `Self` reference found in `require` declaration; "
-                      "`Self` must appear in the self-type or as a generic "
-                      "parameter for each `interface` or `constraint`");
-    context.emitter().Emit(loc_id, RequireImplsMissingSelf);
+  if (!TypeStructureReferencesSelf(context, loc_id, self_inst_id, identified)) {
     return std::nullopt;
   }
 

toolchain/check/testdata/interface/require.carbon

@@ -175,7 +175,7 @@ interface Z(T:! type) {
   // it would appear in the type structure used for impl lookup (so inside a
   // `where` does not count). But they don't.
   //
-  // CHECK:STDERR: fail_require_impls_without_self.carbon:[[@LINE+4]]:3: error: no `Self` reference found in `require` declaration; `Self` must appear in the self-type or as a generic parameter for each `interface` or `constraint` [RequireImplsMissingSelf]
+  // CHECK:STDERR: fail_require_impls_without_self.carbon:[[@LINE+4]]:3: error: no `Self` reference found in `require` declaration; `Self` must appear in the self-type or as a generic argument for each required interface, but found interface `Y` without a `Self` argument [RequireImplsMissingSelf]
   // CHECK:STDERR:   require T impls Y where .Y1 = Self;
   // CHECK:STDERR:   ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
   // CHECK:STDERR:
@@ -190,13 +190,32 @@ interface Y(T:! type) {}
 interface Z(T:! type) {
   // Self is in one interface but not the other.
   //
-  // CHECK:STDERR: fail_require_impls_without_self_in_one_interface.carbon:[[@LINE+4]]:3: error: no `Self` reference found in `require` declaration; `Self` must appear in the self-type or as a generic parameter for each `interface` or `constraint` [RequireImplsMissingSelf]
+  // CHECK:STDERR: fail_require_impls_without_self_in_one_interface.carbon:[[@LINE+4]]:3: error: no `Self` reference found in `require` declaration; `Self` must appear in the self-type or as a generic argument for each required interface, but found interface `Y({})` without a `Self` argument [RequireImplsMissingSelf]
   // CHECK:STDERR:   require T impls Y(Self) & Y({});
   // CHECK:STDERR:   ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
   // CHECK:STDERR:
   require T impls Y(Self) & Y({});
 }
 
+// --- fail_require_impls_without_self_in_multiple_interfaces.carbon
+library "[[@TEST_NAME]]";
+
+interface Y(T:! type) {}
+
+interface Z(T:! type) {
+  // Self does not appear in either interface, we should get an error for each.
+  //
+  // CHECK:STDERR: fail_require_impls_without_self_in_multiple_interfaces.carbon:[[@LINE+8]]:3: error: no `Self` reference found in `require` declaration; `Self` must appear in the self-type or as a generic argument for each required interface, but found interface `Y(())` without a `Self` argument [RequireImplsMissingSelf]
+  // CHECK:STDERR:   require T impls Y(()) & Y({});
+  // CHECK:STDERR:   ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
+  // CHECK:STDERR:
+  // CHECK:STDERR: fail_require_impls_without_self_in_multiple_interfaces.carbon:[[@LINE+4]]:3: error: no `Self` reference found in `require` declaration; `Self` must appear in the self-type or as a generic argument for each required interface, but found interface `Y({})` without a `Self` argument [RequireImplsMissingSelf]
+  // CHECK:STDERR:   require T impls Y(()) & Y({});
+  // CHECK:STDERR:   ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
+  // CHECK:STDERR:
+  require T impls Y(()) & Y({});
+}
+
 // --- fail_self_impls_self.carbon
 library "[[@TEST_NAME]]";
 

toolchain/check/testdata/named_constraint/require.carbon

@@ -195,7 +195,7 @@ constraint Z(T:! type) {
   // it would appear in the type structure used for impl lookup (so inside a
   // `where` does not count). But they don't.
   //
-  // CHECK:STDERR: fail_require_impls_without_self.carbon:[[@LINE+4]]:3: error: no `Self` reference found in `require` declaration; `Self` must appear in the self-type or as a generic parameter for each `interface` or `constraint` [RequireImplsMissingSelf]
+  // CHECK:STDERR: fail_require_impls_without_self.carbon:[[@LINE+4]]:3: error: no `Self` reference found in `require` declaration; `Self` must appear in the self-type or as a generic argument for each required interface, but found interface `Y` without a `Self` argument [RequireImplsMissingSelf]
   // CHECK:STDERR:   require T impls Y where .Y1 = Self;
   // CHECK:STDERR:   ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
   // CHECK:STDERR:
@@ -211,13 +211,32 @@ interface Y(T:! type) {}
 constraint Z(T:! type) {
   // Self is in one interface but not the other.
   //
-  // CHECK:STDERR: fail_require_impls_without_self_in_one_interface.carbon:[[@LINE+4]]:3: error: no `Self` reference found in `require` declaration; `Self` must appear in the self-type or as a generic parameter for each `interface` or `constraint` [RequireImplsMissingSelf]
+  // CHECK:STDERR: fail_require_impls_without_self_in_one_interface.carbon:[[@LINE+4]]:3: error: no `Self` reference found in `require` declaration; `Self` must appear in the self-type or as a generic argument for each required interface, but found interface `Y({})` without a `Self` argument [RequireImplsMissingSelf]
   // CHECK:STDERR:   require T impls Y(Self) & Y({});
   // CHECK:STDERR:   ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
   // CHECK:STDERR:
   require T impls Y(Self) & Y({});
 }
 
+// --- fail_require_impls_without_self_in_multiple_interfaces.carbon
+library "[[@TEST_NAME]]";
+
+interface Y(T:! type) {}
+
+constraint Z(T:! type) {
+  // Self does not appear in either interface, we should get an error for each.
+  //
+  // CHECK:STDERR: fail_require_impls_without_self_in_multiple_interfaces.carbon:[[@LINE+8]]:3: error: no `Self` reference found in `require` declaration; `Self` must appear in the self-type or as a generic argument for each required interface, but found interface `Y(())` without a `Self` argument [RequireImplsMissingSelf]
+  // CHECK:STDERR:   require T impls Y(()) & Y({});
+  // CHECK:STDERR:   ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
+  // CHECK:STDERR:
+  // CHECK:STDERR: fail_require_impls_without_self_in_multiple_interfaces.carbon:[[@LINE+4]]:3: error: no `Self` reference found in `require` declaration; `Self` must appear in the self-type or as a generic argument for each required interface, but found interface `Y({})` without a `Self` argument [RequireImplsMissingSelf]
+  // CHECK:STDERR:   require T impls Y(()) & Y({});
+  // CHECK:STDERR:   ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
+  // CHECK:STDERR:
+  require T impls Y(()) & Y({});
+}
+
 // --- fail_self_impls_self.carbon
 library "[[@TEST_NAME]]";
 
@@ -358,6 +377,61 @@ fn F() {
   () as N;
 }
 
+// --- require_different_type_impls_nested_constraint.carbon
+library "[[@TEST_NAME]]";
+
+interface Z(T:! type) {}
+
+constraint M(T:! type) {
+  extend require impls Z(T);
+}
+
+constraint N {
+  require {} impls M(Self);
+}
+
+impl {} as Z(()) {}
+
+fn F() {
+  () as N;
+}
+
+// --- fail_require_different_type_impls_nested_constraint_without_reference_to_outer_self.carbon
+library "[[@TEST_NAME]]";
+
+interface Z(T:! type) {}
+
+constraint M(T:! type) {
+  extend require impls Z(Self);
+}
+
+constraint N {
+  // `M(T)` does not make use of the `T`, which is assigned `Self`, so we end up
+  // without any interface specific referencing the `Self` of `N`.
+  //
+  // CHECK:STDERR: fail_require_different_type_impls_nested_constraint_without_reference_to_outer_self.carbon:[[@LINE+4]]:3: error: no `Self` reference found in `require` declaration; `Self` must appear in the self-type or as a generic argument for each required interface, but found interface `Z({})` without a `Self` argument [RequireImplsMissingSelf]
+  // CHECK:STDERR:   require {} impls M(Self);
+  // CHECK:STDERR:   ^~~~~~~~~~~~~~~~~~~~~~~~~
+  // CHECK:STDERR:
+  require {} impls M(Self);
+}
+
+// --- fail_require_different_type_impls_nested_constraint_empty.carbon
+library "[[@TEST_NAME]]";
+
+constraint M(T:! type) {}
+
+constraint N {
+  // `M(T)` does not make use of the `T`, which is assigned `Self`, so we end up
+  // without any interface specific referencing the `Self` of `N`.
+  //
+  // CHECK:STDERR: fail_require_different_type_impls_nested_constraint_empty.carbon:[[@LINE+4]]:3: error: no `Self` reference found in `require` declaration; `Self` must appear in the self-type or as a generic argument for each required interface, but no interfaces were found [RequireImplsMissingSelfEmptyFacetType]
+  // CHECK:STDERR:   require {} impls M(Self);
+  // CHECK:STDERR:   ^~~~~~~~~~~~~~~~~~~~~~~~~
+  // CHECK:STDERR:
+  require {} impls M(Self);
+}
+
 // --- fail_require_different_type_impls_different_parameter.carbon
 library "[[@TEST_NAME]]";
 

toolchain/diagnostics/diagnostic_kind.def

@@ -366,6 +366,7 @@ CARBON_DIAGNOSTIC_KIND(ImplLookupInUnidentifiedFacetType)
 CARBON_DIAGNOSTIC_KIND(RequireImplsExtendWithExplicitSelf)
 CARBON_DIAGNOSTIC_KIND(RequireImplsMissingFacetType)
 CARBON_DIAGNOSTIC_KIND(RequireImplsMissingSelf)
+CARBON_DIAGNOSTIC_KIND(RequireImplsMissingSelfEmptyFacetType)
 CARBON_DIAGNOSTIC_KIND(RequireImplsIncompleteFacetType)
 CARBON_DIAGNOSTIC_KIND(RequireImplsUnidentifiedFacetType)
 CARBON_DIAGNOSTIC_KIND(RequireImplsNotImplemented)