diff --git a/toolchain/check/decl_name_stack.cpp b/toolchain/check/decl_name_stack.cpp index 628690f7d1b4..483164a8d554 100644 --- a/toolchain/check/decl_name_stack.cpp +++ b/toolchain/check/decl_name_stack.cpp @@ -10,6 +10,29 @@ namespace Carbon::Check { +auto DeclNameStack::NameContext::prev_inst_id() -> SemIR::InstId { + switch (state) { + case NameContext::State::Error: + // The name is invalid and a diagnostic has already been emitted. + return SemIR::InstId::Invalid; + + case NameContext::State::Empty: + CARBON_FATAL() + << "Name is missing, not expected to call existing_inst_id (but " + "that may change based on error handling)."; + + case NameContext::State::Resolved: + case NameContext::State::ResolvedNonScope: + return resolved_inst_id; + + case NameContext::State::Unresolved: + return SemIR::InstId::Invalid; + + case NameContext::State::Finished: + CARBON_FATAL() << "Finished state should only be used internally"; + } +} + auto DeclNameStack::MakeEmptyNameContext() -> NameContext { return NameContext{ .enclosing_scope = context_->scope_stack().PeekIndex(), @@ -94,20 +117,11 @@ auto DeclNameStack::Restore(SuspendedName sus) -> void { } } -auto DeclNameStack::LookupOrAddName(NameContext name_context, - SemIR::InstId target_id) -> SemIR::InstId { +auto DeclNameStack::AddName(NameContext name_context, SemIR::InstId target_id) + -> void { switch (name_context.state) { case NameContext::State::Error: - // The name is invalid and a diagnostic has already been emitted. - return SemIR::InstId::Invalid; - - case NameContext::State::Empty: - CARBON_FATAL() << "Name is missing, not expected to call AddNameToLookup " - "(but that may change based on error handling)."; - - case NameContext::State::Resolved: - case NameContext::State::ResolvedNonScope: - return name_context.resolved_inst_id; + return; case NameContext::State::Unresolved: if (!name_context.target_scope_id.is_valid()) { @@ -141,21 +155,33 @@ auto DeclNameStack::LookupOrAddName(NameContext name_context, << name_context.unresolved_name_id << " in " << name_context.target_scope_id; } - return SemIR::InstId::Invalid; + break; - case NameContext::State::Finished: - CARBON_FATAL() << "Finished state should only be used internally"; + default: + CARBON_FATAL() << "Should not be calling AddName"; + break; } } -auto DeclNameStack::AddNameToLookup(NameContext name_context, - SemIR::InstId target_id) -> void { - auto existing_inst_id = LookupOrAddName(name_context, target_id); - if (existing_inst_id.is_valid()) { - context_->DiagnoseDuplicateName(target_id, existing_inst_id); +auto DeclNameStack::AddNameOrDiagnoseDuplicate(NameContext name_context, + SemIR::InstId target_id) + -> void { + if (auto id = name_context.prev_inst_id(); id.is_valid()) { + context_->DiagnoseDuplicateName(target_id, id); + } else { + AddName(name_context, target_id); } } +auto DeclNameStack::LookupOrAddName(NameContext name_context, + SemIR::InstId target_id) -> SemIR::InstId { + if (auto id = name_context.prev_inst_id(); id.is_valid()) { + return id; + } + AddName(name_context, target_id); + return SemIR::InstId::Invalid; +} + auto DeclNameStack::ApplyNameQualifier(SemIR::LocId loc_id, SemIR::NameId name_id) -> void { ApplyNameQualifierTo(decl_name_stack_.back(), loc_id, name_id, diff --git a/toolchain/check/decl_name_stack.h b/toolchain/check/decl_name_stack.h index 3dc33485d8c5..82d178be7d7c 100644 --- a/toolchain/check/decl_name_stack.h +++ b/toolchain/check/decl_name_stack.h @@ -91,15 +91,14 @@ class DeclNameStack { Error, }; - // Returns whether the name resolved to an existing entity. - auto is_resolved() -> bool { - return state != State::Unresolved && state != State::Empty; - } + // Returns any name collision found, or Invalid. + auto prev_inst_id() -> SemIR::InstId; // Returns the name_id for a new instruction. This is invalid when the name // resolved. auto name_id_for_new_inst() -> SemIR::NameId { - return !is_resolved() ? unresolved_name_id : SemIR::NameId::Invalid; + return state == State::Unresolved ? unresolved_name_id + : SemIR::NameId::Invalid; } // Returns the enclosing_scope_id for a new instruction. This is invalid @@ -107,7 +106,8 @@ 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 !is_resolved() ? target_scope_id : SemIR::NameScopeId::Invalid; + return state == State::Unresolved ? target_scope_id + : SemIR::NameScopeId::Invalid; } // The current scope when this name began. This is the scope that we will @@ -130,10 +130,10 @@ class DeclNameStack { union { // The ID of a resolved qualifier, including both identifiers and // expressions. Invalid indicates resolution failed. - SemIR::InstId resolved_inst_id = SemIR::InstId::Invalid; + SemIR::InstId resolved_inst_id; // The ID of an unresolved identifier. - SemIR::NameId unresolved_name_id; + SemIR::NameId unresolved_name_id = SemIR::NameId::Invalid; }; }; @@ -214,9 +214,12 @@ class DeclNameStack { // describes an existing scope, such as a namespace or a defined class. auto ApplyNameQualifier(SemIR::LocId loc_id, SemIR::NameId name_id) -> void; + // Adds a name to name lookup. Assumes duplicates are already handled. + auto AddName(NameContext name_context, SemIR::InstId target_id) -> void; + // Adds a name to name lookup. Prints a diagnostic for name conflicts. - auto AddNameToLookup(NameContext name_context, SemIR::InstId target_id) - -> void; + auto AddNameOrDiagnoseDuplicate(NameContext name_context, + SemIR::InstId target_id) -> void; // Adds a name to name lookup, or returns the existing instruction if this // name has already been declared in this scope. diff --git a/toolchain/check/handle_alias.cpp b/toolchain/check/handle_alias.cpp index 1ce672abc0ef..a2a10e3b9379 100644 --- a/toolchain/check/handle_alias.cpp +++ b/toolchain/check/handle_alias.cpp @@ -69,7 +69,7 @@ auto HandleAlias(Context& context, Parse::AliasId /*node_id*/) -> bool { // Add the name of the binding to the current scope. context.decl_name_stack().PopScope(); - context.decl_name_stack().AddNameToLookup(name_context, alias_id); + context.decl_name_stack().AddNameOrDiagnoseDuplicate(name_context, alias_id); return true; } diff --git a/toolchain/check/handle_class.cpp b/toolchain/check/handle_class.cpp index c40907eaed4e..2d655ffd382d 100644 --- a/toolchain/check/handle_class.cpp +++ b/toolchain/check/handle_class.cpp @@ -435,7 +435,7 @@ auto HandleBaseDecl(Context& context, Parse::BaseDeclId node_id) -> bool { SemIR::StructTypeField{SemIR::NameId::Base, base_info.type_id}})); // Bind the name `base` in the class to the base field. - context.decl_name_stack().AddNameToLookup( + context.decl_name_stack().AddNameOrDiagnoseDuplicate( context.decl_name_stack().MakeUnqualifiedName(node_id, SemIR::NameId::Base), class_info.base_id); diff --git a/toolchain/check/handle_function.cpp b/toolchain/check/handle_function.cpp index 6e59a90e50f2..0fdf5f233df8 100644 --- a/toolchain/check/handle_function.cpp +++ b/toolchain/check/handle_function.cpp @@ -132,24 +132,8 @@ static auto BuildFunctionDecl(Context& context, function_info.definition_id = function_info.decl_id; } - // At interface scope, a function declaration introduces an associated - // function. - auto lookup_result_id = function_info.decl_id; - if (name_context.enclosing_scope_id_for_new_inst().is_valid() && - !name_context.has_qualifiers) { - auto scope_inst_id = context.name_scopes().GetInstIdIfValid( - name_context.enclosing_scope_id_for_new_inst()); - if (auto interface_scope = - context.insts().TryGetAsIfValid( - scope_inst_id)) { - lookup_result_id = BuildAssociatedEntity( - context, interface_scope->interface_id, function_info.decl_id); - } - } - // Check whether this is a redeclaration. - auto prev_id = - context.decl_name_stack().LookupOrAddName(name_context, lookup_result_id); + auto prev_id = name_context.prev_inst_id(); if (prev_id.is_valid()) { auto prev_inst_for_merge = ResolvePrevInstForMerge(context, node_id, prev_id); @@ -179,6 +163,27 @@ static auto BuildFunctionDecl(Context& context, // Write the function ID into the FunctionDecl. context.ReplaceInstBeforeConstantUse(function_info.decl_id, function_decl); + // Check if we need to add this to name lookup, now that the function decl is + // done. + if (!prev_id.is_valid()) { + // At interface scope, a function declaration introduces an associated + // function. + auto lookup_result_id = function_info.decl_id; + if (name_context.enclosing_scope_id_for_new_inst().is_valid() && + !name_context.has_qualifiers) { + auto scope_inst_id = context.name_scopes().GetInstIdIfValid( + name_context.enclosing_scope_id_for_new_inst()); + if (auto interface_scope = + context.insts().TryGetAsIfValid( + scope_inst_id)) { + lookup_result_id = BuildAssociatedEntity( + context, interface_scope->interface_id, function_info.decl_id); + } + } + + context.decl_name_stack().AddName(name_context, lookup_result_id); + } + if (SemIR::IsEntryPoint(context.sem_ir(), function_decl.function_id)) { // TODO: Update this once valid signatures for the entry point are decided. if (!context.inst_blocks().Get(implicit_param_refs_id).empty() || diff --git a/toolchain/check/handle_let.cpp b/toolchain/check/handle_let.cpp index fcba3ea5cea8..2db3c4c60afe 100644 --- a/toolchain/check/handle_let.cpp +++ b/toolchain/check/handle_let.cpp @@ -56,7 +56,7 @@ static auto BuildAssociatedConstantDecl( auto assoc_id = BuildAssociatedEntity(context, interface_id, decl_id); auto name_context = context.decl_name_stack().MakeUnqualifiedName(pattern.loc_id, name_id); - context.decl_name_stack().AddNameToLookup(name_context, assoc_id); + context.decl_name_stack().AddNameOrDiagnoseDuplicate(name_context, assoc_id); } auto HandleLetDecl(Context& context, Parse::LetDeclId node_id) -> bool { diff --git a/toolchain/check/handle_variable.cpp b/toolchain/check/handle_variable.cpp index 5770d091b69d..6afe8a0769f4 100644 --- a/toolchain/check/handle_variable.cpp +++ b/toolchain/check/handle_variable.cpp @@ -57,14 +57,16 @@ auto HandleVariableDecl(Context& context, Parse::VariableDeclId node_id) auto name_context = context.decl_name_stack().MakeUnqualifiedName( context.insts().GetLocId(value_id), context.bind_names().Get(bind_name->bind_name_id).name_id); - context.decl_name_stack().AddNameToLookup(name_context, value_id); + context.decl_name_stack().AddNameOrDiagnoseDuplicate(name_context, + value_id); value_id = bind_name->value_id; } else if (auto field_decl = context.insts().TryGetAs(value_id)) { // Introduce the field name into the class. auto name_context = context.decl_name_stack().MakeUnqualifiedName( context.insts().GetLocId(value_id), field_decl->name_id); - context.decl_name_stack().AddNameToLookup(name_context, value_id); + context.decl_name_stack().AddNameOrDiagnoseDuplicate(name_context, + value_id); } // TODO: Handle other kinds of pattern.