From bad9beddc79347abb66485877c6610f28987a930 Mon Sep 17 00:00:00 2001 From: Dana Jansens Date: Fri, 13 Mar 2026 12:54:58 -0400 Subject: [PATCH] Diagnose using named constraint's name inside its definition (#6906) Using a named constraint inside itself is problematic: - If there were not require decls written above, it identifies as an empty set. This makes `Z(Self)` essentially disappear in the identified facet type, which produces "no use of Self" diagnostics while the user can see a use of Self in the code. - It won't include require decls that are written after, and so `require T impls Z` won't actually enforce that `T` impls all of `Z`. Previously this was an error because using the named constraint would require it to be identified, and it's not identified until it is complete. But this will change in proposal #6902. So that proposal also includes changes to preserve diagnostics for incorrect use of a named constraint before it's complete, which is implemented here. Discussed in open discussion [on 2026-03-12](https://docs.google.com/document/d/1mjllGO3ZCL4qGt9uJHUtcxKoHAGEY7Y999ie4EtBWB8/edit?tab=t.0#heading=h.1dvbbrp5a6t3). The new tests exposed a bug where we're not copying named constraints in a facet type on the RHS of `where .Self impls` into the facet type on the left, which is now fixed. The `fail_require_impls_incomplete_self_in_period_self_impls.carbon` test would not diagnose its error without this fix. --- toolchain/check/eval.cpp | 4 + toolchain/check/handle_require.cpp | 31 ++++ .../testdata/named_constraint/require.carbon | 153 +++++++----------- toolchain/diagnostics/kind.def | 1 + 4 files changed, 96 insertions(+), 93 deletions(-) diff --git a/toolchain/check/eval.cpp b/toolchain/check/eval.cpp index 8620514567aa..ccf668695f7c 100644 --- a/toolchain/check/eval.cpp +++ b/toolchain/check/eval.cpp @@ -2494,6 +2494,10 @@ auto TryEvalTypedInst(EvalContext& eval_context, more_info.extend_constraints); llvm::append_range(info.self_impls_constraints, more_info.self_impls_constraints); + llvm::append_range(info.self_impls_named_constraints, + more_info.extend_named_constraints); + llvm::append_range(info.self_impls_named_constraints, + more_info.self_impls_named_constraints); // Other requirements are copied in. llvm::append_range(info.rewrite_constraints, more_info.rewrite_constraints); diff --git a/toolchain/check/handle_require.cpp b/toolchain/check/handle_require.cpp index 9e9ef2ada34b..527bd3b3b366 100644 --- a/toolchain/check/handle_require.cpp +++ b/toolchain/check/handle_require.cpp @@ -17,6 +17,7 @@ #include "toolchain/parse/node_ids.h" #include "toolchain/sem_ir/ids.h" #include "toolchain/sem_ir/named_constraint.h" +#include "toolchain/sem_ir/specific_named_constraint.h" #include "toolchain/sem_ir/type_iterator.h" #include "toolchain/sem_ir/typed_insts.h" @@ -209,6 +210,36 @@ static auto ValidateRequire(Context& context, SemIR::LocId loc_id, return std::nullopt; } + if (auto named_constraint = + context.insts().TryGetAs( + scope_inst_id)) { + const auto& constraint_facet_type_info = + context.facet_types().Get(constraint_facet_type->facet_type_id); + // TODO: Handle other impls named constraints for the + // RequireImplsReferenceCycle diagnostic. + if (constraint_facet_type_info.other_requirements) { + context.TODO(constraint_inst_id, + "facet type has constraints that we don't handle yet"); + return std::nullopt; + } + auto named_constraints = llvm::concat( + constraint_facet_type_info.extend_named_constraints, + constraint_facet_type_info.self_impls_named_constraints); + for (auto c : named_constraints) { + if (c.named_constraint_id == named_constraint->named_constraint_id) { + const auto& named_constraint = + context.named_constraints().Get(c.named_constraint_id); + CARBON_DIAGNOSTIC(RequireImplsReferenceCycle, Error, + "facet type in `require` declaration refers to the " + "named constraint `{0}` from within its definition", + SemIR::NameId); + context.emitter().Emit(constraint_inst_id, RequireImplsReferenceCycle, + named_constraint.name_id); + return std::nullopt; + } + } + } + auto identified_facet_type_id = RequireIdentifiedFacetType( context, SemIR::LocId(constraint_inst_id), self_constant_value_id, *constraint_facet_type, [&](auto& builder) { diff --git a/toolchain/check/testdata/named_constraint/require.carbon b/toolchain/check/testdata/named_constraint/require.carbon index 1ca1ef7f1688..e317faa308e3 100644 --- a/toolchain/check/testdata/named_constraint/require.carbon +++ b/toolchain/check/testdata/named_constraint/require.carbon @@ -175,34 +175,82 @@ constraint Z { // --- fail_require_impls_incomplete_self.carbon library "[[@TEST_NAME]]"; -//@dump-sem-ir-begin constraint Z { - // CHECK:STDERR: fail_require_impls_incomplete_self.carbon:[[@LINE+7]]:17: error: facet type `Z` cannot be identified in `require` declaration [RequireImplsUnidentifiedFacetType] + // CHECK:STDERR: fail_require_impls_incomplete_self.carbon:[[@LINE+4]]:17: error: facet type in `require` declaration refers to the named constraint `Z` from within its definition [RequireImplsReferenceCycle] // CHECK:STDERR: require impls Z; // CHECK:STDERR: ^ - // CHECK:STDERR: fail_require_impls_incomplete_self.carbon:[[@LINE-4]]:1: note: constraint is currently being defined [NamedConstraintIncompleteWithinDefinition] - // CHECK:STDERR: constraint Z { - // CHECK:STDERR: ^~~~~~~~~~~~~~ // CHECK:STDERR: require impls Z; } -//@dump-sem-ir-end + +// --- fail_require_impls_incomplete_self_forward_declared.carbon +library "[[@TEST_NAME]]"; + +constraint Z; + +constraint Z { + // CHECK:STDERR: fail_require_impls_incomplete_self_forward_declared.carbon:[[@LINE+4]]:17: error: facet type in `require` declaration refers to the named constraint `Z` from within its definition [RequireImplsReferenceCycle] + // CHECK:STDERR: require impls Z; + // CHECK:STDERR: ^ + // CHECK:STDERR: + require impls Z; +} + +// --- fail_require_impls_incomplete_self_in_period_self_impls.carbon +library "[[@TEST_NAME]]"; + +constraint Z { + // CHECK:STDERR: fail_require_impls_incomplete_self_in_period_self_impls.carbon:[[@LINE+4]]:17: error: facet type in `require` declaration refers to the named constraint `Z` from within its definition [RequireImplsReferenceCycle] + // CHECK:STDERR: require impls type where .Self impls Z; + // CHECK:STDERR: ^~~~~~~~~~~~~~~~~~~~~~~~ + // CHECK:STDERR: + require impls type where .Self impls Z; +} + +// --- fail_require_impls_incomplete_self_in_class_impls.carbon +library "[[@TEST_NAME]]"; + +class C; +constraint Z { + // TODO: This should be a RequireImplsReferenceCycle error since `Z` is being + // used inside `Z`. + // CHECK:STDERR: fail_require_impls_incomplete_self_in_class_impls.carbon:[[@LINE+4]]:17: error: semantics TODO: `facet type has constraints that we don't handle yet` [SemanticsTodo] + // CHECK:STDERR: require impls type where C impls Z; + // CHECK:STDERR: ^~~~~~~~~~~~~~~~~~~~ + // CHECK:STDERR: + require impls type where C impls Z; +} + +// --- fail_require_impls_incomplete_indirect.carbon +library "[[@TEST_NAME]]"; + +constraint Z; + +constraint Y { + // CHECK:STDERR: fail_require_impls_incomplete_indirect.carbon:[[@LINE+7]]:17: error: facet type `Z` cannot be identified in `require` declaration [RequireImplsUnidentifiedFacetType] + // CHECK:STDERR: require impls Z; + // CHECK:STDERR: ^ + // CHECK:STDERR: fail_require_impls_incomplete_indirect.carbon:[[@LINE-6]]:1: note: constraint was forward declared here [NamedConstraintForwardDeclaredHere] + // CHECK:STDERR: constraint Z; + // CHECK:STDERR: ^~~~~~~~~~~~~ + // CHECK:STDERR: + require impls Z; +} + +constraint Z { + require impls Y; +} // --- fail_require_impls_incomplete_self_specific.carbon library "[[@TEST_NAME]]"; -//@dump-sem-ir-begin constraint Z(T:! type) { - // CHECK:STDERR: fail_require_impls_incomplete_self_specific.carbon:[[@LINE+7]]:19: error: facet type `Z(Self)` cannot be identified in `require` declaration [RequireImplsUnidentifiedFacetType] + // CHECK:STDERR: fail_require_impls_incomplete_self_specific.carbon:[[@LINE+4]]:19: error: facet type in `require` declaration refers to the named constraint `Z` from within its definition [RequireImplsReferenceCycle] // CHECK:STDERR: require T impls Z(Self); // CHECK:STDERR: ^~~~~~~ - // CHECK:STDERR: fail_require_impls_incomplete_self_specific.carbon:[[@LINE-4]]:1: note: constraint is currently being defined [NamedConstraintIncompleteWithinDefinition] - // CHECK:STDERR: constraint Z(T:! type) { - // CHECK:STDERR: ^~~~~~~~~~~~~~~~~~~~~~~~ // CHECK:STDERR: require T impls Z(Self); } -//@dump-sem-ir-end // --- fail_require_impls_without_self.carbon library "[[@TEST_NAME]]"; @@ -1064,87 +1112,6 @@ fn F() { // CHECK:STDOUT: // CHECK:STDOUT: specific @Z.WithSelf(constants.%Self) {} // CHECK:STDOUT: -// CHECK:STDOUT: --- fail_require_impls_incomplete_self.carbon -// CHECK:STDOUT: -// CHECK:STDOUT: constants { -// CHECK:STDOUT: %Z.type: type = facet_type <@Z> [concrete] -// CHECK:STDOUT: %Self: %Z.type = symbolic_binding Self, 0 [symbolic] -// CHECK:STDOUT: } -// CHECK:STDOUT: -// CHECK:STDOUT: file { -// CHECK:STDOUT: %Z.decl: type = constraint_decl @Z [concrete = constants.%Z.type] {} {} -// CHECK:STDOUT: } -// CHECK:STDOUT: -// CHECK:STDOUT: constraint @Z { -// CHECK:STDOUT: %Self: %Z.type = symbolic_binding Self, 0 [symbolic = constants.%Self] -// CHECK:STDOUT: %Z.WithSelf.decl = constraint_with_self_decl @Z [concrete] -// CHECK:STDOUT: -// CHECK:STDOUT: !members: -// CHECK:STDOUT: .Self = %Self -// CHECK:STDOUT: .Z = -// CHECK:STDOUT: .Z = -// CHECK:STDOUT: -// CHECK:STDOUT: !requires: -// CHECK:STDOUT: } -// CHECK:STDOUT: -// CHECK:STDOUT: specific @Z.WithSelf(constants.%Self) {} -// CHECK:STDOUT: -// CHECK:STDOUT: --- fail_require_impls_incomplete_self_specific.carbon -// CHECK:STDOUT: -// CHECK:STDOUT: constants { -// CHECK:STDOUT: %T: type = symbolic_binding T, 0 [symbolic] -// CHECK:STDOUT: %pattern_type: type = pattern_type type [concrete] -// CHECK:STDOUT: %Z.type.7a8: type = generic_named_constaint_type @Z [concrete] -// CHECK:STDOUT: %empty_struct: %Z.type.7a8 = struct_value () [concrete] -// CHECK:STDOUT: %Z.type.d682d6.1: type = facet_type <@Z, @Z(%T)> [symbolic] -// CHECK:STDOUT: %Self: %Z.type.d682d6.1 = symbolic_binding Self, 1 [symbolic] -// CHECK:STDOUT: %Self.binding.as_type: type = symbolic_binding_type Self, 1, %Self [symbolic] -// CHECK:STDOUT: } -// CHECK:STDOUT: -// CHECK:STDOUT: file { -// CHECK:STDOUT: %Z.decl: %Z.type.7a8 = constraint_decl @Z [concrete = constants.%empty_struct] { -// CHECK:STDOUT: %T.patt: %pattern_type = symbolic_binding_pattern T, 0 [concrete] -// CHECK:STDOUT: } { -// CHECK:STDOUT: %.loc4_18.1: type = splice_block %.loc4_18.2 [concrete = type] { -// CHECK:STDOUT: -// CHECK:STDOUT: %.loc4_18.2: type = type_literal type [concrete = type] -// CHECK:STDOUT: } -// CHECK:STDOUT: %T.loc4_14.2: type = symbolic_binding T, 0 [symbolic = %T.loc4_14.1 (constants.%T)] -// CHECK:STDOUT: } -// CHECK:STDOUT: } -// CHECK:STDOUT: -// CHECK:STDOUT: generic constraint @Z(%T.loc4_14.2: type) { -// CHECK:STDOUT: %T.loc4_14.1: type = symbolic_binding T, 0 [symbolic = %T.loc4_14.1 (constants.%T)] -// CHECK:STDOUT: -// CHECK:STDOUT: !definition: -// CHECK:STDOUT: %Z.type: type = facet_type <@Z, @Z(%T.loc4_14.1)> [symbolic = %Z.type (constants.%Z.type.d682d6.1)] -// CHECK:STDOUT: %Self.loc4_24.2: @Z.%Z.type (%Z.type.d682d6.1) = symbolic_binding Self, 1 [symbolic = %Self.loc4_24.2 (constants.%Self)] -// CHECK:STDOUT: -// CHECK:STDOUT: constraint { -// CHECK:STDOUT: %Self.loc4_24.1: @Z.%Z.type (%Z.type.d682d6.1) = symbolic_binding Self, 1 [symbolic = %Self.loc4_24.2 (constants.%Self)] -// CHECK:STDOUT: %Z.WithSelf.decl = constraint_with_self_decl @Z [concrete] -// CHECK:STDOUT: -// CHECK:STDOUT: !members: -// CHECK:STDOUT: .Self = %Self.loc4_24.1 -// CHECK:STDOUT: .T = -// CHECK:STDOUT: .Z = -// CHECK:STDOUT: .T = -// CHECK:STDOUT: .Z = -// CHECK:STDOUT: -// CHECK:STDOUT: !requires: -// CHECK:STDOUT: } -// CHECK:STDOUT: } -// CHECK:STDOUT: -// CHECK:STDOUT: specific @Z(constants.%T) { -// CHECK:STDOUT: %T.loc4_14.1 => constants.%T -// CHECK:STDOUT: } -// CHECK:STDOUT: -// CHECK:STDOUT: specific @Z.WithSelf(constants.%T, constants.%Self) {} -// CHECK:STDOUT: -// CHECK:STDOUT: specific @Z(constants.%Self.binding.as_type) { -// CHECK:STDOUT: %T.loc4_14.1 => constants.%Self.binding.as_type -// CHECK:STDOUT: } -// CHECK:STDOUT: // CHECK:STDOUT: --- fail_require_impls_without_self.carbon // CHECK:STDOUT: // CHECK:STDOUT: constants { diff --git a/toolchain/diagnostics/kind.def b/toolchain/diagnostics/kind.def index f63749fe5b50..07a1e7b2dacd 100644 --- a/toolchain/diagnostics/kind.def +++ b/toolchain/diagnostics/kind.def @@ -381,6 +381,7 @@ CARBON_DIAGNOSTIC_KIND(RequireImplsMissingFacetType) CARBON_DIAGNOSTIC_KIND(RequireImplsMissingSelf) CARBON_DIAGNOSTIC_KIND(RequireImplsMissingSelfEmptyFacetType) CARBON_DIAGNOSTIC_KIND(RequireImplsIncompleteFacetType) +CARBON_DIAGNOSTIC_KIND(RequireImplsReferenceCycle) CARBON_DIAGNOSTIC_KIND(RequireImplsUnidentifiedFacetType) CARBON_DIAGNOSTIC_KIND(RequireImplsNotImplemented) CARBON_DIAGNOSTIC_KIND(RequireInWrongScope)