mirror of
https://github.com/carbon-language/carbon-lang.git
synced 2026-10-04 22:02:52 +01:00
Stop allowing impl redeclarations to differ syntactically in where clause (#4850)
Based on [the lastest thinking on #4672](https://github.com/carbon-language/carbon-lang/issues/4672#issuecomment-2606209281) , require a full syntactic match for impl redeclaration, instead of excluding the `where` restriction. This means no updates to the impl witness on redeclaration, and no diagnostics that those updates are consistent. Not included in this PR, but will need to be done in the future: * Support for assigning values to associated constants in the body of the impl definition. This will require moving the checking that non-function associated constants are set from the definition start to definition end. * Identify semantic redeclarations that are not syntactic matches to give a failed redeclaration diagnostic. This should be done once we are already identifying impl declarations with the same type structure in order to require they be identified in an impl_priority/match_first block. * Merging of the functions in `check/impl.cpp` that are now always called together. Also add some test coverage of `where` parsing I developed in PR I've now abandoned because of this new simplification of the impl redeclaration semantics. --------- Co-authored-by: Josh L <josh11b@users.noreply.github.com> Co-authored-by: Jon Ross-Perkins <jperkins@google.com>
This commit is contained in:
co-authored by
Josh L
Jon Ross-Perkins
parent
0d0e202ce8
commit
5abe5a3c21
@@ -214,40 +214,11 @@ static auto PopImplIntroducerAndParamsAsNameComponent(
|
||||
|
||||
Parse::NodeId first_param_node_id =
|
||||
context.node_stack().PopForSoloNodeId<Parse::NodeKind::ImplIntroducer>();
|
||||
|
||||
// Subtracting 1 since we don't want to include the final `{` or `;` of the
|
||||
// declaration when performing syntactic match.
|
||||
auto end_node_kind = context.parse_tree().node_kind(end_of_decl_node_id);
|
||||
CARBON_CHECK(end_node_kind == Parse::NodeKind::ImplDefinitionStart ||
|
||||
end_node_kind == Parse::NodeKind::ImplDecl);
|
||||
Parse::Tree::PostorderIterator last_param_iter(end_of_decl_node_id);
|
||||
--last_param_iter;
|
||||
|
||||
// Following proposal #3763, exclude a final `where` clause, if present. See:
|
||||
// https://github.com/carbon-language/carbon-lang/blob/trunk/proposals/p3763.md#redeclarations
|
||||
|
||||
// Caches the NodeKind for the current value of *last_param_iter so
|
||||
if (context.parse_tree().node_kind(*last_param_iter) ==
|
||||
Parse::NodeKind::WhereExpr) {
|
||||
int where_operands_to_skip = 1;
|
||||
--last_param_iter;
|
||||
CARBON_CHECK(Parse::Tree::PostorderIterator(first_param_node_id) <
|
||||
last_param_iter);
|
||||
do {
|
||||
auto node_kind = context.parse_tree().node_kind(*last_param_iter);
|
||||
if (node_kind == Parse::NodeKind::WhereExpr) {
|
||||
// If we have a nested `where`, we need to see another `WhereOperand`
|
||||
// before we find the one that matches our original `WhereExpr` node.
|
||||
++where_operands_to_skip;
|
||||
} else if (node_kind == Parse::NodeKind::WhereOperand) {
|
||||
--where_operands_to_skip;
|
||||
}
|
||||
--last_param_iter;
|
||||
CARBON_CHECK(Parse::Tree::PostorderIterator(first_param_node_id) <
|
||||
last_param_iter);
|
||||
} while (where_operands_to_skip > 0);
|
||||
}
|
||||
|
||||
return {
|
||||
.name_loc_id = Parse::NodeId::None,
|
||||
.name_id = SemIR::NameId::None,
|
||||
@@ -315,9 +286,6 @@ static auto IsValidImplRedecl(Context& context, SemIR::Impl& new_impl,
|
||||
|
||||
// TODO: Only allow redeclaration in a match_first/impl_priority block.
|
||||
|
||||
// TODO: Merge information from the new declaration into the old one as
|
||||
// needed.
|
||||
|
||||
return true;
|
||||
}
|
||||
|
||||
@@ -375,6 +343,8 @@ static auto BuildImplDecl(Context& context, Parse::AnyImplDeclId node_id,
|
||||
// Add the impl declaration.
|
||||
bool invalid_redeclaration = false;
|
||||
auto lookup_bucket_ref = context.impls().GetOrAddLookupBucket(impl_info);
|
||||
// TODO: Detect two impl declarations with the same self type and interface,
|
||||
// and issue an error if they don't match.
|
||||
for (auto prev_impl_id : lookup_bucket_ref) {
|
||||
if (MergeImplRedecl(context, impl_info, prev_impl_id)) {
|
||||
if (IsValidImplRedecl(context, impl_info, prev_impl_id)) {
|
||||
@@ -392,15 +362,13 @@ static auto BuildImplDecl(Context& context, Parse::AnyImplDeclId node_id,
|
||||
if (!impl_decl.impl_id.has_value()) {
|
||||
impl_info.generic_id = BuildGeneric(context, impl_decl_id);
|
||||
impl_info.witness_id = ImplWitnessForDeclaration(context, impl_info);
|
||||
AddConstantsToImplWitnessFromConstraint(
|
||||
context, impl_info, impl_info.witness_id, impl_decl.impl_id);
|
||||
AddConstantsToImplWitnessFromConstraint(context, impl_info,
|
||||
impl_info.witness_id);
|
||||
FinishGenericDecl(context, impl_decl_id, impl_info.generic_id);
|
||||
impl_decl.impl_id = context.impls().Add(impl_info);
|
||||
lookup_bucket_ref.push_back(impl_decl.impl_id);
|
||||
} else {
|
||||
const auto& first_impl = context.impls().Get(impl_decl.impl_id);
|
||||
AddConstantsToImplWitnessFromConstraint(
|
||||
context, impl_info, first_impl.witness_id, impl_decl.impl_id);
|
||||
FinishGenericRedecl(context, impl_decl_id, first_impl.generic_id);
|
||||
}
|
||||
|
||||
|
||||
Reference in New Issue
Block a user