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

@@ -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",

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,

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<Parse::NodeKind::TuplePattern>();
-  auto [implicit_params_loc_id, implicit_params_id] =
-      context_->node_stack()
-          .PopWithNodeIdIf<Parse::NodeKind::ImplicitParamList>();
-  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<SemIRLoc>(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);
 }
 

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;
 

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);

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<SemIR::ClassId, SemIR::InstId> {
-  auto param_refs_id =
-      context.node_stack().PopIf<Parse::NodeKind::TuplePattern>().value_or(
-          SemIR::InstBlockId::Invalid);
-  auto implicit_param_refs_id =
-      context.node_stack().PopIf<Parse::NodeKind::ImplicitParamList>().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<Parse::NodeKind::ClassIntroducer>();
 
@@ -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,

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,

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<Parse::NodeKind::TuplePattern>();
-  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<Parse::NodeKind::ImplicitParamList>().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<Parse::NodeKind::FunctionIntroducer>();
 
@@ -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<SemIR::InstId>(
-           context.inst_blocks().Get(function.implicit_param_refs_id),
-           context.inst_blocks().Get(function.param_refs_id))) {
+  for (auto param_id : llvm::concat<const SemIR::InstId>(
+           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<SemIR::TypeId> 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<SemIR::InstId>(implicit_param_refs, param_refs)) {
+       llvm::concat<const SemIR::InstId>(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());

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<SemIR::InterfaceId, SemIR::InstId> {
-  if (context.node_stack().PopIf<Parse::NodeKind::TuplePattern>()) {
-    context.TODO(node_id, "generic interface");
-  }
-  if (context.node_stack().PopIf<Parse::NodeKind::ImplicitParamList>()) {
+  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<Parse::NodeKind::InterfaceIntroducer>();
 

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<Parse::NodeKind::TuplePattern>();
-  auto implicit_params_id =
-      context.node_stack().PopIf<Parse::NodeKind::ImplicitParamList>();
-  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;
 }
 

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{

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) {

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<Parse::NodeKind::TuplePattern>();
+  auto [implicit_params_loc_id, implicit_params_id] =
+      context.node_stack()
+          .PopWithNodeIdIf<Parse::NodeKind::ImplicitParamList>();
+  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

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_

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:

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> = namespace [template] {
+// CHECK:STDOUT:     .Core = %Core
+// CHECK:STDOUT:     .Class = %Class.decl
+// CHECK:STDOUT:   }
+// CHECK:STDOUT:   %Core: <namespace> = 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, <unexpected instref inst+3> [symbolic = constants.%T]
+// CHECK:STDOUT:   %T.ref.loc8: type = name_ref T, file.%T.loc7_13.2 [symbolic = constants.%T]
 // CHECK:STDOUT:   %.loc8: <unbound element of class Class> = 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, <unexpected instref inst+3> [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:

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> = namespace [template] {
+// CHECK:STDOUT:     .Core = %Core
+// CHECK:STDOUT:     .Class = %Class.decl
+// CHECK:STDOUT:   }
+// CHECK:STDOUT:   %Core: <namespace> = 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 <return slot>
+// 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, <unexpected instref inst+3> [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, <unexpected instref inst+3> [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 <return slot>
 // 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:

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:

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: ^~~~~~~~~~~~~~~~

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);

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);

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,

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_ << " -> ";

toolchain/sem_ir/inst.h

@@ -450,7 +450,7 @@ class InstBlockStore : public BlockValueStore<InstBlockId> {
 
   // Returns the contents of the specified block, or an empty array if the block
   // is empty.
-  auto GetOrEmpty(InstBlockId block_id) -> llvm::ArrayRef<InstId> {
+  auto GetOrEmpty(InstBlockId block_id) const -> llvm::ArrayRef<InstId> {
     return block_id.is_valid() ? Get(block_id) : llvm::ArrayRef<InstId>();
   }
 };