From 28cefe98df0972efa3f719a74204962014be545a Mon Sep 17 00:00:00 2001 From: Richard Smith Date: Thu, 30 May 2024 09:05:35 -0700 Subject: [PATCH] Factor out pushing / popping of names plus parameters. (#4005) Previously we did some of this in decl_name_stack and some of it in the callers of decl_name_stack. Factor out a single place to pop a name and its optional following parameters. Part of making this behavior consistent is that we now track whether an implicit parameter list was present or absent rather than mapping an absent list to `InstBlockId::Empty`. This improves our redeclaration checking and the precision of some diagnostics. --- toolchain/check/BUILD | 2 + toolchain/check/context.cpp | 2 +- toolchain/check/decl_name_stack.cpp | 30 +++------- toolchain/check/decl_name_stack.h | 58 +++++++++---------- toolchain/check/handle_alias.cpp | 4 +- toolchain/check/handle_class.cpp | 15 ++--- toolchain/check/handle_export.cpp | 4 +- toolchain/check/handle_function.cpp | 36 +++++------- toolchain/check/handle_interface.cpp | 9 ++- toolchain/check/handle_name.cpp | 15 +---- toolchain/check/handle_namespace.cpp | 4 +- toolchain/check/member_access.cpp | 2 +- toolchain/check/name_component.cpp | 50 ++++++++++++++++ toolchain/check/name_component.h | 42 ++++++++++++++ .../alias/no_prelude/fail_params.carbon | 2 +- .../class/fail_todo_generic_method.carbon | 36 ++++++++++-- .../fail_todo_member_out_of_line.carbon | 35 +++++++++-- .../impl/fail_impl_bad_assoc_fn.carbon | 8 +-- .../no_prelude/fail_todo_generic.carbon | 4 -- .../testdata/namespace/fail_params.carbon | 6 +- .../no_prelude/fail_export_name_params.carbon | 2 +- toolchain/lower/file_context.cpp | 9 +-- toolchain/sem_ir/formatter.cpp | 10 ++-- toolchain/sem_ir/inst.h | 2 +- 24 files changed, 249 insertions(+), 138 deletions(-) create mode 100644 toolchain/check/name_component.cpp create mode 100644 toolchain/check/name_component.h diff --git a/toolchain/check/BUILD b/toolchain/check/BUILD index 7b50435c9280..92f03d41c523 100644 --- a/toolchain/check/BUILD +++ b/toolchain/check/BUILD @@ -59,6 +59,7 @@ cc_library( "inst_block_stack.cpp", "merge.cpp", "modifiers.cpp", + "name_component.cpp", "return.cpp", "subst.cpp", ], @@ -75,6 +76,7 @@ cc_library( "keyword_modifier_set.h", "merge.h", "modifiers.h", + "name_component.h", "param_and_arg_refs_stack.h", "pending_block.h", "return.h", diff --git a/toolchain/check/context.cpp b/toolchain/check/context.cpp index 58fa95fc97f8..29a774d52fd1 100644 --- a/toolchain/check/context.cpp +++ b/toolchain/check/context.cpp @@ -626,7 +626,7 @@ auto Context::FinalizeGlobalInit() -> void { {.name_id = SemIR::NameId::ForIdentifier(name_id), .enclosing_scope_id = SemIR::NameScopeId::Package, .decl_id = SemIR::InstId::Invalid, - .implicit_param_refs_id = SemIR::InstBlockId::Empty, + .implicit_param_refs_id = SemIR::InstBlockId::Invalid, .param_refs_id = SemIR::InstBlockId::Empty, .return_type_id = SemIR::TypeId::Invalid, .return_storage_id = SemIR::InstId::Invalid, diff --git a/toolchain/check/decl_name_stack.cpp b/toolchain/check/decl_name_stack.cpp index 4ddf3d9bfa26..aa101613aa21 100644 --- a/toolchain/check/decl_name_stack.cpp +++ b/toolchain/check/decl_name_stack.cpp @@ -55,28 +55,12 @@ auto DeclNameStack::PushScopeAndStartName() -> void { context_->scope_stack().Push(); } -auto DeclNameStack::FinishName() -> NameContext { +auto DeclNameStack::FinishName(const NameComponent& name) -> NameContext { CARBON_CHECK(decl_name_stack_.back().state != NameContext::State::Finished) << "Finished name twice"; - auto [params_loc_id, params_id] = - context_->node_stack().PopWithNodeIdIf(); - auto [implicit_params_loc_id, implicit_params_id] = - context_->node_stack() - .PopWithNodeIdIf(); - auto [loc_id, name_id] = context_->node_stack().PopNameWithNodeId(); - ApplyNameQualifier(loc_id, name_id); - - if (params_id || implicit_params_id) { - // TODO: Say which kind of declaration we're parsing. - CARBON_DIAGNOSTIC(UnexpectedDeclNameParams, Error, - "Declaration cannot have parameters."); - // Point to the lexically first parameter list in the diagnostic. - context_->emitter().Emit(implicit_params_id - ? static_cast(implicit_params_loc_id) - : params_loc_id, - UnexpectedDeclNameParams); - } + ApplyNameQualifierTo(decl_name_stack_.back(), name.name_loc_id, name.name_id, + /*is_unqualified=*/false); NameContext result = decl_name_stack_.back(); decl_name_stack_.back().state = NameContext::State::Finished; @@ -194,9 +178,11 @@ auto DeclNameStack::LookupOrAddName(NameContext name_context, 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, +auto DeclNameStack::ApplyNameQualifier(const NameComponent& name) -> void { + if (name.implicit_params_id.is_valid() || name.params_id.is_valid()) { + context_->TODO(name.params_loc_id, "name qualifier with parameters"); + } + ApplyNameQualifierTo(decl_name_stack_.back(), name.name_loc_id, name.name_id, /*is_unqualified=*/false); } diff --git a/toolchain/check/decl_name_stack.h b/toolchain/check/decl_name_stack.h index d087f08b1b35..1b4d445351bb 100644 --- a/toolchain/check/decl_name_stack.h +++ b/toolchain/check/decl_name_stack.h @@ -6,6 +6,7 @@ #define CARBON_TOOLCHAIN_CHECK_DECL_NAME_STACK_H_ #include "llvm/ADT/SmallVector.h" +#include "toolchain/check/name_component.h" #include "toolchain/check/scope_index.h" #include "toolchain/check/scope_stack.h" #include "toolchain/sem_ir/ids.h" @@ -157,26 +158,23 @@ class DeclNameStack { // state, `FinishName` and `PopScope` must be called, in that order. auto PushScopeAndStartName() -> void; - // Peeks the current enclosing 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. Also, if the name up to this point was already - // declared and is a scope, this will be that scope, rather than the scope - // enclosing it. - auto PeekEnclosingScopeId() const -> SemIR::NameScopeId { - return decl_name_stack_.back().enclosing_scope_id; - } + // Creates and returns a name context corresponding to declaring an + // unqualified name in the current context. This is suitable for adding to + // name lookup in situations where a qualified name is not permitted, such as + // a pattern binding. + auto MakeUnqualifiedName(SemIR::LocId loc_id, SemIR::NameId name_id) + -> NameContext; - // Peeks the resolution scope index of the name on top of the stack. - auto PeekInitialScopeIndex() const -> ScopeIndex { - return decl_name_stack_.back().initial_scope_index; - } + // Applies a name component as a qualifier for the current name. This will + // enter the scope corresponding to the name if the name describes an existing + // scope, such as a namespace or a defined class. + auto ApplyNameQualifier(const NameComponent& name) -> void; // Finishes the current declaration name processing, returning the final - // context for adding the name to lookup. - // - // This also pops the final name instruction from the instruction stack, - // which will be applied to the declaration name if appropriate. - auto FinishName() -> NameContext; + // context for adding the name to lookup. The final name component should be + // popped and passed to this function, and will be added to the declaration + // name. + auto FinishName(const NameComponent& name) -> NameContext; // Finishes the current declaration name processing for an `impl`, returning // the final context for adding the name to lookup. @@ -193,6 +191,20 @@ class DeclNameStack { // This should be called at the end of the declaration. auto PopScope() -> void; + // Peeks the current enclosing 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. Also, if the name up to this point was already + // declared and is a scope, this will be that scope, rather than the scope + // enclosing it. + auto PeekEnclosingScopeId() const -> SemIR::NameScopeId { + return decl_name_stack_.back().enclosing_scope_id; + } + + // Peeks the resolution scope index of the name on top of the stack. + auto PeekInitialScopeIndex() const -> ScopeIndex { + return decl_name_stack_.back().initial_scope_index; + } + // Temporarily remove the current declaration name and its associated scopes // from the stack. Can only be called once the name is finished. auto Suspend() -> SuspendedName; @@ -200,18 +212,6 @@ class DeclNameStack { // Restore a previously suspended name. auto Restore(SuspendedName sus) -> void; - // Creates and returns a name context corresponding to declaring an - // unqualified name in the current context. This is suitable for adding to - // name lookup in situations where a qualified name is not permitted, such as - // a pattern binding. - auto MakeUnqualifiedName(SemIR::LocId loc_id, SemIR::NameId name_id) - -> NameContext; - - // Applies a Name from the name stack to the top of the declaration name - // stack. This will enter the scope corresponding to the name if the name - // 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; diff --git a/toolchain/check/handle_alias.cpp b/toolchain/check/handle_alias.cpp index 864dc3135a6b..9e402f3e3840 100644 --- a/toolchain/check/handle_alias.cpp +++ b/toolchain/check/handle_alias.cpp @@ -4,6 +4,7 @@ #include "toolchain/check/context.h" #include "toolchain/check/modifiers.h" +#include "toolchain/check/name_component.h" #include "toolchain/parse/node_ids.h" #include "toolchain/sem_ir/ids.h" #include "toolchain/sem_ir/typed_insts.h" @@ -25,7 +26,8 @@ auto HandleAliasInitializer(Context& /*context*/, auto HandleAlias(Context& context, Parse::AliasId /*node_id*/) -> bool { auto [expr_node, expr_id] = context.node_stack().PopExprWithNodeId(); - auto name_context = context.decl_name_stack().FinishName(); + auto name_context = context.decl_name_stack().FinishName( + PopNameComponentWithoutParams(context, Lex::TokenKind::Alias)); LimitModifiersOnDecl(context, KeywordModifierSet::Access, Lex::TokenKind::Alias); diff --git a/toolchain/check/handle_class.cpp b/toolchain/check/handle_class.cpp index 9aaa3b623065..74afd4d3ba52 100644 --- a/toolchain/check/handle_class.cpp +++ b/toolchain/check/handle_class.cpp @@ -9,6 +9,7 @@ #include "toolchain/check/eval.h" #include "toolchain/check/merge.h" #include "toolchain/check/modifiers.h" +#include "toolchain/check/name_component.h" #include "toolchain/sem_ir/ids.h" #include "toolchain/sem_ir/typed_insts.h" @@ -176,14 +177,8 @@ static auto MergeOrAddName(Context& context, Parse::AnyClassDeclId node_id, static auto BuildClassDecl(Context& context, Parse::AnyClassDeclId node_id, bool is_definition) -> std::tuple { - auto param_refs_id = - context.node_stack().PopIf().value_or( - SemIR::InstBlockId::Invalid); - auto implicit_param_refs_id = - context.node_stack().PopIf().value_or( - SemIR::InstBlockId::Invalid); - - auto name_context = context.decl_name_stack().FinishName(); + auto name = PopNameComponent(context); + auto name_context = context.decl_name_stack().FinishName(name); context.node_stack() .PopAndDiscardSoloNodeId(); @@ -222,8 +217,8 @@ static auto BuildClassDecl(Context& context, Parse::AnyClassDeclId node_id, SemIR::Class class_info = { .name_id = name_context.name_id_for_new_inst(), .enclosing_scope_id = name_context.enclosing_scope_id_for_new_inst(), - .implicit_param_refs_id = implicit_param_refs_id, - .param_refs_id = param_refs_id, + .implicit_param_refs_id = name.implicit_params_id, + .param_refs_id = name.params_id, // `.self_type_id` depends on the ClassType, so is set below. .self_type_id = SemIR::TypeId::Invalid, .decl_id = class_decl_id, diff --git a/toolchain/check/handle_export.cpp b/toolchain/check/handle_export.cpp index 6d3697e9e1fa..db2c589ce208 100644 --- a/toolchain/check/handle_export.cpp +++ b/toolchain/check/handle_export.cpp @@ -5,6 +5,7 @@ #include "toolchain/check/context.h" #include "toolchain/check/decl_name_stack.h" #include "toolchain/check/modifiers.h" +#include "toolchain/check/name_component.h" #include "toolchain/parse/typed_nodes.h" #include "toolchain/sem_ir/ids.h" #include "toolchain/sem_ir/typed_insts.h" @@ -20,7 +21,8 @@ auto HandleExportIntroducer(Context& context, } auto HandleExportDecl(Context& context, Parse::ExportDeclId node_id) -> bool { - auto name_context = context.decl_name_stack().FinishName(); + auto name_context = context.decl_name_stack().FinishName( + PopNameComponentWithoutParams(context, Lex::TokenKind::Export)); context.decl_name_stack().PopScope(); LimitModifiersOnDecl(context, KeywordModifierSet::None, diff --git a/toolchain/check/handle_function.cpp b/toolchain/check/handle_function.cpp index 2012929457be..1c8521397cab 100644 --- a/toolchain/check/handle_function.cpp +++ b/toolchain/check/handle_function.cpp @@ -11,6 +11,7 @@ #include "toolchain/check/interface.h" #include "toolchain/check/merge.h" #include "toolchain/check/modifiers.h" +#include "toolchain/check/name_component.h" #include "toolchain/parse/tree_node_diagnostic_converter.h" #include "toolchain/sem_ir/builtin_function_kind.h" #include "toolchain/sem_ir/entry_point.h" @@ -211,18 +212,12 @@ static auto BuildFunctionDecl(Context& context, return_slot = SemIR::Function::ReturnSlot::Absent; } - auto param_refs_id = - context.node_stack().PopIf(); - if (!param_refs_id) { + auto name = PopNameComponent(context); + if (!name.params_id.is_valid()) { context.TODO(node_id, "function with positional parameters"); - param_refs_id = SemIR::InstBlockId::Empty; + name.params_id = SemIR::InstBlockId::Empty; } - // TODO: Use Invalid rather than Empty if there was no implicit parameter - // list. - SemIR::InstBlockId implicit_param_refs_id = - context.node_stack().PopIf().value_or( - SemIR::InstBlockId::Empty); - auto name_context = context.decl_name_stack().FinishName(); + auto name_context = context.decl_name_stack().FinishName(name); context.node_stack() .PopAndDiscardSoloNodeId(); @@ -256,8 +251,8 @@ static auto BuildFunctionDecl(Context& context, .name_id = name_context.name_id_for_new_inst(), .enclosing_scope_id = name_context.enclosing_scope_id_for_new_inst(), .decl_id = context.AddPlaceholderInst({node_id, function_decl}), - .implicit_param_refs_id = implicit_param_refs_id, - .param_refs_id = *param_refs_id, + .implicit_param_refs_id = name.implicit_params_id, + .param_refs_id = name.params_id, .return_type_id = return_type_id, .return_storage_id = return_storage_id, .is_extern = is_extern, @@ -301,9 +296,8 @@ static auto BuildFunctionDecl(Context& context, 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(function_info.implicit_param_refs_id) - .empty() || + if (function_info.implicit_param_refs_id.is_valid() || + !function_info.param_refs_id.is_valid() || !context.inst_blocks().Get(function_info.param_refs_id).empty() || (function_info.return_type_id.is_valid() && function_info.return_type_id != @@ -344,9 +338,9 @@ static auto HandleFunctionDefinitionAfterSignature( CheckFunctionReturnType(context, function.return_storage_id, function); // Check the parameter types are complete. - for (auto param_id : llvm::concat( - context.inst_blocks().Get(function.implicit_param_refs_id), - context.inst_blocks().Get(function.param_refs_id))) { + for (auto param_id : llvm::concat( + context.inst_blocks().GetOrEmpty(function.implicit_param_refs_id), + context.inst_blocks().GetOrEmpty(function.param_refs_id))) { auto param = context.insts().Get(param_id); // Find the parameter in the pattern. @@ -464,11 +458,11 @@ static auto IsValidBuiltinDeclaration(Context& context, // Form the list of parameter types for the declaration. llvm::SmallVector param_type_ids; auto implicit_param_refs = - context.inst_blocks().Get(function.implicit_param_refs_id); - auto param_refs = context.inst_blocks().Get(function.param_refs_id); + context.inst_blocks().GetOrEmpty(function.implicit_param_refs_id); + auto param_refs = context.inst_blocks().GetOrEmpty(function.param_refs_id); param_type_ids.reserve(implicit_param_refs.size() + param_refs.size()); for (auto param_id : - llvm::concat(implicit_param_refs, param_refs)) { + llvm::concat(implicit_param_refs, param_refs)) { // TODO: We also need to track whether the parameter is declared with // `var`. param_type_ids.push_back(context.insts().Get(param_id).type_id()); diff --git a/toolchain/check/handle_interface.cpp b/toolchain/check/handle_interface.cpp index 034106b92fb7..39b780786b62 100644 --- a/toolchain/check/handle_interface.cpp +++ b/toolchain/check/handle_interface.cpp @@ -5,6 +5,7 @@ #include "toolchain/check/context.h" #include "toolchain/check/interface.h" #include "toolchain/check/modifiers.h" +#include "toolchain/check/name_component.h" #include "toolchain/sem_ir/typed_insts.h" namespace Carbon::Check { @@ -25,14 +26,12 @@ auto HandleInterfaceIntroducer(Context& context, static auto BuildInterfaceDecl(Context& context, Parse::AnyInterfaceDeclId node_id) -> std::tuple { - if (context.node_stack().PopIf()) { - context.TODO(node_id, "generic interface"); - } - if (context.node_stack().PopIf()) { + auto name = PopNameComponent(context); + if (name.params_id.is_valid() || name.implicit_params_id.is_valid()) { context.TODO(node_id, "generic interface"); } - auto name_context = context.decl_name_stack().FinishName(); + auto name_context = context.decl_name_stack().FinishName(name); context.node_stack() .PopAndDiscardSoloNodeId(); diff --git a/toolchain/check/handle_name.cpp b/toolchain/check/handle_name.cpp index c092506c6611..cbe801d8eed6 100644 --- a/toolchain/check/handle_name.cpp +++ b/toolchain/check/handle_name.cpp @@ -4,6 +4,7 @@ #include "toolchain/check/context.h" #include "toolchain/check/member_access.h" +#include "toolchain/check/name_component.h" #include "toolchain/check/pointer_dereference.h" #include "toolchain/lex/token_kind.h" #include "toolchain/sem_ir/inst.h" @@ -125,19 +126,9 @@ auto HandleSelfValueNameExpr(Context& context, return HandleNameAsExpr(context, node_id, SemIR::NameId::SelfValue); } -auto HandleNameQualifier(Context& context, Parse::NameQualifierId node_id) +auto HandleNameQualifier(Context& context, Parse::NameQualifierId /*node_id*/) -> bool { - auto params_id = context.node_stack().PopIf(); - auto implicit_params_id = - context.node_stack().PopIf(); - if (params_id || implicit_params_id) { - // TODO: Pass the parameters into decl_name_stack. - context.TODO(node_id, "name qualifier with parameters"); - return false; - } - - auto [name_node_id, name_id] = context.node_stack().PopNameWithNodeId(); - context.decl_name_stack().ApplyNameQualifier(name_node_id, name_id); + context.decl_name_stack().ApplyNameQualifier(PopNameComponent(context)); return true; } diff --git a/toolchain/check/handle_namespace.cpp b/toolchain/check/handle_namespace.cpp index c21ea6967fd3..7d6d4e1218cd 100644 --- a/toolchain/check/handle_namespace.cpp +++ b/toolchain/check/handle_namespace.cpp @@ -5,6 +5,7 @@ #include "toolchain/check/context.h" #include "toolchain/check/decl_state.h" #include "toolchain/check/modifiers.h" +#include "toolchain/check/name_component.h" #include "toolchain/sem_ir/ids.h" namespace Carbon::Check { @@ -18,7 +19,8 @@ auto HandleNamespaceStart(Context& context, Parse::NamespaceStartId /*node_id*/) } auto HandleNamespace(Context& context, Parse::NamespaceId node_id) -> bool { - auto name_context = context.decl_name_stack().FinishName(); + auto name_context = context.decl_name_stack().FinishName( + PopNameComponentWithoutParams(context, Lex::TokenKind::Namespace)); LimitModifiersOnDecl(context, KeywordModifierSet::None, Lex::TokenKind::Namespace); auto namespace_inst = SemIR::Namespace{ diff --git a/toolchain/check/member_access.cpp b/toolchain/check/member_access.cpp index d73369aac779..dec4f451fdae 100644 --- a/toolchain/check/member_access.cpp +++ b/toolchain/check/member_access.cpp @@ -82,7 +82,7 @@ static auto IsInstanceMethod(const SemIR::File& sem_ir, SemIR::FunctionId function_id) -> bool { const auto& function = sem_ir.functions().Get(function_id); for (auto param_id : - sem_ir.inst_blocks().Get(function.implicit_param_refs_id)) { + sem_ir.inst_blocks().GetOrEmpty(function.implicit_param_refs_id)) { auto param = SemIR::Function::GetParamFromParamRefId(sem_ir, param_id).second; if (param.name_id == SemIR::NameId::SelfValue) { diff --git a/toolchain/check/name_component.cpp b/toolchain/check/name_component.cpp new file mode 100644 index 000000000000..b5c785132af8 --- /dev/null +++ b/toolchain/check/name_component.cpp @@ -0,0 +1,50 @@ +// Part of the Carbon Language project, under the Apache License v2.0 with LLVM +// Exceptions. See /LICENSE for license information. +// SPDX-License-Identifier: Apache-2.0 WITH LLVM-exception + +#include "toolchain/check/name_component.h" + +#include "toolchain/check/context.h" + +namespace Carbon::Check { + +auto PopNameComponent(Context& context) -> NameComponent { + auto [params_loc_id, params_id] = + context.node_stack().PopWithNodeIdIf(); + auto [implicit_params_loc_id, implicit_params_id] = + context.node_stack() + .PopWithNodeIdIf(); + auto [name_loc_id, name_id] = context.node_stack().PopNameWithNodeId(); + return { + .name_loc_id = name_loc_id, + .name_id = name_id, + .implicit_params_loc_id = implicit_params_loc_id, + .implicit_params_id = + implicit_params_id.value_or(SemIR::InstBlockId::Invalid), + .params_loc_id = params_loc_id, + .params_id = params_id.value_or(SemIR::InstBlockId::Invalid), + }; +} + +// Pop the name of a declaration from the node stack, and diagnose if it has +// parameters. +auto PopNameComponentWithoutParams(Context& context, Lex::TokenKind introducer) + -> NameComponent { + NameComponent name = PopNameComponent(context); + if (name.implicit_params_id.is_valid() || name.params_id.is_valid()) { + CARBON_DIAGNOSTIC(UnexpectedDeclNameParams, Error, + "`{0}` declaration cannot have parameters.", + Lex::TokenKind); + // Point to the lexically first parameter list in the diagnostic. + context.emitter().Emit(name.implicit_params_id.is_valid() + ? name.implicit_params_loc_id + : name.params_loc_id, + UnexpectedDeclNameParams, introducer); + + name.implicit_params_id = SemIR::InstBlockId::Invalid; + name.params_id = SemIR::InstBlockId::Invalid; + } + return name; +} + +} // namespace Carbon::Check diff --git a/toolchain/check/name_component.h b/toolchain/check/name_component.h new file mode 100644 index 000000000000..4adecac7ce82 --- /dev/null +++ b/toolchain/check/name_component.h @@ -0,0 +1,42 @@ +// Part of the Carbon Language project, under the Apache License v2.0 with LLVM +// Exceptions. See /LICENSE for license information. +// SPDX-License-Identifier: Apache-2.0 WITH LLVM-exception + +#ifndef CARBON_TOOLCHAIN_CHECK_NAME_COMPONENT_H_ +#define CARBON_TOOLCHAIN_CHECK_NAME_COMPONENT_H_ + +#include "toolchain/check/node_stack.h" +#include "toolchain/parse/node_ids.h" +#include "toolchain/sem_ir/ids.h" + +namespace Carbon::Check { + +class Context; + +// A component in a declaration name, such as `C[T:! type](N:! T)` in +// `fn C[T:! type](N:! T).F() {}`. +struct NameComponent { + // The name of the declaration. + Parse::NodeId name_loc_id; + SemIR::NameId name_id; + + // The implicit parameter list. + Parse::NodeId implicit_params_loc_id; + SemIR::InstBlockId implicit_params_id; + + // The explicit parameter list. + Parse::NodeId params_loc_id; + SemIR::InstBlockId params_id; +}; + +// Pop a name component from the node stack. +auto PopNameComponent(Context& context) -> NameComponent; + +// Pop the name of a declaration from the node stack, and diagnose if it has +// parameters. +auto PopNameComponentWithoutParams(Context& context, Lex::TokenKind introducer) + -> NameComponent; + +} // namespace Carbon::Check + +#endif // CARBON_TOOLCHAIN_CHECK_NAME_COMPONENT_H_ diff --git a/toolchain/check/testdata/alias/no_prelude/fail_params.carbon b/toolchain/check/testdata/alias/no_prelude/fail_params.carbon index 1cadc96aa6b8..cdc0e240aa41 100644 --- a/toolchain/check/testdata/alias/no_prelude/fail_params.carbon +++ b/toolchain/check/testdata/alias/no_prelude/fail_params.carbon @@ -4,7 +4,7 @@ // // AUTOUPDATE -// CHECK:STDERR: fail_params.carbon:[[@LINE+7]]:8: ERROR: Declaration cannot have parameters. +// CHECK:STDERR: fail_params.carbon:[[@LINE+7]]:8: ERROR: `alias` declaration cannot have parameters. // CHECK:STDERR: alias A(T:! type) = T*; // CHECK:STDERR: ^~~~~~~~~~ // CHECK:STDERR: diff --git a/toolchain/check/testdata/class/fail_todo_generic_method.carbon b/toolchain/check/testdata/class/fail_todo_generic_method.carbon index 0a4f9e628f71..812e313fb6df 100644 --- a/toolchain/check/testdata/class/fail_todo_generic_method.carbon +++ b/toolchain/check/testdata/class/fail_todo_generic_method.carbon @@ -9,9 +9,9 @@ class Class(T:! type) { fn F[self: Self](n: T); } -// CHECK:STDERR: fail_todo_generic_method.carbon:[[@LINE+3]]:4: ERROR: Semantics TODO: `name qualifier with parameters`. +// CHECK:STDERR: fail_todo_generic_method.carbon:[[@LINE+3]]:9: ERROR: Semantics TODO: `name qualifier with parameters`. // CHECK:STDERR: fn Class(T:! type).F[self: Self](n: T) {} -// CHECK:STDERR: ^~~~~~~~~~~~~~~~ +// CHECK:STDERR: ^~~~~~~~~~ fn Class(T:! type).F[self: Self](n: T) {} // CHECK:STDOUT: --- fail_todo_generic_method.carbon @@ -26,18 +26,39 @@ fn Class(T:! type).F[self: Self](n: T) {} // CHECK:STDOUT: %F: type = fn_type @F [template] // CHECK:STDOUT: %struct.2: F = struct_value () [template] // CHECK:STDOUT: %.3: type = struct_type {.a: T} [symbolic] +// CHECK:STDOUT: %.4: type = ptr_type {.a: T} [symbolic] // CHECK:STDOUT: } // CHECK:STDOUT: -// CHECK:STDOUT: file {} +// CHECK:STDOUT: file { +// CHECK:STDOUT: package: = namespace [template] { +// CHECK:STDOUT: .Core = %Core +// CHECK:STDOUT: .Class = %Class.decl +// CHECK:STDOUT: } +// CHECK:STDOUT: %Core: = namespace [template] {} +// CHECK:STDOUT: %Class.decl: Class = class_decl @Class [template = constants.%struct.1] { +// CHECK:STDOUT: %T.loc7_13.1: type = param T +// CHECK:STDOUT: %T.loc7_13.2: type = bind_symbolic_name T 0, %T.loc7_13.1 [symbolic = constants.%T] +// CHECK:STDOUT: } +// CHECK:STDOUT: %F.decl: F = fn_decl @F [template = constants.%struct.2] { +// CHECK:STDOUT: %T.loc15_10.1: type = param T +// CHECK:STDOUT: %T.loc15_10.2: type = bind_symbolic_name T 0, %T.loc15_10.1 [symbolic = constants.%T] +// CHECK:STDOUT: %Self.ref: type = name_ref Self, constants.%Class.2 [template = constants.%Class.2] +// CHECK:STDOUT: %self.loc15_22.1: Class = param self +// CHECK:STDOUT: @F.%self: Class = bind_name self, %self.loc15_22.1 +// CHECK:STDOUT: %T.ref: type = name_ref T, %T.loc15_10.2 [symbolic = constants.%T] +// CHECK:STDOUT: %n.loc15_34.1: T = param n +// CHECK:STDOUT: @F.%n: T = bind_name n, %n.loc15_34.1 +// CHECK:STDOUT: } +// CHECK:STDOUT: } // CHECK:STDOUT: // CHECK:STDOUT: class @Class { -// CHECK:STDOUT: %T.ref.loc8: type = name_ref T, [symbolic = constants.%T] +// CHECK:STDOUT: %T.ref.loc8: type = name_ref T, file.%T.loc7_13.2 [symbolic = constants.%T] // CHECK:STDOUT: %.loc8: = field_decl a, element0 [template] // CHECK:STDOUT: %F.decl: F = fn_decl @F [template = constants.%struct.2] { // CHECK:STDOUT: %Self.ref: type = name_ref Self, constants.%Class.2 [template = constants.%Class.2] // CHECK:STDOUT: %self.loc9_8.1: Class = param self // CHECK:STDOUT: %self.loc9_8.2: Class = bind_name self, %self.loc9_8.1 -// CHECK:STDOUT: %T.ref.loc9: type = name_ref T, [symbolic = constants.%T] +// CHECK:STDOUT: %T.ref.loc9: type = name_ref T, file.%T.loc7_13.2 [symbolic = constants.%T] // CHECK:STDOUT: %n.loc9_20.1: T = param n // CHECK:STDOUT: %n.loc9_20.2: T = bind_name n, %n.loc9_20.1 // CHECK:STDOUT: } @@ -48,5 +69,8 @@ fn Class(T:! type).F[self: Self](n: T) {} // CHECK:STDOUT: .F = %F.decl // CHECK:STDOUT: } // CHECK:STDOUT: -// CHECK:STDOUT: fn @F[@Class.%self.loc9_8.2: Class](@Class.%n.loc9_20.2: T); +// CHECK:STDOUT: fn @F[%self: Class](%n: T) { +// CHECK:STDOUT: !entry: +// CHECK:STDOUT: return +// CHECK:STDOUT: } // CHECK:STDOUT: diff --git a/toolchain/check/testdata/class/generic/fail_todo_member_out_of_line.carbon b/toolchain/check/testdata/class/generic/fail_todo_member_out_of_line.carbon index 5bbd9cc86b8f..f360eb3ff665 100644 --- a/toolchain/check/testdata/class/generic/fail_todo_member_out_of_line.carbon +++ b/toolchain/check/testdata/class/generic/fail_todo_member_out_of_line.carbon @@ -8,9 +8,9 @@ class Class(T:! type) { fn F(n: T) -> T; } -// CHECK:STDERR: fail_todo_member_out_of_line.carbon:[[@LINE+3]]:4: ERROR: Semantics TODO: `name qualifier with parameters`. +// CHECK:STDERR: fail_todo_member_out_of_line.carbon:[[@LINE+3]]:9: ERROR: Semantics TODO: `name qualifier with parameters`. // CHECK:STDERR: fn Class(T:! type).F(n: T) -> T { -// CHECK:STDERR: ^~~~~~~~~~~~~~~~ +// CHECK:STDERR: ^~~~~~~~~~ fn Class(T:! type).F(n: T) -> T { return n; } @@ -28,14 +28,33 @@ fn Class(T:! type).F(n: T) -> T { // CHECK:STDOUT: %.2: type = struct_type {} [template] // CHECK:STDOUT: } // CHECK:STDOUT: -// CHECK:STDOUT: file {} +// CHECK:STDOUT: file { +// CHECK:STDOUT: package: = namespace [template] { +// CHECK:STDOUT: .Core = %Core +// CHECK:STDOUT: .Class = %Class.decl +// CHECK:STDOUT: } +// CHECK:STDOUT: %Core: = namespace [template] {} +// CHECK:STDOUT: %Class.decl: Class = class_decl @Class [template = constants.%struct.1] { +// CHECK:STDOUT: %T.loc7_13.1: type = param T +// CHECK:STDOUT: %T.loc7_13.2: type = bind_symbolic_name T 0, %T.loc7_13.1 [symbolic = constants.%T] +// CHECK:STDOUT: } +// CHECK:STDOUT: %F.decl: F = fn_decl @F [template = constants.%struct.2] { +// CHECK:STDOUT: %T.loc14_10.1: type = param T +// CHECK:STDOUT: %T.loc14_10.2: type = bind_symbolic_name T 0, %T.loc14_10.1 [symbolic = constants.%T] +// CHECK:STDOUT: %T.ref.loc14_25: type = name_ref T, %T.loc14_10.2 [symbolic = constants.%T] +// CHECK:STDOUT: %n.loc14_22.1: T = param n +// CHECK:STDOUT: @F.%n: T = bind_name n, %n.loc14_22.1 +// CHECK:STDOUT: %T.ref.loc14_31: type = name_ref T, %T.loc14_10.2 [symbolic = constants.%T] +// CHECK:STDOUT: @F.%return: ref T = var +// CHECK:STDOUT: } +// CHECK:STDOUT: } // CHECK:STDOUT: // CHECK:STDOUT: class @Class { // CHECK:STDOUT: %F.decl: F = fn_decl @F [template = constants.%struct.2] { -// CHECK:STDOUT: %T.ref.loc8_11: type = name_ref T, [symbolic = constants.%T] +// CHECK:STDOUT: %T.ref.loc8_11: type = name_ref T, file.%T.loc7_13.2 [symbolic = constants.%T] // CHECK:STDOUT: %n.loc8_8.1: T = param n // CHECK:STDOUT: %n.loc8_8.2: T = bind_name n, %n.loc8_8.1 -// CHECK:STDOUT: %T.ref.loc8_17: type = name_ref T, [symbolic = constants.%T] +// CHECK:STDOUT: %T.ref.loc8_17: type = name_ref T, file.%T.loc7_13.2 [symbolic = constants.%T] // CHECK:STDOUT: %return.var: ref T = var // CHECK:STDOUT: } // CHECK:STDOUT: @@ -44,5 +63,9 @@ fn Class(T:! type).F(n: T) -> T { // CHECK:STDOUT: .F = %F.decl // CHECK:STDOUT: } // CHECK:STDOUT: -// CHECK:STDOUT: fn @F(@Class.%n.loc8_8.2: T) -> T; +// CHECK:STDOUT: fn @F(%n: T) -> T { +// CHECK:STDOUT: !entry: +// CHECK:STDOUT: %n.ref: T = name_ref n, %n +// CHECK:STDOUT: return %n.ref +// CHECK:STDOUT: } // CHECK:STDOUT: diff --git a/toolchain/check/testdata/impl/fail_impl_bad_assoc_fn.carbon b/toolchain/check/testdata/impl/fail_impl_bad_assoc_fn.carbon index eb2aab9beb7b..036a8f1718ae 100644 --- a/toolchain/check/testdata/impl/fail_impl_bad_assoc_fn.carbon +++ b/toolchain/check/testdata/impl/fail_impl_bad_assoc_fn.carbon @@ -61,10 +61,10 @@ class FExtraParam { class FExtraImplicitParam { impl as I { - // CHECK:STDERR: fail_impl_bad_assoc_fn.carbon:[[@LINE+7]]:5: ERROR: Redeclaration differs because of implicit parameter count of 1. + // CHECK:STDERR: fail_impl_bad_assoc_fn.carbon:[[@LINE+7]]:5: ERROR: Redeclaration differs because of implicit parameter list. // CHECK:STDERR: fn F[self: Self](); // CHECK:STDERR: ^~~~~~~~~~~~~~~~~~~ - // CHECK:STDERR: fail_impl_bad_assoc_fn.carbon:[[@LINE-60]]:15: Previously declared with implicit parameter count of 0. + // CHECK:STDERR: fail_impl_bad_assoc_fn.carbon:[[@LINE-60]]:15: Previously declared without implicit parameter list. // CHECK:STDERR: interface I { fn F(); } // CHECK:STDERR: ^~~~~~~ // CHECK:STDERR: @@ -103,10 +103,10 @@ class FMissingParam { class FMissingImplicitParam { impl as J { - // CHECK:STDERR: fail_impl_bad_assoc_fn.carbon:[[@LINE+7]]:5: ERROR: Redeclaration differs because of implicit parameter count of 0. + // CHECK:STDERR: fail_impl_bad_assoc_fn.carbon:[[@LINE+7]]:5: ERROR: Redeclaration differs because of missing implicit parameter list. // CHECK:STDERR: fn F(b: bool) -> bool; // CHECK:STDERR: ^~~~~~~~~~~~~~~~~~~~~~ - // CHECK:STDERR: fail_impl_bad_assoc_fn.carbon:[[@LINE-20]]:15: Previously declared with implicit parameter count of 1. + // CHECK:STDERR: fail_impl_bad_assoc_fn.carbon:[[@LINE-20]]:15: Previously declared with implicit parameter list. // CHECK:STDERR: interface J { fn F[self: bool](b: bool) -> bool; } // CHECK:STDERR: ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~ // CHECK:STDERR: diff --git a/toolchain/check/testdata/interface/no_prelude/fail_todo_generic.carbon b/toolchain/check/testdata/interface/no_prelude/fail_todo_generic.carbon index d27f11092277..3e835357735b 100644 --- a/toolchain/check/testdata/interface/no_prelude/fail_todo_generic.carbon +++ b/toolchain/check/testdata/interface/no_prelude/fail_todo_generic.carbon @@ -4,10 +4,6 @@ // // AUTOUPDATE -// CHECK:STDERR: fail_todo_generic.carbon:[[@LINE+7]]:1: ERROR: Semantics TODO: `generic interface`. -// CHECK:STDERR: interface I[](); -// CHECK:STDERR: ^~~~~~~~~~~~~~~~ -// CHECK:STDERR: // CHECK:STDERR: fail_todo_generic.carbon:[[@LINE+3]]:1: ERROR: Semantics TODO: `generic interface`. // CHECK:STDERR: interface I[](); // CHECK:STDERR: ^~~~~~~~~~~~~~~~ diff --git a/toolchain/check/testdata/namespace/fail_params.carbon b/toolchain/check/testdata/namespace/fail_params.carbon index 9ff4406fd419..c6f9c2c8dbf8 100644 --- a/toolchain/check/testdata/namespace/fail_params.carbon +++ b/toolchain/check/testdata/namespace/fail_params.carbon @@ -4,7 +4,7 @@ // // AUTOUPDATE -// CHECK:STDERR: fail_params.carbon:[[@LINE+4]]:12: ERROR: Declaration cannot have parameters. +// CHECK:STDERR: fail_params.carbon:[[@LINE+4]]:12: ERROR: `namespace` declaration cannot have parameters. // CHECK:STDERR: namespace A(); // CHECK:STDERR: ^~ // CHECK:STDERR: @@ -13,13 +13,13 @@ namespace A(); // Parameters are ignored for error recovery. fn A.F() {} -// CHECK:STDERR: fail_params.carbon:[[@LINE+4]]:12: ERROR: Declaration cannot have parameters. +// CHECK:STDERR: fail_params.carbon:[[@LINE+4]]:12: ERROR: `namespace` declaration cannot have parameters. // CHECK:STDERR: namespace B(n: i32); // CHECK:STDERR: ^~~~~~~~ // CHECK:STDERR: namespace B(n: i32); -// CHECK:STDERR: fail_params.carbon:[[@LINE+3]]:12: ERROR: Declaration cannot have parameters. +// CHECK:STDERR: fail_params.carbon:[[@LINE+3]]:12: ERROR: `namespace` declaration cannot have parameters. // CHECK:STDERR: namespace C[T:! type](x: T); // CHECK:STDERR: ^~~~~~~~~~ namespace C[T:! type](x: T); diff --git a/toolchain/check/testdata/packages/no_prelude/fail_export_name_params.carbon b/toolchain/check/testdata/packages/no_prelude/fail_export_name_params.carbon index b370b1686e2c..89c31f1ce7ba 100644 --- a/toolchain/check/testdata/packages/no_prelude/fail_export_name_params.carbon +++ b/toolchain/check/testdata/packages/no_prelude/fail_export_name_params.carbon @@ -19,7 +19,7 @@ import library "a"; export C1; -// CHECK:STDERR: fail_b.carbon:[[@LINE+3]]:10: ERROR: Declaration cannot have parameters. +// CHECK:STDERR: fail_b.carbon:[[@LINE+3]]:10: ERROR: `export` declaration cannot have parameters. // CHECK:STDERR: export C2(T:! type); // CHECK:STDERR: ^~~~~~~~~~ export C2(T:! type); diff --git a/toolchain/lower/file_context.cpp b/toolchain/lower/file_context.cpp index 865d23589826..7644b8a748ba 100644 --- a/toolchain/lower/file_context.cpp +++ b/toolchain/lower/file_context.cpp @@ -139,8 +139,9 @@ auto FileContext::BuildFunctionDecl(SemIR::FunctionId function_id) const bool has_return_slot = function.has_return_slot(); auto implicit_param_refs = - sem_ir().inst_blocks().Get(function.implicit_param_refs_id); - auto param_refs = sem_ir().inst_blocks().Get(function.param_refs_id); + sem_ir().inst_blocks().GetOrEmpty(function.implicit_param_refs_id); + // TODO: Include parameters corresponding to positional parameters. + auto param_refs = sem_ir().inst_blocks().GetOrEmpty(function.param_refs_id); SemIR::InitRepr return_rep = function.return_type_id.is_valid() @@ -245,8 +246,8 @@ auto FileContext::BuildFunctionDefinition(SemIR::FunctionId function_id) // function parameters that was already computed in BuildFunctionDecl. // We should only do that once. auto implicit_param_refs = - sem_ir().inst_blocks().Get(function.implicit_param_refs_id); - auto param_refs = sem_ir().inst_blocks().Get(function.param_refs_id); + sem_ir().inst_blocks().GetOrEmpty(function.implicit_param_refs_id); + auto param_refs = sem_ir().inst_blocks().GetOrEmpty(function.param_refs_id); int param_index = 0; if (has_return_slot) { function_lowering.SetLocal(function.return_storage_id, diff --git a/toolchain/sem_ir/formatter.cpp b/toolchain/sem_ir/formatter.cpp index 39be4f57fd82..8084bd1b750a 100644 --- a/toolchain/sem_ir/formatter.cpp +++ b/toolchain/sem_ir/formatter.cpp @@ -233,15 +233,17 @@ class Formatter { llvm::SaveAndRestore function_scope(scope_, inst_namer_.GetScopeFor(id)); - if (fn.implicit_param_refs_id != InstBlockId::Empty) { + if (fn.implicit_param_refs_id.is_valid()) { out_ << "["; FormatParamList(fn.implicit_param_refs_id); out_ << "]"; } - out_ << "("; - FormatParamList(fn.param_refs_id); - out_ << ")"; + if (fn.param_refs_id.is_valid()) { + out_ << "("; + FormatParamList(fn.param_refs_id); + out_ << ")"; + } if (fn.return_type_id.is_valid()) { out_ << " -> "; diff --git a/toolchain/sem_ir/inst.h b/toolchain/sem_ir/inst.h index a19f31d8d264..64b14ff75262 100644 --- a/toolchain/sem_ir/inst.h +++ b/toolchain/sem_ir/inst.h @@ -450,7 +450,7 @@ class InstBlockStore : public BlockValueStore { // Returns the contents of the specified block, or an empty array if the block // is empty. - auto GetOrEmpty(InstBlockId block_id) -> llvm::ArrayRef { + auto GetOrEmpty(InstBlockId block_id) const -> llvm::ArrayRef { return block_id.is_valid() ? Get(block_id) : llvm::ArrayRef(); } };