carbon-language/carbon-lang
1
0
Fork 0
mirror of https://github.com/carbon-language/carbon-lang.git synced 2026-09-24 20:20:10 +01:00
Code Issues Packages Projects Releases 10 Wiki Activity

Avoid speculatively pushing a pattern block in impl handling (#4943)

Browse Source 
To do this, we restructure the parse tree to make `forall` a leaf node
that comes before the parameter list.

---------

Co-authored-by: Jon Ross-Perkins <jperkins@google.com>
This commit is contained in:
Geoff Romer
2025-02-12 23:21:57 +00:00
committed by GitHub
co-authored by Jon Ross-Perkins
parent 95f2140a04
commit f502e8d6ff
7 changed files with 67 additions and 70 deletions
Download Patch File Download Diff File Expand all files Collapse all files
toolchain/check/handle_impl.cpp
+12 -15
View File
@@ -38,22 +38,14 @@ auto HandleParseNode(Context& context, Parse::ImplIntroducerId node_id)
// This might be a generic impl.
StartGenericDecl(context);
// Push a pattern block for the signature of the `forall` (if any).
// TODO: Instead use a separate parse node kinds for `impl` and `impl forall`,
// and only push a pattern block in `forall` case.
context.pattern_block_stack().Push();
context.full_pattern_stack().PushFullPattern(
FullPatternStack::Kind::ImplicitParamList);
return true;
}
auto HandleParseNode(Context& context, Parse::ImplForallId node_id) -> bool {
auto params_id =
context.node_stack().Pop<Parse::NodeKind::ImplicitParamList>();
context.node_stack()
.PopAndDiscardSoloNodeId<Parse::NodeKind::ImplicitParamListStart>();
context.node_stack().Push(node_id, params_id);
auto HandleParseNode(Context& context, Parse::ForallId /*node_id*/) -> bool {
// Push a pattern block for the signature of the `forall`.
context.pattern_block_stack().Push();
context.full_pattern_stack().PushFullPattern(
FullPatternStack::Kind::ImplicitParamList);
return true;
}
@@ -208,9 +200,12 @@ static auto PopImplIntroducerAndParamsAsNameComponent(
Context& context, Parse::AnyImplDeclId end_of_decl_node_id)
-> NameComponent {
auto [implicit_params_loc_id, implicit_param_patterns_id] =
context.node_stack().PopWithNodeIdIf<Parse::NodeKind::ImplForall>();
context.node_stack()
.PopWithNodeIdIf<Parse::NodeKind::ImplicitParamList>();
if (implicit_param_patterns_id) {
context.node_stack()
.PopAndDiscardSoloNodeId<Parse::NodeKind::ImplicitParamListStart>();
// Emit the `forall` match. This shouldn't produce any valid `Call` params,
// because `impl`s are never actually called at runtime.
auto call_params_id =
@@ -243,7 +238,9 @@ static auto PopImplIntroducerAndParamsAsNameComponent(
.param_patterns_id = SemIR::InstBlockId::None,
.call_params_id = SemIR::InstBlockId::None,
.return_slot_pattern_id = SemIR::InstId::None,
.pattern_block_id = context.pattern_block_stack().Pop(),
.pattern_block_id = implicit_param_patterns_id
? context.pattern_block_stack().Pop()
: SemIR::InstBlockId::None,
};
}
toolchain/check/node_stack.h
+1 -1
View File
@@ -409,7 +409,6 @@ class NodeStack {
return Id::KindFor<SemIR::InstId>();
case Parse::NodeKind::IfCondition:
case Parse::NodeKind::IfExprIf:
case Parse::NodeKind::ImplForall:
case Parse::NodeKind::ImplicitParamList:
case Parse::NodeKind::TuplePattern:
case Parse::NodeKind::WhileCondition:
@@ -467,6 +466,7 @@ class NodeStack {
case Parse::NodeKind::ExportIntroducer:
case Parse::NodeKind::FileEnd:
case Parse::NodeKind::FileStart:
case Parse::NodeKind::Forall:
case Parse::NodeKind::ForHeader:
case Parse::NodeKind::ForHeaderStart:
case Parse::NodeKind::ForIn:
toolchain/parse/handle_impl.cpp
+1 -2
View File
@@ -27,7 +27,7 @@ auto HandleImplAfterIntroducer(Context& context) -> void {
if (context.PositionIs(Lex::TokenKind::Forall)) {
// forall [<implicit parameter list>] ...
context.PushState(State::ImplAfterForall);
context.ConsumeAndDiscard();
context.AddLeafNode(NodeKind::Forall, context.Consume());
if (context.PositionIs(Lex::TokenKind::OpenSquareBracket)) {
context.PushState(State::PatternListAsImplicit);
} else {
@@ -53,7 +53,6 @@ auto HandleImplAfterForall(Context& context) -> void {
if (state.has_error) {
context.ReturnErrorOnState();
}
context.AddNode(NodeKind::ImplForall, state.token, state.has_error);
// One of:
// as <expression> ...
// <expression> as <expression>...
toolchain/parse/node_kind.def
+1 -1
View File
@@ -340,7 +340,7 @@ CARBON_PARSE_NODE_KIND(ImplIntroducer)
CARBON_PARSE_NODE_KIND(ImplDefinitionStart)
CARBON_PARSE_NODE_KIND(ImplDefinition)
CARBON_PARSE_NODE_KIND(ImplDecl)
CARBON_PARSE_NODE_KIND(ImplForall)
CARBON_PARSE_NODE_KIND(Forall)
CARBON_PARSE_NODE_KIND(TypeImplAs)
CARBON_PARSE_NODE_KIND(DefaultSelfImplAs)
toolchain/parse/testdata/generics/impl/fail_impl.carbon
Vendored
+31 -31
View File
@@ -123,57 +123,57 @@ impl
// CHECK:STDOUT: {kind: 'InvalidParse', text: 'return', has_error: yes},
// CHECK:STDOUT: {kind: 'ImplDecl', text: ';', has_error: yes, subtree_size: 3},
// CHECK:STDOUT: {kind: 'ImplIntroducer', text: 'impl'},
// CHECK:STDOUT: {kind: 'InvalidParse', text: 'f32', has_error: yes},
// CHECK:STDOUT: {kind: 'ImplForall', text: 'forall', has_error: yes, subtree_size: 2},
// CHECK:STDOUT: {kind: 'Forall', text: 'forall'},
// CHECK:STDOUT: {kind: 'InvalidParse', text: 'f32', has_error: yes},
// CHECK:STDOUT: {kind: 'FloatTypeLiteral', text: 'f32'},
// CHECK:STDOUT: {kind: 'ImplDecl', text: ';', has_error: yes, subtree_size: 5},
// CHECK:STDOUT: {kind: 'ImplIntroducer', text: 'impl'},
// CHECK:STDOUT: {kind: 'ImplicitParamListStart', text: '['},
// CHECK:STDOUT: {kind: 'ImplicitParamList', text: ']', subtree_size: 2},
// CHECK:STDOUT: {kind: 'ImplForall', text: 'forall', subtree_size: 3},
// CHECK:STDOUT: {kind: 'Forall', text: 'forall'},
// CHECK:STDOUT: {kind: 'ImplicitParamListStart', text: '['},
// CHECK:STDOUT: {kind: 'ImplicitParamList', text: ']', subtree_size: 2},
// CHECK:STDOUT: {kind: 'UnsignedIntTypeLiteral', text: 'u32'},
// CHECK:STDOUT: {kind: 'ImplDecl', text: ';', has_error: yes, subtree_size: 6},
// CHECK:STDOUT: {kind: 'ImplIntroducer', text: 'impl'},
// CHECK:STDOUT: {kind: 'ImplicitParamListStart', text: '['},
// CHECK:STDOUT: {kind: 'IdentifierNameNotBeforeParams', text: 'invalid'},
// CHECK:STDOUT: {kind: 'InvalidParse', text: ']', has_error: yes},
// CHECK:STDOUT: {kind: 'LetBindingPattern', text: 'invalid', has_error: yes, subtree_size: 3},
// CHECK:STDOUT: {kind: 'ImplicitParamList', text: ']', has_error: yes, subtree_size: 5},
// CHECK:STDOUT: {kind: 'ImplForall', text: 'forall', subtree_size: 6},
// CHECK:STDOUT: {kind: 'Forall', text: 'forall'},
// CHECK:STDOUT: {kind: 'ImplicitParamListStart', text: '['},
// CHECK:STDOUT: {kind: 'IdentifierNameNotBeforeParams', text: 'invalid'},
// CHECK:STDOUT: {kind: 'InvalidParse', text: ']', has_error: yes},
// CHECK:STDOUT: {kind: 'LetBindingPattern', text: 'invalid', has_error: yes, subtree_size: 3},
// CHECK:STDOUT: {kind: 'ImplicitParamList', text: ']', has_error: yes, subtree_size: 5},
// CHECK:STDOUT: {kind: 'IntTypeLiteral', text: 'i8'},
// CHECK:STDOUT: {kind: 'ImplDecl', text: ';', has_error: yes, subtree_size: 9},
// CHECK:STDOUT: {kind: 'ImplIntroducer', text: 'impl'},
// CHECK:STDOUT: {kind: 'InvalidParse', text: 'f16', has_error: yes},
// CHECK:STDOUT: {kind: 'ImplForall', text: 'forall', has_error: yes, subtree_size: 2},
// CHECK:STDOUT: {kind: 'Forall', text: 'forall'},
// CHECK:STDOUT: {kind: 'InvalidParse', text: 'f16', has_error: yes},
// CHECK:STDOUT: {kind: 'FloatTypeLiteral', text: 'f16'},
// CHECK:STDOUT: {kind: 'TypeImplAs', text: 'as', subtree_size: 2},
// CHECK:STDOUT: {kind: 'IdentifierNameExpr', text: 'Quux'},
// CHECK:STDOUT: {kind: 'ImplDecl', text: ';', has_error: yes, subtree_size: 7},
// CHECK:STDOUT: {kind: 'ImplIntroducer', text: 'impl'},
// CHECK:STDOUT: {kind: 'ImplicitParamListStart', text: '['},
// CHECK:STDOUT: {kind: 'IdentifierNameNotBeforeParams', text: 'T'},
// CHECK:STDOUT: {kind: 'TypeTypeLiteral', text: 'type'},
// CHECK:STDOUT: {kind: 'CompileTimeBindingPattern', text: ':!', subtree_size: 3},
// CHECK:STDOUT: {kind: 'ImplicitParamList', text: ']', subtree_size: 5},
// CHECK:STDOUT: {kind: 'ImplForall', text: 'forall', subtree_size: 6},
// CHECK:STDOUT: {kind: 'Forall', text: 'forall'},
// CHECK:STDOUT: {kind: 'ImplicitParamListStart', text: '['},
// CHECK:STDOUT: {kind: 'IdentifierNameNotBeforeParams', text: 'T'},
// CHECK:STDOUT: {kind: 'TypeTypeLiteral', text: 'type'},
// CHECK:STDOUT: {kind: 'CompileTimeBindingPattern', text: ':!', subtree_size: 3},
// CHECK:STDOUT: {kind: 'ImplicitParamList', text: ']', subtree_size: 5},
// CHECK:STDOUT: {kind: 'StringTypeLiteral', text: 'String'},
// CHECK:STDOUT: {kind: 'ImplDecl', text: ';', has_error: yes, subtree_size: 9},
// CHECK:STDOUT: {kind: 'ImplIntroducer', text: 'impl'},
// CHECK:STDOUT: {kind: 'ImplicitParamListStart', text: '['},
// CHECK:STDOUT: {kind: 'IdentifierNameNotBeforeParams', text: 'T'},
// CHECK:STDOUT: {kind: 'TypeTypeLiteral', text: 'type'},
// CHECK:STDOUT: {kind: 'CompileTimeBindingPattern', text: ':!', subtree_size: 3},
// CHECK:STDOUT: {kind: 'ImplicitParamList', text: ']', subtree_size: 5},
// CHECK:STDOUT: {kind: 'ImplForall', text: 'forall', subtree_size: 6},
// CHECK:STDOUT: {kind: 'Forall', text: 'forall'},
// CHECK:STDOUT: {kind: 'ImplicitParamListStart', text: '['},
// CHECK:STDOUT: {kind: 'IdentifierNameNotBeforeParams', text: 'T'},
// CHECK:STDOUT: {kind: 'TypeTypeLiteral', text: 'type'},
// CHECK:STDOUT: {kind: 'CompileTimeBindingPattern', text: ':!', subtree_size: 3},
// CHECK:STDOUT: {kind: 'ImplicitParamList', text: ']', subtree_size: 5},
// CHECK:STDOUT: {kind: 'IdentifierNameExpr', text: 'T'},
// CHECK:STDOUT: {kind: 'ImplDecl', text: ';', has_error: yes, subtree_size: 9},
// CHECK:STDOUT: {kind: 'ImplIntroducer', text: 'impl'},
// CHECK:STDOUT: {kind: 'ImplicitParamListStart', text: '['},
// CHECK:STDOUT: {kind: 'IdentifierNameNotBeforeParams', text: 'T'},
// CHECK:STDOUT: {kind: 'TypeTypeLiteral', text: 'type'},
// CHECK:STDOUT: {kind: 'CompileTimeBindingPattern', text: ':!', subtree_size: 3},
// CHECK:STDOUT: {kind: 'ImplicitParamList', text: ']', subtree_size: 5},
// CHECK:STDOUT: {kind: 'ImplForall', text: 'forall', subtree_size: 6},
// CHECK:STDOUT: {kind: 'Forall', text: 'forall'},
// CHECK:STDOUT: {kind: 'ImplicitParamListStart', text: '['},
// CHECK:STDOUT: {kind: 'IdentifierNameNotBeforeParams', text: 'T'},
// CHECK:STDOUT: {kind: 'TypeTypeLiteral', text: 'type'},
// CHECK:STDOUT: {kind: 'CompileTimeBindingPattern', text: ':!', subtree_size: 3},
// CHECK:STDOUT: {kind: 'ImplicitParamList', text: ']', subtree_size: 5},
// CHECK:STDOUT: {kind: 'IdentifierNameExpr', text: 'T'},
// CHECK:STDOUT: {kind: 'TypeImplAs', text: 'as', subtree_size: 2},
// CHECK:STDOUT: {kind: 'IdentifierNameExpr', text: 'Interface'},
toolchain/parse/testdata/generics/impl/forall.carbon
Vendored
+16 -16
View File
@@ -17,27 +17,27 @@ impl forall [T:! type, U:! Interface] U as Interface(T) {
// CHECK:STDOUT: parse_tree: [
// CHECK:STDOUT: {kind: 'FileStart', text: ''},
// CHECK:STDOUT: {kind: 'ImplIntroducer', text: 'impl'},
// CHECK:STDOUT: {kind: 'ImplicitParamListStart', text: '['},
// CHECK:STDOUT: {kind: 'IdentifierNameNotBeforeParams', text: 'T'},
// CHECK:STDOUT: {kind: 'TypeTypeLiteral', text: 'type'},
// CHECK:STDOUT: {kind: 'CompileTimeBindingPattern', text: ':!', subtree_size: 3},
// CHECK:STDOUT: {kind: 'ImplicitParamList', text: ']', subtree_size: 5},
// CHECK:STDOUT: {kind: 'ImplForall', text: 'forall', subtree_size: 6},
// CHECK:STDOUT: {kind: 'Forall', text: 'forall'},
// CHECK:STDOUT: {kind: 'ImplicitParamListStart', text: '['},
// CHECK:STDOUT: {kind: 'IdentifierNameNotBeforeParams', text: 'T'},
// CHECK:STDOUT: {kind: 'TypeTypeLiteral', text: 'type'},
// CHECK:STDOUT: {kind: 'CompileTimeBindingPattern', text: ':!', subtree_size: 3},
// CHECK:STDOUT: {kind: 'ImplicitParamList', text: ']', subtree_size: 5},
// CHECK:STDOUT: {kind: 'IdentifierNameExpr', text: 'T'},
// CHECK:STDOUT: {kind: 'TypeImplAs', text: 'as', subtree_size: 2},
// CHECK:STDOUT: {kind: 'IdentifierNameExpr', text: 'Interface'},
// CHECK:STDOUT: {kind: 'ImplDecl', text: ';', subtree_size: 11},
// CHECK:STDOUT: {kind: 'ImplIntroducer', text: 'impl'},
// CHECK:STDOUT: {kind: 'ImplicitParamListStart', text: '['},
// CHECK:STDOUT: {kind: 'IdentifierNameNotBeforeParams', text: 'T'},
// CHECK:STDOUT: {kind: 'TypeTypeLiteral', text: 'type'},
// CHECK:STDOUT: {kind: 'CompileTimeBindingPattern', text: ':!', subtree_size: 3},
// CHECK:STDOUT: {kind: 'PatternListComma', text: ','},
// CHECK:STDOUT: {kind: 'IdentifierNameNotBeforeParams', text: 'U'},
// CHECK:STDOUT: {kind: 'IdentifierNameExpr', text: 'Interface'},
// CHECK:STDOUT: {kind: 'CompileTimeBindingPattern', text: ':!', subtree_size: 3},
// CHECK:STDOUT: {kind: 'ImplicitParamList', text: ']', subtree_size: 9},
// CHECK:STDOUT: {kind: 'ImplForall', text: 'forall', subtree_size: 10},
// CHECK:STDOUT: {kind: 'Forall', text: 'forall'},
// CHECK:STDOUT: {kind: 'ImplicitParamListStart', text: '['},
// CHECK:STDOUT: {kind: 'IdentifierNameNotBeforeParams', text: 'T'},
// CHECK:STDOUT: {kind: 'TypeTypeLiteral', text: 'type'},
// CHECK:STDOUT: {kind: 'CompileTimeBindingPattern', text: ':!', subtree_size: 3},
// CHECK:STDOUT: {kind: 'PatternListComma', text: ','},
// CHECK:STDOUT: {kind: 'IdentifierNameNotBeforeParams', text: 'U'},
// CHECK:STDOUT: {kind: 'IdentifierNameExpr', text: 'Interface'},
// CHECK:STDOUT: {kind: 'CompileTimeBindingPattern', text: ':!', subtree_size: 3},
// CHECK:STDOUT: {kind: 'ImplicitParamList', text: ']', subtree_size: 9},
// CHECK:STDOUT: {kind: 'IdentifierNameExpr', text: 'U'},
// CHECK:STDOUT: {kind: 'TypeImplAs', text: 'as', subtree_size: 2},
// CHECK:STDOUT: {kind: 'IdentifierNameExpr', text: 'Interface'},
toolchain/parse/typed_nodes.h
+5 -4
View File
@@ -1341,11 +1341,12 @@ struct InterfaceDefinition {
// `impl`
using ImplIntroducer = LeafNode<NodeKind::ImplIntroducer, Lex::ImplTokenIndex>;
// `forall`
using Forall = LeafNode<NodeKind::Forall, Lex::ForallTokenIndex>;
// `forall [...]`
struct ImplForall {
static constexpr auto Kind = NodeKind::ImplForall.Define({.child_count = 1});
Lex::ForallTokenIndex token;
ForallId forall;
ImplicitParamListId params;
};
@@ -1371,7 +1372,7 @@ struct ImplSignature {
ImplIntroducerId introducer;
llvm::SmallVector<AnyModifierId> modifiers;
std::optional<ImplForallId> forall;
std::optional<ImplForall> forall;
AnyImplAsId as;
AnyExprId interface;
TokenKind token;