diff --git a/toolchain/check/decl_name_stack.cpp b/toolchain/check/decl_name_stack.cpp index 0b6fa8b25b70..8f831f79eb81 100644 --- a/toolchain/check/decl_name_stack.cpp +++ b/toolchain/check/decl_name_stack.cpp @@ -47,6 +47,15 @@ auto DeclNameStack::FinishName() -> NameContext { return result; } +auto DeclNameStack::FinishImplName() -> NameContext { + CARBON_CHECK(decl_name_stack_.back().state == NameContext::State::Empty) + << "Impl has a name"; + + NameContext result = decl_name_stack_.back(); + decl_name_stack_.back().state = NameContext::State::Finished; + return result; +} + auto DeclNameStack::PopScope() -> void { CARBON_CHECK(decl_name_stack_.back().state == NameContext::State::Finished) << "Missing call to FinishName before PopScope"; diff --git a/toolchain/check/decl_name_stack.h b/toolchain/check/decl_name_stack.h index 4d2f2b418d81..a7ca1600434b 100644 --- a/toolchain/check/decl_name_stack.h +++ b/toolchain/check/decl_name_stack.h @@ -91,11 +91,15 @@ class DeclNameStack { Error, }; + // Returns whether the name resolved to an existing entity. + auto is_resolved() -> bool { + return state != State::Unresolved && state != State::Empty; + } + // Returns the name_id for a new instruction. This is invalid when the name // resolved. auto name_id_for_new_inst() -> SemIR::NameId { - return state == State::Unresolved ? unresolved_name_id - : SemIR::NameId::Invalid; + return !is_resolved() ? unresolved_name_id : SemIR::NameId::Invalid; } // Returns the enclosing_scope_id for a new instruction. This is invalid @@ -103,8 +107,7 @@ class DeclNameStack { // the NameContext, which refers to the scope of the introducer rather than // the scope of the name. auto enclosing_scope_id_for_new_inst() -> SemIR::NameScopeId { - return state == State::Unresolved ? target_scope_id - : SemIR::NameScopeId::Invalid; + return !is_resolved() ? target_scope_id : SemIR::NameScopeId::Invalid; } // The current scope when this name began. This is the scope that we will @@ -141,6 +144,13 @@ class DeclNameStack { // state, `FinishName` and `PopScope` must be called, in that order. auto PushScopeAndStartName() -> void; + // Peeks the current target scope of the name on top of the stack. Note that + // if we're still processing the name qualifiers, this can change before the + // name is completed. + auto PeekTargetScope() -> SemIR::NameScopeId { + return decl_name_stack_.back().target_scope_id; + } + // Finishes the current declaration name processing, returning the final // context for adding the name to lookup. // @@ -148,6 +158,13 @@ class DeclNameStack { // which will be applied to the declaration name if appropriate. auto FinishName() -> NameContext; + // Finishes the current declaration name processing for an `impl`, returning + // the final context for adding the name to lookup. + // + // `impl`s don't actually have names, but want the rest of the name processing + // logic such as building parameter scopes, so are a special case. + auto FinishImplName() -> NameContext; + // Pops the declaration name from the declaration name stack, and pops all // scopes that were entered as part of handling the declaration name. These // are the scopes corresponding to name qualifiers in the name, for example diff --git a/toolchain/check/handle_impl.cpp b/toolchain/check/handle_impl.cpp index 023eb3a36dd0..e313bb16ca30 100644 --- a/toolchain/check/handle_impl.cpp +++ b/toolchain/check/handle_impl.cpp @@ -4,6 +4,7 @@ #include "toolchain/check/context.h" #include "toolchain/check/convert.h" +#include "toolchain/check/decl_name_stack.h" #include "toolchain/check/modifiers.h" #include "toolchain/sem_ir/ids.h" #include "toolchain/sem_ir/typed_insts.h" @@ -17,15 +18,15 @@ auto HandleImplIntroducer(Context& context, Parse::ImplIntroducerId parse_node) context.inst_block_stack().Push(); // Push the bracketing node. - context.node_stack().Push(parse_node, - context.scope_stack().PeekNameScopeId()); + context.node_stack().Push(parse_node); // Optional modifiers follow. context.decl_state_stack().Push(DeclState::Impl); - // Create a scope for implicit parameters. We may not use it, but it's simpler - // to create it unconditionally than to track whether it exists. - context.scope_stack().Push(); + // An impl doesn't have a name per se, but it makes the processing more + // consistent to imagine that it does. This also gives us a scope for implicit + // parameters. + context.decl_name_stack().PushScopeAndStartName(); return true; } @@ -68,8 +69,7 @@ auto HandleDefaultSelfImplAs(Context& context, // TODO: Do this without modifying the node stack. auto implicit_param_list = context.node_stack().PopWithParseNodeIf(); - auto enclosing_scope_id = - context.node_stack().Peek(); + auto enclosing_scope_id = context.decl_name_stack().PeekTargetScope(); if (implicit_param_list) { context.node_stack().Push(implicit_param_list->first, implicit_param_list->second); @@ -90,8 +90,9 @@ auto HandleDefaultSelfImplAs(Context& context, // Process an `extend impl` declaration by extending the impl scope with the // `impl`'s scope. static auto ExtendImpl(Context& context, Parse::AnyImplDeclId parse_node, - SemIR::TypeId constraint_id, - SemIR::NameScopeId enclosing_scope_id) -> void { + SemIR::TypeId constraint_id) -> void { + auto enclosing_scope_id = context.decl_name_stack().PeekTargetScope(); + // TODO: This is also valid in a mixin. if (!TryAsClassScope(context, enclosing_scope_id)) { CARBON_DIAGNOSTIC(ExtendImplOutsideClass, Error, @@ -120,26 +121,17 @@ static auto ExtendImpl(Context& context, Parse::AnyImplDeclId parse_node, enclosing_scope.extended_scopes.push_back(interface.scope_id); } -namespace { -struct BuildImplDeclResult { - SemIR::ImplId impl_id; - SemIR::InstId impl_decl_id; - SemIR::NameScopeId enclosing_scope_id; -}; -} // namespace - // Build an ImplDecl describing the signature of an impl. This handles the // common logic shared by impl forward declarations and impl definitions. static auto BuildImplDecl(Context& context, Parse::AnyImplDeclId parse_node) - -> BuildImplDeclResult { + -> std::pair { auto [constraint_node, constraint_id] = context.node_stack().PopExprWithParseNode(); auto self_type_id = context.node_stack().Pop(); auto params_id = context.node_stack().PopIf(); auto decl_block_id = context.inst_block_stack().Pop(); - auto enclosing_scope_id = - context.node_stack().Pop(); + context.node_stack().PopForSoloParseNode(); // Convert the constraint expression to a type. // TODO: Check that its constant value is a constraint. @@ -151,6 +143,11 @@ static auto BuildImplDecl(Context& context, Parse::AnyImplDeclId parse_node) LimitModifiersOnDecl(context, KeywordModifierSet::ImplDecl, Lex::TokenKind::Impl); + // 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); + // Add the impl declaration. auto impl_decl = SemIR::ImplDecl{SemIR::ImplId::Invalid, decl_block_id}; auto impl_decl_id = context.AddPlaceholderInst({parse_node, impl_decl}); @@ -172,25 +169,24 @@ static auto BuildImplDecl(Context& context, Parse::AnyImplDeclId parse_node) KeywordModifierSet::Extend)) { // TODO: Diagnose combining `extend` with `forall`. // TODO: Diagnose combining `extend` with an explicit self type. - ExtendImpl(context, parse_node, constraint_type_id, enclosing_scope_id); + ExtendImpl(context, parse_node, constraint_type_id); } context.decl_state_stack().Pop(DeclState::Impl); - return {impl_decl.impl_id, impl_decl_id, enclosing_scope_id}; + return {impl_decl.impl_id, impl_decl_id}; } auto HandleImplDecl(Context& context, Parse::ImplDeclId parse_node) -> bool { BuildImplDecl(context, parse_node); - context.scope_stack().Pop(); + context.decl_name_stack().PopScope(); return true; } auto HandleImplDefinitionStart(Context& context, Parse::ImplDefinitionStartId parse_node) -> bool { - auto [impl_id, impl_decl_id, enclosing_scope_id] = - BuildImplDecl(context, parse_node); + auto [impl_id, impl_decl_id] = BuildImplDecl(context, parse_node); auto& impl_info = context.impls().Get(impl_id); if (impl_info.definition_id.is_valid()) { @@ -207,8 +203,9 @@ auto HandleImplDefinitionStart(Context& context, .Emit(); } else { impl_info.definition_id = impl_decl_id; - impl_info.scope_id = context.name_scopes().Add( - impl_decl_id, SemIR::NameId::Invalid, enclosing_scope_id); + impl_info.scope_id = + context.name_scopes().Add(impl_decl_id, SemIR::NameId::Invalid, + context.decl_name_stack().PeekTargetScope()); } context.scope_stack().Push(impl_decl_id, impl_info.scope_id); @@ -234,9 +231,7 @@ auto HandleImplDefinition(Context& context, auto impl_id = context.node_stack().Pop(); context.inst_block_stack().Pop(); - // Pop the impl body scope and the parameter scope. - context.scope_stack().Pop(); - context.scope_stack().Pop(); + context.decl_name_stack().PopScope(); // The impl is now fully defined. context.impls().Get(impl_id).defined = true; diff --git a/toolchain/check/node_stack.h b/toolchain/check/node_stack.h index f3561e9af83f..f99656c74207 100644 --- a/toolchain/check/node_stack.h +++ b/toolchain/check/node_stack.h @@ -356,7 +356,7 @@ class NodeStack { // that the parse node should not appear in the node stack at all. using Id = IdUnion; + SemIR::NameId, SemIR::TypeId>; // An entry in stack_. struct Entry { @@ -452,8 +452,6 @@ class NodeStack { return Id::KindFor(); case Parse::NodeKind::SelfValueName: return Id::KindFor(); - case Parse::NodeKind::ImplIntroducer: - return Id::KindFor(); case Parse::NodeKind::ArrayExprSemi: case Parse::NodeKind::ClassIntroducer: case Parse::NodeKind::CodeBlockStart: @@ -461,6 +459,7 @@ class NodeStack { case Parse::NodeKind::FunctionIntroducer: case Parse::NodeKind::IfStatementElse: case Parse::NodeKind::ImplicitParamListStart: + case Parse::NodeKind::ImplIntroducer: case Parse::NodeKind::InterfaceIntroducer: case Parse::NodeKind::LetIntroducer: case Parse::NodeKind::QualifiedName: