diff --git a/toolchain/check/handle_impl.cpp b/toolchain/check/handle_impl.cpp index 30367ceaa7e09..3f731712d8950 100644 --- a/toolchain/check/handle_impl.cpp +++ b/toolchain/check/handle_impl.cpp @@ -191,8 +191,7 @@ static auto ExtendImpl(Context& context, Parse::NodeId extend_node, // Pops the parameters of an `impl`, forming a `NameComponent` with no // associated name that describes them. static auto PopImplIntroducerAndParamsAsNameComponent( - Context& context, Parse::AnyImplDeclId end_of_decl_node_id) - -> NameComponent { + Context& context, Parse::NodeId last_param_node_id) -> NameComponent { auto [implicit_params_loc_id, implicit_param_patterns_id] = context.node_stack().PopWithNodeIdIf(); @@ -213,44 +212,11 @@ static auto PopImplIntroducerAndParamsAsNameComponent( Parse::NodeId first_param_node_id = context.node_stack().PopForSoloNodeId(); - // 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::Invalid, .name_id = SemIR::NameId::Invalid, .first_param_node_id = first_param_node_id, - .last_param_node_id = *last_param_iter, + .last_param_node_id = last_param_node_id, .implicit_params_loc_id = implicit_params_loc_id, .implicit_param_patterns_id = implicit_param_patterns_id.value_or(SemIR::InstBlockId::Invalid), @@ -310,11 +276,35 @@ static auto BuildImplDecl(Context& context, Parse::AnyImplDeclId node_id, -> std::pair { auto [constraint_node, constraint_id] = context.node_stack().PopExprWithNodeId(); + + // If the constraint is a `where` constraint, we need to stop syntactic + // matching at it. For this case, `WhereExpr` handling will have left the + // `WhereOperand` on the node stack. + Parse::NodeId last_param_node_id = Parse::NodeId::Invalid; + if (context.parse_tree().node_kind(constraint_node) == + Parse::NodeKind::WhereExpr) { + // Only keep the node ID, not the inst ID. + auto [operand_node, _] = + context.node_stack().PopWithNodeId(); + // Exclude the `where` itself from params. + last_param_node_id = *--Parse::Tree::PostorderIterator(operand_node); + } else { + // Exclude the `AnyImplDeclId` (either a `;` or `{`) from params. + last_param_node_id = *--Parse::Tree::PostorderIterator(node_id); + } + auto [self_type_node, self_inst_id] = context.node_stack().PopWithNodeId(); auto self_type_id = context.GetTypeIdForTypeInst(self_inst_id); + + // Finish processing the name, which should be empty, but might have + // parameters. + auto name_context = context.decl_name_stack().FinishImplName(); + CARBON_CHECK(name_context.state == DeclNameStack::NameContext::State::Empty); + // Pop the `impl` introducer and any `forall` parameters as a "name". - auto name = PopImplIntroducerAndParamsAsNameComponent(context, node_id); + auto name = + PopImplIntroducerAndParamsAsNameComponent(context, last_param_node_id); auto decl_block_id = context.inst_block_stack().Pop(); // Convert the constraint expression to a type. @@ -334,11 +324,6 @@ static auto BuildImplDecl(Context& context, Parse::AnyImplDeclId node_id, context.decl_introducer_state_stack().Pop(); LimitModifiersOnDecl(context, introducer, KeywordModifierSet::ImplDecl); - // Finish processing the name, which should be empty, but might have - // parameters. - auto name_context = context.decl_name_stack().FinishImplName(); - CARBON_CHECK(name_context.state == DeclNameStack::NameContext::State::Empty); - // TODO: Check for an orphan `impl`. // Add the impl declaration. diff --git a/toolchain/check/handle_paren_expr.cpp b/toolchain/check/handle_paren_expr.cpp index 7742d4af955a2..d6e01c2a3c7a9 100644 --- a/toolchain/check/handle_paren_expr.cpp +++ b/toolchain/check/handle_paren_expr.cpp @@ -4,19 +4,23 @@ #include "toolchain/check/context.h" #include "toolchain/check/handle.h" +#include "toolchain/parse/typed_nodes.h" namespace Carbon::Check { -auto HandleParseNode(Context& /*context*/, Parse::ParenExprStartId /*node_id*/) +auto HandleParseNode(Context& context, Parse::ParenExprStartId node_id) -> bool { - // The open paren is unused. + // Push the start to help track nesting. + context.node_stack().Push(node_id); return true; } auto HandleParseNode(Context& context, Parse::ParenExprId node_id) -> bool { - // We re-push because the ParenExpr is valid for member expressions, whereas - // the child expression might not be. - context.node_stack().Push(node_id, context.node_stack().PopExpr()); + auto expr = context.node_stack().PopExpr(); + context.node_stack().PopForSoloNodeId(); + // We push with the ParenExpr node because it's valid for member expressions, + // whereas the child expression might not be. + context.node_stack().Push(node_id, expr); return true; } diff --git a/toolchain/check/handle_where.cpp b/toolchain/check/handle_where.cpp index 6de36ed159021..8722b35db59ba 100644 --- a/toolchain/check/handle_where.cpp +++ b/toolchain/check/handle_where.cpp @@ -122,8 +122,15 @@ auto HandleParseNode(Context& context, Parse::WhereExprId node_id) -> bool { // Remove `PeriodSelf` from name lookup, undoing the `Push` done for the // `WhereOperand`. context.scope_stack().Pop(); - SemIR::InstId period_self_id = - context.node_stack().Pop(); + + auto period_self_id = + context.node_stack().Peek(); + // In most expressions, pop the `WhereOperand` node. In the `impl ... as ... + // where ...` case, leave the `WhereOperand` node on the stack so that part + // can be trimmed off by impl's syntactic decl matching. + if (!context.node_stack().PeekNextIs(Parse::NodeCategory::ImplAs)) { + context.node_stack().Pop(); + } SemIR::InstBlockId requirements_id = context.args_type_info_stack().Pop(); context.AddInstAndPush( node_id, {.type_id = SemIR::TypeType::SingletonTypeId, diff --git a/toolchain/check/node_stack.h b/toolchain/check/node_stack.h index 0de0e75b0aa53..e956b18cfe01e 100644 --- a/toolchain/check/node_stack.h +++ b/toolchain/check/node_stack.h @@ -134,15 +134,28 @@ class NodeStack { NodeKindToIdKind(PeekNodeKind()) == Id::KindFor(); } - // Returns whether the *next* node on the stack is a given kind. This doesn't - // have the breadth of support versus other Peek functions because it's - // expected to be used in narrow circumstances when determining how to treat - // the *current* top of the stack. + // Returns whether the *next* node on the stack is a given kind. + // + // This doesn't have the breadth of support versus other Peek functions + // because it's expected to be used in narrow circumstances when determining + // how to treat the *current* top of the stack. template auto PeekNextIs() const -> bool { CARBON_CHECK(stack_.size() >= 2); - return parse_tree_->node_kind(stack_[stack_.size() - 2].node_id) == - RequiredParseKind; + auto next_kind = parse_tree_->node_kind(stack_[stack_.size() - 2].node_id); + return next_kind == RequiredParseKind; + } + + // Returns whether the *next* node on the stack has an overlapping + // category. + // + // This doesn't have the breadth of support versus other Peek functions + // because it's expected to be used in narrow circumstances when determining + // how to treat the *current* top of the stack. + auto PeekNextIs(Parse::NodeCategory category) const -> bool { + CARBON_CHECK(stack_.size() >= 2); + auto next_kind = parse_tree_->node_kind(stack_[stack_.size() - 2].node_id); + return next_kind.category().HasAnyOf(category); } // Pops the top of the stack without any verification. @@ -458,6 +471,7 @@ class NodeStack { case Parse::NodeKind::InterfaceIntroducer: case Parse::NodeKind::LetInitializer: case Parse::NodeKind::LetIntroducer: + case Parse::NodeKind::ParenExprStart: case Parse::NodeKind::ReturnedModifier: case Parse::NodeKind::ReturnStatementStart: case Parse::NodeKind::ReturnVarModifier: @@ -602,7 +616,6 @@ class NodeStack { case Parse::NodeKind::PackageIntroducer: case Parse::NodeKind::PackageName: case Parse::NodeKind::ParenExpr: - case Parse::NodeKind::ParenExprStart: case Parse::NodeKind::PatternListComma: case Parse::NodeKind::Placeholder: case Parse::NodeKind::PointerMemberAccessExpr: