carbon-language · zygoloid · May 29, 2024 · May 24, 2024 · May 24, 2024 · May 24, 2024
diff --git a/toolchain/check/decl_name_stack.cpp b/toolchain/check/decl_name_stack.cpp
@@ -6,6 +6,8 @@
 
 #include "toolchain/base/kind_switch.h"
 #include "toolchain/check/context.h"
+#include "toolchain/check/diagnostic_helpers.h"
+#include "toolchain/diagnostics/diagnostic.h"
 #include "toolchain/sem_ir/ids.h"
 
 namespace Carbon::Check {
@@ -56,14 +58,24 @@ auto DeclNameStack::PushScopeAndStartName() -> void {
 auto DeclNameStack::FinishName() -> NameContext {
   CARBON_CHECK(decl_name_stack_.back().state != NameContext::State::Finished)
       << "Finished name twice";
-  if (context_->node_stack()
-          .PopAndDiscardSoloNodeIdIf<Parse::NodeKind::QualifiedName>()) {
-    // Any parts from a QualifiedName will already have been processed
-    // into the name.
-  } else {
-    // The name had no qualifiers, so we need to process the node now.
-    auto [loc_id, name_id] = context_->node_stack().PopNameWithNodeId();
-    ApplyNameQualifier(loc_id, name_id);
+  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);
   }
 
   NameContext result = decl_name_stack_.back();

diff --git a/toolchain/check/handle_function.cpp b/toolchain/check/handle_function.cpp
@@ -211,8 +211,12 @@ static auto BuildFunctionDecl(Context& context,
     return_slot = SemIR::Function::ReturnSlot::Absent;
   }
 
-  SemIR::InstBlockId param_refs_id =
-      context.node_stack().Pop<Parse::NodeKind::TuplePattern>();
+  auto param_refs_id =
+      context.node_stack().PopIf<Parse::NodeKind::TuplePattern>();
+  if (!param_refs_id) {
+    context.TODO(node_id, "function with positional parameters");
+    param_refs_id = SemIR::InstBlockId::Empty;
+  }
   // TODO: Use Invalid rather than Empty if there was no implicit parameter
   // list.
   SemIR::InstBlockId implicit_param_refs_id =
@@ -253,7 +257,7 @@ static auto BuildFunctionDecl(Context& context,
       .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,
+      .param_refs_id = *param_refs_id,
       .return_type_id = return_type_id,
       .return_storage_id = return_storage_id,
       .is_extern = is_extern,
@@ -297,12 +301,14 @@ 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(implicit_param_refs_id).empty() ||
-        !context.inst_blocks().Get(param_refs_id).empty() ||
-        (return_type_id.is_valid() &&
-         return_type_id !=
+    if (!context.inst_blocks()
+             .Get(function_info.implicit_param_refs_id)
+             .empty() ||
+        !context.inst_blocks().Get(function_info.param_refs_id).empty() ||
+        (function_info.return_type_id.is_valid() &&
+         function_info.return_type_id !=
              context.GetBuiltinType(SemIR::BuiltinKind::IntType) &&
-         return_type_id != context.GetTupleType({}))) {
+         function_info.return_type_id != context.GetTupleType({}))) {
       CARBON_DIAGNOSTIC(InvalidMainRunSignature, Error,
                         "Invalid signature for `Main.Run` function. Expected "
                         "`fn ()` or `fn () -> i32`.");

diff --git a/toolchain/check/handle_name.cpp b/toolchain/check/handle_name.cpp
@@ -125,35 +125,19 @@ auto HandleSelfValueNameExpr(Context& context,
   return HandleNameAsExpr(context, node_id, SemIR::NameId::SelfValue);
 }
 
-auto HandleQualifiedName(Context& context, Parse::QualifiedNameId node_id)
+auto HandleNameQualifier(Context& context, Parse::NameQualifierId node_id)
     -> bool {
-  auto [node_id2, name_id2] = context.node_stack().PopNameWithNodeId();
-
-  Parse::NodeId node_id1 = context.node_stack().PeekNodeId();
-  switch (context.parse_tree().node_kind(node_id1)) {
-    case Parse::NodeKind::QualifiedName:
-      // This is the second or subsequent QualifiedName in a chain.
-      // Nothing to do: the first QualifiedName remains as a
-      // bracketing node for later QualifiedNames.
-      break;
-
-    case Parse::NodeKind::IdentifierName: {
-      // This is the first QualifiedName in a chain, and starts with an
-      // identifier name.
-      auto name_id =
-          context.node_stack().Pop<Parse::NodeKind::IdentifierName>();
-      context.decl_name_stack().ApplyNameQualifier(node_id1, name_id);
-      // Add the QualifiedName so that it can be used for bracketing.
-      context.node_stack().Push(node_id);
-      break;
-    }
-
-    default:
-      CARBON_FATAL() << "Unexpected node kind on left side of qualified "
-                        "declaration name";
+  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;
   }
 
-  context.decl_name_stack().ApplyNameQualifier(node_id2, name_id2);
+  auto [name_node_id, name_id] = context.node_stack().PopNameWithNodeId();
+  context.decl_name_stack().ApplyNameQualifier(name_node_id, name_id);
   return true;
 }
 

diff --git a/toolchain/check/node_stack.h b/toolchain/check/node_stack.h
@@ -445,7 +445,6 @@ class NodeStack {
         case Parse::NodeKind::InterfaceIntroducer:
         case Parse::NodeKind::LetInitializer:
         case Parse::NodeKind::LetIntroducer:
-        case Parse::NodeKind::QualifiedName:
         case Parse::NodeKind::ReturnedModifier:
         case Parse::NodeKind::ReturnStatementStart:
         case Parse::NodeKind::ReturnVarModifier:

diff --git a/toolchain/check/testdata/alias/no_prelude/fail_params.carbon b/toolchain/check/testdata/alias/no_prelude/fail_params.carbon
@@ -0,0 +1,33 @@
+// 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
+//
+// AUTOUPDATE
+
+// CHECK:STDERR: fail_params.carbon:[[@LINE+7]]:8: ERROR: Declaration cannot have parameters.
+// CHECK:STDERR: alias A(T:! type) = T*;
+// CHECK:STDERR:        ^~~~~~~~~~
+// CHECK:STDERR:
+// CHECK:STDERR: fail_params.carbon:[[@LINE+3]]:21: ERROR: Alias initializer must be a name reference.
+// CHECK:STDERR: alias A(T:! type) = T*;
+// CHECK:STDERR:                     ^~
+alias A(T:! type) = T*;
+
+// CHECK:STDOUT: --- fail_params.carbon
+// CHECK:STDOUT:
+// CHECK:STDOUT: constants {
+// CHECK:STDOUT:   %T: type = bind_symbolic_name T 0 [symbolic]
+// CHECK:STDOUT:   %.1: type = ptr_type T [symbolic]
+// CHECK:STDOUT: }
+// CHECK:STDOUT:
+// CHECK:STDOUT: file {
+// CHECK:STDOUT:   package: <namespace> = namespace [template] {
+// CHECK:STDOUT:     .A = %A
+// CHECK:STDOUT:   }
+// CHECK:STDOUT:   %T.loc14_9.1: type = param T
+// CHECK:STDOUT:   %T.loc14_9.2: type = bind_symbolic_name T 0, %T.loc14_9.1 [symbolic = constants.%T]
+// CHECK:STDOUT:   %T.ref: type = name_ref T, %T.loc14_9.2 [symbolic = constants.%T]
+// CHECK:STDOUT:   %.loc14: type = ptr_type T [symbolic = constants.%.1]
+// CHECK:STDOUT:   %A: <error> = bind_alias A, <error> [template = <error>]
+// CHECK:STDOUT: }
+// CHECK:STDOUT:
diff --git a/toolchain/check/testdata/class/fail_base_as_declared_name.carbon b/toolchain/check/testdata/class/fail_base_as_declared_name.carbon
@@ -6,11 +6,11 @@
 
 namespace N;
 
-// CHECK:STDERR: fail_base_as_declared_name.carbon:[[@LINE+7]]:6: ERROR: Expected identifier after `.`.
+// CHECK:STDERR: fail_base_as_declared_name.carbon:[[@LINE+7]]:6: ERROR: `.` should be followed by a name.
 // CHECK:STDERR: fn N.base() {}
 // CHECK:STDERR:      ^~~~
 // CHECK:STDERR:
-// CHECK:STDERR: fail_base_as_declared_name.carbon:[[@LINE+3]]:6: ERROR: Semantics TODO: `Error recovery from keyword name.`.
+// CHECK:STDERR: fail_base_as_declared_name.carbon:[[@LINE+3]]:6: ERROR: Semantics TODO: `HandleInvalidParse`.
 // CHECK:STDERR: fn N.base() {}
 // CHECK:STDERR:      ^~~~
 fn N.base() {}

diff --git a/toolchain/check/testdata/class/fail_todo_generic_method.carbon b/toolchain/check/testdata/class/fail_todo_generic_method.carbon
@@ -9,16 +9,9 @@ class Class(T:! type) {
   fn F[self: Self](n: T);
 }
 
-// CHECK:STDERR: fail_todo_generic_method.carbon:[[@LINE+10]]:1: ERROR: Duplicate name being declared in the same scope.
+// CHECK:STDERR: fail_todo_generic_method.carbon:[[@LINE+3]]:4: ERROR: Semantics TODO: `name qualifier with parameters`.
 // CHECK:STDERR: fn Class(T:! type).F[self: Self](n: T) {}
-// CHECK:STDERR: ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
-// CHECK:STDERR: fail_todo_generic_method.carbon:[[@LINE-8]]:1: Name is previously declared here.
-// CHECK:STDERR: class Class(T:! type) {
-// CHECK:STDERR: ^~~~~~~~~~~~~~~~~~~~~~~
-// CHECK:STDERR:
-// CHECK:STDERR: fail_todo_generic_method.carbon:[[@LINE+3]]:19: ERROR: `fn` declarations must either end with a `;` or have a `{ ... }` block for a definition.
-// 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
@@ -33,34 +26,18 @@ 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 = fn_type @.1 [template]
-// CHECK:STDOUT:   %struct.3: <invalid> = struct_value () [template]
 // CHECK:STDOUT: }
 // CHECK:STDOUT:
-// 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:   %.decl: <invalid> = fn_decl @.1 [template = constants.%struct.3] {
-// CHECK:STDOUT:     %T.loc22_10.1: type = param T
-// CHECK:STDOUT:     @.1.%T: type = bind_symbolic_name T 0, %T.loc22_10.1 [symbolic = constants.%T]
-// CHECK:STDOUT:   }
-// CHECK:STDOUT: }
+// CHECK:STDOUT: file {}
 // CHECK:STDOUT:
 // CHECK:STDOUT: class @Class {
-// CHECK:STDOUT:   %T.ref.loc8: type = name_ref T, file.%T.loc7_13.2 [symbolic = constants.%T]
+// CHECK:STDOUT:   %T.ref.loc8: type = name_ref T, <unexpected instref inst+3> [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, file.%T.loc7_13.2 [symbolic = constants.%T]
+// CHECK:STDOUT:     %T.ref.loc9: type = name_ref T, <unexpected instref inst+3> [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:   }
@@ -73,5 +50,3 @@ fn Class(T:! type).F[self: Self](n: T) {}
 // CHECK:STDOUT:
 // CHECK:STDOUT: fn @F[@Class.%self.loc9_8.2: Class](@Class.%n.loc9_20.2: T);
 // CHECK:STDOUT:
-// CHECK:STDOUT: fn @.1(%T: type);
-// 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
@@ -8,16 +8,9 @@ class Class(T:! type) {
   fn F(n: T) -> T;
 }
 
-// CHECK:STDERR: fail_todo_member_out_of_line.carbon:[[@LINE+10]]:1: ERROR: Duplicate name being declared in the same scope.
+// CHECK:STDERR: fail_todo_member_out_of_line.carbon:[[@LINE+3]]:4: ERROR: Semantics TODO: `name qualifier with parameters`.
 // CHECK:STDERR: fn Class(T:! type).F(n: T) -> T {
-// CHECK:STDERR: ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
-// CHECK:STDERR: fail_todo_member_out_of_line.carbon:[[@LINE-7]]:1: Name is previously declared here.
-// CHECK:STDERR: class Class(T:! type) {
-// CHECK:STDERR: ^~~~~~~~~~~~~~~~~~~~~~~
-// CHECK:STDERR:
-// CHECK:STDERR: fail_todo_member_out_of_line.carbon:[[@LINE+3]]:19: ERROR: `fn` declarations must either end with a `;` or have a `{ ... }` block for a definition.
-// CHECK:STDERR: fn Class(T:! type).F(n: T) -> T {
-// CHECK:STDERR:                   ^
+// CHECK:STDERR:    ^~~~~~~~~~~~~~~~
 fn Class(T:! type).F(n: T) -> T {
   return n;
 }
@@ -33,32 +26,16 @@ fn Class(T:! type).F(n: T) -> T {
 // CHECK:STDOUT:   %F: type = fn_type @F [template]
 // CHECK:STDOUT:   %struct.2: F = struct_value () [template]
 // CHECK:STDOUT:   %.2: type = struct_type {} [template]
-// CHECK:STDOUT:   %.3: type = fn_type @.1 [template]
-// CHECK:STDOUT:   %struct.3: <invalid> = struct_value () [template]
 // CHECK:STDOUT: }
 // CHECK:STDOUT:
-// 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:   %.decl: <invalid> = fn_decl @.1 [template = constants.%struct.3] {
-// CHECK:STDOUT:     %T.loc21_10.1: type = param T
-// CHECK:STDOUT:     @.1.%T: type = bind_symbolic_name T 0, %T.loc21_10.1 [symbolic = constants.%T]
-// CHECK:STDOUT:   }
-// CHECK:STDOUT: }
+// CHECK:STDOUT: file {}
 // 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, file.%T.loc7_13.2 [symbolic = constants.%T]
+// CHECK:STDOUT:     %T.ref.loc8_11: type = name_ref T, <unexpected instref inst+3> [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, file.%T.loc7_13.2 [symbolic = constants.%T]
+// CHECK:STDOUT:     %T.ref.loc8_17: type = name_ref T, <unexpected instref inst+3> [symbolic = constants.%T]
 // CHECK:STDOUT:     %return.var: ref T = var <return slot>
 // CHECK:STDOUT:   }
 // CHECK:STDOUT:
@@ -69,5 +46,3 @@ fn Class(T:! type).F(n: T) -> T {
 // CHECK:STDOUT:
 // CHECK:STDOUT: fn @F(@Class.%n.loc8_8.2: T) -> T;
 // CHECK:STDOUT:
-// CHECK:STDOUT: fn @.1(%T: type);
-// CHECK:STDOUT: