diff --git a/toolchain/check/full_pattern_stack.h b/toolchain/check/full_pattern_stack.h index 1043b86453ab..6e0a13e2142a 100644 --- a/toolchain/check/full_pattern_stack.h +++ b/toolchain/check/full_pattern_stack.h @@ -12,12 +12,16 @@ namespace Carbon::Check { -// Stack of full-patterns currently being checked. When a pattern -// is followed by an explicit initializer, name bindings should not be used -// within that initializer, although they are usable before it (within the -// pattern) and after it. This class keeps track of those state transitions. -// It is structured as a stack to handle situations like a pattern that -// contains an initializer, or a pattern in a lambda in an expression pattern. +// Stack of full-patterns currently being checked (a full-pattern is a pattern +// that is not part of an enclosing pattern). It is structured as a stack to +// handle situations like a pattern that contains an initializer, or a pattern +// in a lambda in an expression pattern. +// +// When a pattern is followed by an explicit initializer, name bindings should +// not be used within that initializer, although they are usable before it +// (within the pattern) and after it. This class keeps track of those state +// transitions, as well as the kind of full-pattern (e.g. parameter list or name +// binding pattern). // // TODO: Unify this with Context::pattern_block_stack, or differentiate them // more clearly (and consider unifying this with ScopeStack instead). @@ -25,13 +29,36 @@ class FullPatternStack { public: explicit FullPatternStack(LexicalLookup* lookup) : lookup_(lookup) {} - // Marks the possible start of a new full-pattern (i.e. a pattern which occurs - // in a non-pattern context). - auto PushFullPattern() -> void { bind_name_stack_.PushArray(); } + // The kind of a full-pattern. Note that an implicit parameter list and + // adjacent explicit parameter list together form a single full-pattern, + // but we separate them here in order to represent the state transition + // between them. + enum class Kind { + NameBindingDecl, + ImplicitParamList, + ExplicitParamList, + }; - // Marks the start of the initializer for the full-pattern at the top of the - // stack. + // The kind of the current full-pattern. + auto CurrentKind() const -> Kind { return kind_stack_.back(); } + + // Marks the start of a new full-pattern of the specified kind. + auto PushFullPattern(Kind kind) -> void { + kind_stack_.push_back(kind); + bind_name_stack_.PushArray(); + } + + // Marks the end of an implicit parameter list, and the presumptive start + // of the corresponding explicit parameter list. + auto EndImplicitParamList() -> void { + CARBON_CHECK(kind_stack_.back() == Kind::ImplicitParamList, "{0}", + kind_stack_.back()); + kind_stack_.back() = Kind::ExplicitParamList; + } + + // Marks the start of the initializer for the current name binding decl. auto StartPatternInitializer() -> void { + CARBON_CHECK(kind_stack_.back() == Kind::NameBindingDecl); for (auto& [name_id, inst_id] : bind_name_stack_.PeekArray()) { CARBON_CHECK(inst_id == SemIR::InstId::InitTombstone); auto& lookup_result = lookup_->Get(name_id); @@ -44,8 +71,7 @@ class FullPatternStack { } } - // Marks the end of the initializer for the full-pattern at the top of the - // stack. + // Marks the end of the initializer for the current name-binding decl. auto EndPatternInitializer() -> void { for (auto& [name_id, inst_id] : bind_name_stack_.PeekArray()) { auto& lookup_result = lookup_->Get(name_id); @@ -56,13 +82,14 @@ class FullPatternStack { } } - // Marks the end of checking for the full-pattern at the top of the stack. - // This cannot be called while processing an initializer for the top - // pattern. - auto PopFullPattern() -> void { bind_name_stack_.PopArray(); } + // Marks the end of checking for the current full-pattern. This cannot be + // called while processing an initializer for the top pattern. + auto PopFullPattern() -> void { + kind_stack_.pop_back(); + bind_name_stack_.PopArray(); + } - // Records that `bind_inst_id` was introduced by the full-pattern at the - // top of the stack. + // Records that `name_id` was introduced by the current full-pattern. auto AddBindName(SemIR::NameId name_id) -> void { bind_name_stack_.AppendToTop( {.name_id = name_id, .inst_id = SemIR::InstId::InitTombstone}); @@ -70,6 +97,9 @@ class FullPatternStack { private: LexicalLookup* lookup_; + + llvm::SmallVector kind_stack_; + struct LookupEntry { SemIR::NameId name_id; SemIR::InstId inst_id; diff --git a/toolchain/check/handle_binding_pattern.cpp b/toolchain/check/handle_binding_pattern.cpp index 8391860ad4dd..b10bf0752c9d 100644 --- a/toolchain/check/handle_binding_pattern.cpp +++ b/toolchain/check/handle_binding_pattern.cpp @@ -107,15 +107,11 @@ static auto HandleAnyBindingPattern(Context& context, Parse::NodeId node_id, context.emitter().Emit(node_id, SelfOutsideImplicitParamList); } - // TODO: The node stack is a fragile way of getting context information. - // Get this information from somewhere else. - auto context_node_kind = context.node_stack().PeekNodeKind(); - // A `var` binding in a class scope declares a field, not a true binding, // so we handle it separately. if (auto parent_class_decl = context.GetCurrentScopeAs(); parent_class_decl.has_value() && - context_node_kind == Parse::NodeKind::VariableIntroducer) { + node_kind == Parse::NodeKind::VarBindingPattern) { cast_type_id = context.AsConcreteType( cast_type_id, type_node, [&] { @@ -185,44 +181,9 @@ static auto HandleAnyBindingPattern(Context& context, Parse::NodeId node_id, // Allocate an instruction of the appropriate kind, linked to the name for // error locations. - switch (context_node_kind) { - case Parse::NodeKind::VariableIntroducer: { - CARBON_CHECK(!is_generic); - - cast_type_id = context.AsConcreteType( - cast_type_id, type_node, - [&] { - CARBON_DIAGNOSTIC(IncompleteTypeInVarDecl, Error, - "variable has incomplete type {0}", - SemIR::TypeId); - return context.emitter().Build(type_node, IncompleteTypeInVarDecl, - cast_type_id); - }, - [&] { - CARBON_DIAGNOSTIC(AbstractTypeInVarDecl, Error, - "variable has abstract type {0}", SemIR::TypeId); - return context.emitter().Build(type_node, AbstractTypeInVarDecl, - cast_type_id); - }); - - auto binding_pattern_id = make_binding_pattern(); - if (introducer.modifier_set.HasAnyOf(KeywordModifierSet::Returned)) { - // TODO: Should we check this for the `var` as a whole, rather than for - // the name binding? - auto bind_id = context.bind_name_map() - .Lookup(binding_pattern_id) - .value() - .bind_name_id; - RegisterReturnedVar(context, - introducer.modifier_node_id(ModifierOrder::Decl), - type_node, cast_type_id, bind_id); - } - context.node_stack().Push(node_id, binding_pattern_id); - break; - } - - case Parse::NodeKind::ImplicitParamListStart: - case Parse::NodeKind::TuplePatternStart: { + switch (context.full_pattern_stack().CurrentKind()) { + case FullPatternStack::Kind::ImplicitParamList: + case FullPatternStack::Kind::ExplicitParamList: { // Parameters can have incomplete types in a function declaration, but not // in a function definition. We don't know which kind we have here. // TODO: A tuple pattern can appear in other places than function @@ -231,7 +192,8 @@ static auto HandleAnyBindingPattern(Context& context, Parse::NodeId node_id, bool had_error = false; switch (introducer.kind) { case Lex::TokenKind::Fn: { - if (context_node_kind == Parse::NodeKind::ImplicitParamListStart && + if (context.full_pattern_stack().CurrentKind() == + FullPatternStack::Kind::ImplicitParamList && !(is_generic || name_id == SemIR::NameId::SelfValue)) { CARBON_DIAGNOSTIC( ImplictParamMustBeConstant, Error, @@ -277,28 +239,52 @@ static auto HandleAnyBindingPattern(Context& context, Parse::NodeId node_id, }); } context.node_stack().Push(node_id, param_pattern_id); - - // TODO: Use the pattern insts to generate the pattern-match insts - // at the end of the full pattern, instead of eagerly generating them - // here. break; } - case Parse::NodeKind::LetIntroducer: { - cast_type_id = context.AsCompleteType(cast_type_id, type_node, [&] { - CARBON_DIAGNOSTIC(IncompleteTypeInLetDecl, Error, - "`let` binding has incomplete type {0}", + case FullPatternStack::Kind::NameBindingDecl: { + auto incomplete_diagnoser = [&] { + CARBON_DIAGNOSTIC(IncompleteTypeInBindingDecl, Error, + "binding pattern has incomplete type {0} in name " + "binding declaration", InstIdAsType); - return context.emitter().Build(type_node, IncompleteTypeInLetDecl, + return context.emitter().Build(type_node, IncompleteTypeInBindingDecl, cast_type_inst_id); - }); - context.node_stack().Push(node_id, make_binding_pattern()); + }; + if (node_kind == Parse::NodeKind::VarBindingPattern) { + cast_type_id = context.AsConcreteType( + cast_type_id, type_node, incomplete_diagnoser, [&] { + CARBON_DIAGNOSTIC( + AbstractTypeInVarPattern, Error, + "binding pattern has abstract type {0} in `var` " + "pattern", + SemIR::TypeId); + return context.emitter().Build( + type_node, AbstractTypeInVarPattern, cast_type_id); + }); + } else { + cast_type_id = context.AsCompleteType(cast_type_id, type_node, + incomplete_diagnoser); + } + auto binding_pattern_id = make_binding_pattern(); + if (node_kind == Parse::NodeKind::VarBindingPattern) { + CARBON_CHECK(!is_generic); + + if (introducer.modifier_set.HasAnyOf(KeywordModifierSet::Returned)) { + // TODO: Should we check this for the `var` as a whole, rather than + // for the name binding? + auto bind_id = context.bind_name_map() + .Lookup(binding_pattern_id) + .value() + .bind_name_id; + RegisterReturnedVar(context, + introducer.modifier_node_id(ModifierOrder::Decl), + type_node, cast_type_id, bind_id); + } + } + context.node_stack().Push(node_id, binding_pattern_id); break; } - - default: - CARBON_FATAL("Found a pattern binding in unexpected context {0}", - context_node_kind); } return true; } diff --git a/toolchain/check/handle_impl.cpp b/toolchain/check/handle_impl.cpp index 5a193f473d76..0a826c69a367 100644 --- a/toolchain/check/handle_impl.cpp +++ b/toolchain/check/handle_impl.cpp @@ -42,7 +42,8 @@ auto HandleParseNode(Context& context, Parse::ImplIntroducerId node_id) // 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(); + context.full_pattern_stack().PushFullPattern( + FullPatternStack::Kind::ImplicitParamList); return true; } diff --git a/toolchain/check/handle_let_and_var.cpp b/toolchain/check/handle_let_and_var.cpp index 2b1da10eb4fc..0d8352624bd9 100644 --- a/toolchain/check/handle_let_and_var.cpp +++ b/toolchain/check/handle_let_and_var.cpp @@ -24,7 +24,8 @@ static auto HandleIntroducer(Context& context, Parse::NodeId node_id) -> bool { // Push a bracketing node and pattern block to establish the pattern context. context.node_stack().Push(node_id); context.pattern_block_stack().Push(); - context.full_pattern_stack().PushFullPattern(); + context.full_pattern_stack().PushFullPattern( + FullPatternStack::Kind::NameBindingDecl); context.BeginSubpattern(); return true; } @@ -63,7 +64,7 @@ static auto GetOrAddStorage(Context& context, SemIR::InstId pattern_id) context.entity_names().Get(binding_pattern->entity_name_id).name_id; } return context.AddInst(SemIR::LocIdAndInst::UncheckedLoc( - pattern.loc_id, SemIR::VarStorage{.type_id = subpattern.type_id(), + pattern.loc_id, SemIR::VarStorage{.type_id = pattern.inst.type_id(), .pretty_name_id = name_id})); } diff --git a/toolchain/check/handle_name.cpp b/toolchain/check/handle_name.cpp index d43771d56ad1..7417e7e769ef 100644 --- a/toolchain/check/handle_name.cpp +++ b/toolchain/check/handle_name.cpp @@ -143,7 +143,8 @@ auto HandleParseNode(Context& context, Parse::IdentifierNameBeforeParamsId node_id) -> bool { // Push a pattern block stack entry to handle the parameter pattern. context.pattern_block_stack().Push(); - context.full_pattern_stack().PushFullPattern(); + context.full_pattern_stack().PushFullPattern( + FullPatternStack::Kind::ImplicitParamList); return HandleIdentifierName(context, node_id); } diff --git a/toolchain/check/handle_pattern_list.cpp b/toolchain/check/handle_pattern_list.cpp index b65324ff69d5..ffee24c3c5d7 100644 --- a/toolchain/check/handle_pattern_list.cpp +++ b/toolchain/check/handle_pattern_list.cpp @@ -37,6 +37,12 @@ auto HandleParseNode(Context& context, Parse::TuplePatternStartId node_id) context.node_stack().Push(node_id); context.param_and_arg_refs_stack().Push(); context.BeginSubpattern(); + // TODO: Remove this branch once the parse tree differentiates between + // tuple patterns and param patterns. + if (context.full_pattern_stack().CurrentKind() == + FullPatternStack::Kind::ImplicitParamList) { + context.full_pattern_stack().EndImplicitParamList(); + } return true; } diff --git a/toolchain/check/testdata/array/fail_incomplete_element.carbon b/toolchain/check/testdata/array/fail_incomplete_element.carbon index 53cf0c15c6a8..43dde0189ee3 100644 --- a/toolchain/check/testdata/array/fail_incomplete_element.carbon +++ b/toolchain/check/testdata/array/fail_incomplete_element.carbon @@ -10,7 +10,7 @@ class Incomplete; -// CHECK:STDERR: fail_incomplete_element.carbon:[[@LINE+7]]:8: error: variable has incomplete type `[Incomplete; 1]` [IncompleteTypeInVarDecl] +// CHECK:STDERR: fail_incomplete_element.carbon:[[@LINE+7]]:8: error: binding pattern has incomplete type `[Incomplete; 1]` in name binding declaration [IncompleteTypeInBindingDecl] // CHECK:STDERR: var a: [Incomplete; 1]; // CHECK:STDERR: ^~~~~~~~~~~~~~~ // CHECK:STDERR: fail_incomplete_element.carbon:[[@LINE-5]]:1: note: class was forward declared here [ClassForwardDeclaredHere] diff --git a/toolchain/check/testdata/class/cross_package_import.carbon b/toolchain/check/testdata/class/cross_package_import.carbon index 66aab11245b9..873805e4ebc1 100644 --- a/toolchain/check/testdata/class/cross_package_import.carbon +++ b/toolchain/check/testdata/class/cross_package_import.carbon @@ -48,7 +48,7 @@ library "[[@TEST_NAME]]"; import Other library "other_extern"; -// CHECK:STDERR: fail_extern.carbon:[[@LINE+8]]:8: error: variable has incomplete type `Other.C` [IncompleteTypeInVarDecl] +// CHECK:STDERR: fail_extern.carbon:[[@LINE+8]]:8: error: binding pattern has incomplete type `C` in name binding declaration [IncompleteTypeInBindingDecl] // CHECK:STDERR: var c: Other.C = {}; // CHECK:STDERR: ^~~~~~~ // CHECK:STDERR: fail_extern.carbon:[[@LINE-5]]:1: in import [InImport] diff --git a/toolchain/check/testdata/class/fail_abstract.carbon b/toolchain/check/testdata/class/fail_abstract.carbon index 235ec036ab16..437b5987d3d5 100644 --- a/toolchain/check/testdata/class/fail_abstract.carbon +++ b/toolchain/check/testdata/class/fail_abstract.carbon @@ -34,7 +34,7 @@ abstract class Abstract { } fn Var() { - // CHECK:STDERR: fail_abstract_var.carbon:[[@LINE+7]]:10: error: variable has abstract type `Abstract` [AbstractTypeInVarDecl] + // CHECK:STDERR: fail_abstract_var.carbon:[[@LINE+7]]:10: error: binding pattern has abstract type `Abstract` in `var` pattern [AbstractTypeInVarPattern] // CHECK:STDERR: var v: Abstract; // CHECK:STDERR: ^~~~~~~~ // CHECK:STDERR: fail_abstract_var.carbon:[[@LINE-7]]:1: note: class was declared abstract here [ClassAbstractHere] diff --git a/toolchain/check/testdata/class/fail_import_misuses.carbon b/toolchain/check/testdata/class/fail_import_misuses.carbon index c84dd256c6e2..87dfebd4ee24 100644 --- a/toolchain/check/testdata/class/fail_import_misuses.carbon +++ b/toolchain/check/testdata/class/fail_import_misuses.carbon @@ -34,7 +34,7 @@ import library "a"; class Empty { } -// CHECK:STDERR: fail_b.carbon:[[@LINE+8]]:8: error: variable has incomplete type `Incomplete` [IncompleteTypeInVarDecl] +// CHECK:STDERR: fail_b.carbon:[[@LINE+8]]:8: error: binding pattern has incomplete type `Incomplete` in name binding declaration [IncompleteTypeInBindingDecl] // CHECK:STDERR: var a: Incomplete; // CHECK:STDERR: ^~~~~~~~~~ // CHECK:STDERR: fail_b.carbon:[[@LINE-16]]:1: in import [InImport] diff --git a/toolchain/check/testdata/class/fail_incomplete.carbon b/toolchain/check/testdata/class/fail_incomplete.carbon index 23397c756b7a..8bf3513d06f3 100644 --- a/toolchain/check/testdata/class/fail_incomplete.carbon +++ b/toolchain/check/testdata/class/fail_incomplete.carbon @@ -34,7 +34,7 @@ fn CallClassFunction() { Class.Function(); } -// CHECK:STDERR: fail_forward_decl.carbon:[[@LINE+7]]:17: error: variable has incomplete type `Class` [IncompleteTypeInVarDecl] +// CHECK:STDERR: fail_forward_decl.carbon:[[@LINE+7]]:17: error: binding pattern has incomplete type `Class` in name binding declaration [IncompleteTypeInBindingDecl] // CHECK:STDERR: var global_var: Class; // CHECK:STDERR: ^~~~~ // CHECK:STDERR: fail_forward_decl.carbon:[[@LINE-25]]:1: note: class was forward declared here [ClassForwardDeclaredHere] @@ -86,7 +86,7 @@ fn Copy(p: Class*) -> Class { } fn Let(p: Class*) { - // CHECK:STDERR: fail_forward_decl.carbon:[[@LINE+7]]:10: error: `let` binding has incomplete type `Class` [IncompleteTypeInLetDecl] + // CHECK:STDERR: fail_forward_decl.carbon:[[@LINE+7]]:10: error: binding pattern has incomplete type `Class` in name binding declaration [IncompleteTypeInBindingDecl] // CHECK:STDERR: let c: Class = *p; // CHECK:STDERR: ^~~~~ // CHECK:STDERR: fail_forward_decl.carbon:[[@LINE-77]]:1: note: class was forward declared here [ClassForwardDeclaredHere] diff --git a/toolchain/check/testdata/struct/no_prelude/fail_nested_incomplete.carbon b/toolchain/check/testdata/struct/no_prelude/fail_nested_incomplete.carbon index c8da2a8e970a..f1156cda5098 100644 --- a/toolchain/check/testdata/struct/no_prelude/fail_nested_incomplete.carbon +++ b/toolchain/check/testdata/struct/no_prelude/fail_nested_incomplete.carbon @@ -10,7 +10,7 @@ class Incomplete; -// CHECK:STDERR: fail_nested_incomplete.carbon:[[@LINE+7]]:8: error: variable has incomplete type `{.a: Incomplete}` [IncompleteTypeInVarDecl] +// CHECK:STDERR: fail_nested_incomplete.carbon:[[@LINE+7]]:8: error: binding pattern has incomplete type `{.a: Incomplete}` in name binding declaration [IncompleteTypeInBindingDecl] // CHECK:STDERR: var s: {.a: Incomplete}; // CHECK:STDERR: ^~~~~~~~~~~~~~~~ // CHECK:STDERR: fail_nested_incomplete.carbon:[[@LINE-5]]:1: note: class was forward declared here [ClassForwardDeclaredHere] diff --git a/toolchain/check/testdata/tuple/fail_nested_incomplete.carbon b/toolchain/check/testdata/tuple/fail_nested_incomplete.carbon index 492f56561cea..36828a725a20 100644 --- a/toolchain/check/testdata/tuple/fail_nested_incomplete.carbon +++ b/toolchain/check/testdata/tuple/fail_nested_incomplete.carbon @@ -10,7 +10,7 @@ class Incomplete; -// CHECK:STDERR: fail_nested_incomplete.carbon:[[@LINE+7]]:8: error: variable has incomplete type `(i32, Incomplete)` [IncompleteTypeInVarDecl] +// CHECK:STDERR: fail_nested_incomplete.carbon:[[@LINE+7]]:8: error: binding pattern has incomplete type `(i32, Incomplete)` in name binding declaration [IncompleteTypeInBindingDecl] // CHECK:STDERR: var t: (i32, Incomplete); // CHECK:STDERR: ^~~~~~~~~~~~~~~~~ // CHECK:STDERR: fail_nested_incomplete.carbon:[[@LINE-5]]:1: note: class was forward declared here [ClassForwardDeclaredHere] diff --git a/toolchain/diagnostics/diagnostic_kind.def b/toolchain/diagnostics/diagnostic_kind.def index 412aaf1a85ab..08acf8a79a95 100644 --- a/toolchain/diagnostics/diagnostic_kind.def +++ b/toolchain/diagnostics/diagnostic_kind.def @@ -305,7 +305,7 @@ CARBON_DIAGNOSTIC_KIND(AbstractTypeInAdaptDecl) CARBON_DIAGNOSTIC_KIND(AbstractTypeInFieldDecl) CARBON_DIAGNOSTIC_KIND(AbstractTypeInFunctionReturnType) CARBON_DIAGNOSTIC_KIND(AbstractTypeInInit) -CARBON_DIAGNOSTIC_KIND(AbstractTypeInVarDecl) +CARBON_DIAGNOSTIC_KIND(AbstractTypeInVarPattern) CARBON_DIAGNOSTIC_KIND(AddrOfEphemeralRef) CARBON_DIAGNOSTIC_KIND(AddrOfNonRef) CARBON_DIAGNOSTIC_KIND(AddrOnNonSelfParam) @@ -344,15 +344,14 @@ CARBON_DIAGNOSTIC_KIND(RepeatedConst) CARBON_DIAGNOSTIC_KIND(IncompleteTypeInAdaptDecl) CARBON_DIAGNOSTIC_KIND(IncompleteTypeInAssociatedDecl) CARBON_DIAGNOSTIC_KIND(IncompleteTypeInBaseDecl) +CARBON_DIAGNOSTIC_KIND(IncompleteTypeInBindingDecl) CARBON_DIAGNOSTIC_KIND(IncompleteTypeInConversion) CARBON_DIAGNOSTIC_KIND(IncompleteTypeInFieldDecl) CARBON_DIAGNOSTIC_KIND(IncompleteTypeInFunctionParam) CARBON_DIAGNOSTIC_KIND(IncompleteTypeInFunctionReturnType) CARBON_DIAGNOSTIC_KIND(IncompleteTypeInMonomorphization) -CARBON_DIAGNOSTIC_KIND(IncompleteTypeInLetDecl) CARBON_DIAGNOSTIC_KIND(IncompleteTypeInMemberAccess) CARBON_DIAGNOSTIC_KIND(IncompleteTypeInValueConversion) -CARBON_DIAGNOSTIC_KIND(IncompleteTypeInVarDecl) CARBON_DIAGNOSTIC_KIND(InCopy) CARBON_DIAGNOSTIC_KIND(IntTooLargeForType) CARBON_DIAGNOSTIC_KIND(IntWidthNotMultipleOf8)