Refactor declaration name context logic to its own class. (#2989)

This started with cleaning up the remaining Name/expression type punning
in the node stack, and grew. I'm factoring out a class because we've
previously expressed the desire to factor logic out of SemanticsContext
where possible, and this seemed like a reasonable cut.

NameExpression as the first node as a QualifiedExpression allows the
qualifier handling to consider Name in one less spot, an incremental
simplification. However, the additional complexity caused by this makes
me split ApplyNameQualifier/ApplyExpressionQualifier in order to avoid
repeat checks of the parse node's kind. The logic is still largely
shared, thus a couple helper functions. I think this is all fairly well
structured in the isolated class.

I can see that we may want to avoid passing SemanticsContext as an
argument in the future if it elides a step of lookup.

toolchain/parser/parse_node_kind.def

@@ -70,7 +70,7 @@ CARBON_PARSE_NODE_KIND_BRACKET(PackageDirective, PackageIntroducer)
 
 // `namespace`:
 //   NamespaceStart
-//   _external_: Identifier or QualifiedDeclaration
+//   _external_: Name or QualifiedDeclaration
 // Namespace
 CARBON_PARSE_NODE_KIND_CHILD_COUNT(NamespaceStart, 0)
 CARBON_PARSE_NODE_KIND_CHILD_COUNT(Namespace, 2)
@@ -84,7 +84,7 @@ CARBON_PARSE_NODE_KIND_BRACKET(CodeBlock, CodeBlockStart)
 
 // `fn`:
 //     FunctionIntroducer
-//     _external_: Identifier or QualifiedDeclaration
+//     _external_: Name or QualifiedDeclaration
 //     _external_: ParameterList
 //       _external_: type expression
 //     ReturnType
@@ -241,8 +241,8 @@ CARBON_PARSE_NODE_KIND_CHILD_COUNT(CallExpressionComma, 0)
 CARBON_PARSE_NODE_KIND_BRACKET(CallExpression, CallExpressionStart)
 
 // A qualified declaration, such as `a.b`:
-//   _external_: Identifier or QualifiedDeclaration
-//   _external_: Identifier
+//   _external_: NameExpression or QualifiedDeclaration
+//   _external_: Name
 // QualifiedDeclaration
 //
 // TODO: This will eventually more general expressions, for example with
@@ -329,7 +329,7 @@ CARBON_PARSE_NODE_KIND_BRACKET(StructTypeLiteral,
 
 // `class`:
 //     ClassIntroducer
-//     _external_: Identifier or QualifiedDeclaration
+//     _external_: Name or QualifiedDeclaration
 //   ClassDefinitionStart
 //   _external_: declarations
 // ClassDefinition
@@ -344,7 +344,7 @@ CARBON_PARSE_NODE_KIND_BRACKET(ClassDeclaration, ClassIntroducer)
 
 // `interface`:
 //     InterfaceIntroducer
-//     _external_: Identifier or QualifiedDeclaration
+//     _external_: Name or QualifiedDeclaration
 //   InterfaceDefinitionStart
 //   _external_: declarations
 // InterfaceDefinition
@@ -359,7 +359,7 @@ CARBON_PARSE_NODE_KIND_BRACKET(InterfaceDeclaration, InterfaceIntroducer)
 
 // `constraint`:
 //     NamedConstraintIntroducer
-//     _external_: Identifier or QualifiedDeclaration
+//     _external_: Name or QualifiedDeclaration
 //   NamedConstraintDefinitionStart
 //   _external_: declarations
 // NamedConstraintDefinition

toolchain/parser/parser_handle_declaration_name_and_params.cpp

@@ -18,7 +18,10 @@ static auto ParserHandleDeclarationNameAndParams(ParserContext& context,
     context.PushState(state);
 
     if (context.PositionIs(TokenKind::Period)) {
-      context.AddLeafNode(ParseNodeKind::Name, *identifier);
+      // Because there's a qualifier, we process the first segment as an
+      // expression for simplicity. This just means semantics has one less thing
+      // to handle here.
+      context.AddLeafNode(ParseNodeKind::NameExpression, *identifier);
       state.state = ParserState::PeriodAsDeclaration;
       context.PushState(state);
     } else {

toolchain/parser/testdata/namespace/fail_incomplete_name.carbon

@@ -5,7 +5,7 @@
 // AUTOUPDATE
 // CHECK:STDOUT: [
 // CHECK:STDOUT:   {kind: 'NamespaceStart', text: 'namespace'},
-// CHECK:STDOUT:     {kind: 'Name', text: 'Foo'},
+// CHECK:STDOUT:     {kind: 'NameExpression', text: 'Foo'},
 // CHECK:STDOUT:     {kind: 'Name', text: ';', has_error: yes},
 // CHECK:STDOUT:   {kind: 'QualifiedDeclaration', text: '.', subtree_size: 3},
 // CHECK:STDOUT: {kind: 'Namespace', text: ';', subtree_size: 5},

toolchain/parser/testdata/namespace/nested.carbon

@@ -8,12 +8,12 @@
 // CHECK:STDOUT:   {kind: 'Name', text: 'Foo'},
 // CHECK:STDOUT: {kind: 'Namespace', text: ';', subtree_size: 3},
 // CHECK:STDOUT:   {kind: 'NamespaceStart', text: 'namespace'},
-// CHECK:STDOUT:     {kind: 'Name', text: 'Foo'},
+// CHECK:STDOUT:     {kind: 'NameExpression', text: 'Foo'},
 // CHECK:STDOUT:     {kind: 'Name', text: 'Bar'},
 // CHECK:STDOUT:   {kind: 'QualifiedDeclaration', text: '.', subtree_size: 3},
 // CHECK:STDOUT: {kind: 'Namespace', text: ';', subtree_size: 5},
 // CHECK:STDOUT:     {kind: 'FunctionIntroducer', text: 'fn'},
-// CHECK:STDOUT:         {kind: 'Name', text: 'Foo'},
+// CHECK:STDOUT:         {kind: 'NameExpression', text: 'Foo'},
 // CHECK:STDOUT:         {kind: 'Name', text: 'Bar'},
 // CHECK:STDOUT:       {kind: 'QualifiedDeclaration', text: '.', subtree_size: 3},
 // CHECK:STDOUT:       {kind: 'Name', text: 'Baz'},

toolchain/semantics/BUILD

@@ -59,6 +59,7 @@ cc_library(
         "semantics_ir.cpp",
         "semantics_context.cpp",
         "semantics_node_block_stack.cpp",
+        "semantics_declaration_name_stack.cpp",
     ] +
     # Glob handler files to avoid missing anyway.
     glob([
@@ -66,6 +67,7 @@ cc_library(
     ]),
     hdrs = [
         "semantics_context.h",
+        "semantics_declaration_name_stack.h",
         "semantics_ir.h",
         "semantics_node_block_stack.h",
     ],

toolchain/semantics/semantics_context.cpp

@@ -10,14 +10,13 @@
 #include "toolchain/diagnostics/diagnostic_kind.h"
 #include "toolchain/lexer/tokenized_buffer.h"
 #include "toolchain/parser/parse_node_kind.h"
+#include "toolchain/semantics/semantics_declaration_name_stack.h"
 #include "toolchain/semantics/semantics_ir.h"
 #include "toolchain/semantics/semantics_node.h"
 #include "toolchain/semantics/semantics_node_block_stack.h"
 
 namespace Carbon {
 
-CARBON_DIAGNOSTIC(NameNotFound, Error, "Name {0} not found", llvm::StringRef);
-
 SemanticsContext::SemanticsContext(const TokenizedBuffer& tokens,
                                    DiagnosticEmitter<ParseTree::Node>& emitter,
                                    const ParseTree& parse_tree,
@@ -31,8 +30,8 @@ SemanticsContext::SemanticsContext(const TokenizedBuffer& tokens,
       node_stack_(parse_tree, vlog_stream),
       node_block_stack_("node_block_stack_", semantics_ir, vlog_stream),
       params_or_args_stack_("params_or_args_stack_", semantics_ir, vlog_stream),
-      args_type_info_stack_("args_type_info_stack_", semantics_ir,
-                            vlog_stream) {
+      args_type_info_stack_("args_type_info_stack_", semantics_ir, vlog_stream),
+      declaration_name_stack_(this) {
   // Inserts the "Error" and "Type" types as "used types" so that
   // canonicalization can skip them. We don't emit either for lowering.
   canonical_types_.insert(
@@ -75,10 +74,24 @@ auto SemanticsContext::AddNodeAndPush(ParseTree::Node parse_node,
   node_stack_.Push(parse_node, node_id);
 }
 
-CARBON_DIAGNOSTIC(NameDeclarationDuplicate, Error,
-                  "Duplicate name being declared in the same scope.");
-CARBON_DIAGNOSTIC(NameDeclarationPrevious, Note,
-                  "Name is previously declared here.");
+auto SemanticsContext::DiagnoseDuplicateName(ParseTree::Node parse_node,
+                                             SemanticsNodeId prev_def_id)
+    -> void {
+  CARBON_DIAGNOSTIC(NameDeclarationDuplicate, Error,
+                    "Duplicate name being declared in the same scope.");
+  CARBON_DIAGNOSTIC(NameDeclarationPrevious, Note,
+                    "Name is previously declared here.");
+  auto prev_def = semantics_ir_->GetNode(prev_def_id);
+  emitter_->Build(parse_node, NameDeclarationDuplicate)
+      .Note(prev_def.parse_node(), NameDeclarationPrevious)
+      .Emit();
+}
+
+auto SemanticsContext::DiagnoseNameNotFound(ParseTree::Node parse_node,
+                                            SemanticsStringId name_id) -> void {
+  CARBON_DIAGNOSTIC(NameNotFound, Error, "Name {0} not found", llvm::StringRef);
+  emitter_->Emit(parse_node, NameNotFound, semantics_ir_->GetString(name_id));
+}
 
 auto SemanticsContext::AddNameToLookup(ParseTree::Node name_node,
                                        SemanticsStringId name_id,
@@ -86,48 +99,7 @@ auto SemanticsContext::AddNameToLookup(ParseTree::Node name_node,
   if (current_scope().names.insert(name_id).second) {
     name_lookup_[name_id].push_back(target_id);
   } else {
-    auto prev_def_id = name_lookup_[name_id].back();
-    auto prev_def = semantics_ir_->GetNode(prev_def_id);
-    emitter_->Build(name_node, NameDeclarationDuplicate)
-        .Note(prev_def.parse_node(), NameDeclarationPrevious)
-        .Emit();
-  }
-}
-
-auto SemanticsContext::AddNameToLookup(DeclarationNameContext name_context,
-                                       SemanticsNodeId target_id) -> void {
-  switch (name_context.state) {
-    case DeclarationNameContext::State::Error:
-      // The name is invalid and a diagnostic has already been emitted.
-      return;
-
-    case DeclarationNameContext::State::New:
-      CARBON_FATAL() << "Name is missing, not expected to call AddNameToLookup "
-                        "(but that may change based on error handling).";
-
-    case DeclarationNameContext::State::Resolved:
-    case DeclarationNameContext::State::ResolvedNonScope: {
-      auto prev_def = semantics_ir_->GetNode(name_context.resolved_node_id);
-      emitter_->Build(name_context.parse_node, NameDeclarationDuplicate)
-          .Note(prev_def.parse_node(), NameDeclarationPrevious)
-          .Emit();
-      return;
-    }
-
-    case DeclarationNameContext::State::Unresolved:
-      if (name_context.target_scope_id == SemanticsNameScopeId::Invalid) {
-        AddNameToLookup(name_context.parse_node,
-                        name_context.unresolved_name_id, target_id);
-      } else {
-        bool success = semantics_ir_->AddNameScopeEntry(
-            name_context.target_scope_id, name_context.unresolved_name_id,
-            target_id);
-        CARBON_CHECK(success)
-            << "Duplicate names should have been resolved previously: "
-            << name_context.unresolved_name_id << " in "
-            << name_context.target_scope_id;
-      }
-      return;
+    DiagnoseDuplicateName(name_node, name_lookup_[name_id].back());
   }
 }
 
@@ -139,8 +111,7 @@ auto SemanticsContext::LookupName(ParseTree::Node parse_node,
     auto it = name_lookup_.find(name_id);
     if (it == name_lookup_.end()) {
       if (print_diagnostics) {
-        emitter_->Emit(parse_node, NameNotFound,
-                       semantics_ir_->GetString(name_id));
+        DiagnoseNameNotFound(parse_node, name_id);
       }
       return SemanticsNodeId::BuiltinError;
     }
@@ -154,8 +125,7 @@ auto SemanticsContext::LookupName(ParseTree::Node parse_node,
     auto it = scope.find(name_id);
     if (it == scope.end()) {
       if (print_diagnostics) {
-        emitter_->Emit(parse_node, NameNotFound,
-                       semantics_ir_->GetString(name_id));
+        DiagnoseNameNotFound(parse_node, name_id);
       }
       return SemanticsNodeId::BuiltinError;
     }
@@ -295,106 +265,6 @@ auto SemanticsContext::is_current_position_reachable() -> bool {
   }
 }
 
-auto SemanticsContext::PushDeclarationName() -> void {
-  declaration_name_stack_.push_back(
-      {.state = DeclarationNameContext::State::New,
-       .target_scope_id = SemanticsNameScopeId::Invalid,
-       .resolved_node_id = SemanticsNodeId::Invalid});
-}
-
-auto SemanticsContext::PopDeclarationName() -> DeclarationNameContext {
-  if (parse_tree_->node_kind(node_stack().PeekParseNode()) ==
-      ParseNodeKind::QualifiedDeclaration) {
-    // Any parts from a QualifiedDeclaration will already have been processed
-    // into the name.
-    node_stack_
-        .PopAndDiscardSoloParseNode<ParseNodeKind::QualifiedDeclaration>();
-  } else {
-    // The name had no qualifiers, so we need to process the node now.
-    auto [parse_node, node_or_name_id] =
-        node_stack_.PopExpressionWithParseNode();
-    ApplyDeclarationNameQualifier(parse_node, node_or_name_id);
-  }
-
-  return declaration_name_stack_.pop_back_val();
-}
-
-auto SemanticsContext::ApplyDeclarationNameQualifier(
-    ParseTree::Node parse_node, SemanticsNodeId node_or_name_id) -> void {
-  auto& name_context = declaration_name_stack_.back();
-  switch (name_context.state) {
-    case DeclarationNameContext::State::Error:
-      // Already in an error state, so return without examining.
-      return;
-
-    case DeclarationNameContext::State::Unresolved:
-      // Because more qualifiers were found, we diagnose that the earlier
-      // qualifier failed to resolve.
-      name_context.state = DeclarationNameContext::State::Error;
-      emitter_->Emit(name_context.parse_node, NameNotFound,
-                     semantics_ir_->GetString(name_context.unresolved_name_id));
-      return;
-
-    case DeclarationNameContext::State::ResolvedNonScope: {
-      // Because more qualifiers were found, we diagnose that the earlier
-      // qualifier didn't resolve to a scoped entity.
-      name_context.state = DeclarationNameContext::State::Error;
-      CARBON_DIAGNOSTIC(QualifiedDeclarationInNonScope, Error,
-                        "Declaration qualifiers are only allowed for entities "
-                        "that provide a scope.");
-      CARBON_DIAGNOSTIC(QualifiedDeclarationNonScopeEntity, Note,
-                        "Non-scope entity referenced here.");
-      emitter_->Build(parse_node, QualifiedDeclarationInNonScope)
-          .Note(name_context.parse_node, QualifiedDeclarationNonScopeEntity)
-          .Emit();
-      return;
-    }
-
-    case DeclarationNameContext::State::New:
-    case DeclarationNameContext::State::Resolved: {
-      name_context.parse_node = parse_node;
-      if (parse_tree().node_kind(name_context.parse_node) ==
-          ParseNodeKind::Name) {
-        // For identifier nodes, we need to perform a lookup on the identifier.
-        // This means the input node_id is actually a string ID.
-        SemanticsStringId name_id(node_or_name_id.index);
-        auto resolved_node_id = LookupName(name_context.parse_node, name_id,
-                                           name_context.target_scope_id,
-                                           /*print_diagnostics=*/false);
-        if (resolved_node_id == SemanticsNodeId::BuiltinError) {
-          // Invalid indicates an unresolved node. Store it and return.
-          name_context.state = DeclarationNameContext::State::Unresolved;
-          name_context.unresolved_name_id = name_id;
-          return;
-        } else {
-          // Store the resolved node and continue for the target scope update.
-          name_context.resolved_node_id = resolved_node_id;
-        }
-      } else {
-        // For other nodes, we expect a regular resolved node, for example a
-        // namespace or generic type. Store it and continue for the target scope
-        // update.
-        name_context.resolved_node_id = node_or_name_id;
-      }
-
-      // This will only be reached for resolved nodes. We update the target
-      // scope based on the resolved type.
-      auto resolved_node =
-          semantics_ir_->GetNode(name_context.resolved_node_id);
-      switch (resolved_node.kind()) {
-        case SemanticsNodeKind::Namespace:
-          name_context.state = DeclarationNameContext::State::Resolved;
-          name_context.target_scope_id = resolved_node.GetAsNamespace();
-          break;
-        default:
-          name_context.state = DeclarationNameContext::State::ResolvedNonScope;
-          break;
-      }
-      return;
-    }
-  }
-}
-
 auto SemanticsContext::ImplicitAsForArgs(
     SemanticsNodeBlockId arg_refs_id, ParseTree::Node param_parse_node,
     SemanticsNodeBlockId param_refs_id,

toolchain/semantics/semantics_context.h

@@ -10,6 +10,7 @@
 #include "llvm/ADT/FoldingSet.h"
 #include "llvm/ADT/SmallVector.h"
 #include "toolchain/parser/parse_tree.h"
+#include "toolchain/semantics/semantics_declaration_name_stack.h"
 #include "toolchain/semantics/semantics_ir.h"
 #include "toolchain/semantics/semantics_node.h"
 #include "toolchain/semantics/semantics_node_block_stack.h"
@@ -20,83 +21,6 @@ namespace Carbon {
 // Context and shared functionality for semantics handlers.
 class SemanticsContext {
  public:
-  // Context for declaration name construction, tracked in
-  // declaration_name_stack_.
-  //
-  // A qualified declaration name will consist of entries which are either
-  // Identifiers or full expressions. Expressions are expected to resolve to
-  // types, such as how `fn Vector(i32).Clear() { ... }` uses the expression
-  // `Vector(i32)` to indicate the type whose member is being declared.
-  // Identifiers such as `Clear` will be resolved to a name if possible, for
-  // example when declaring things that are in a non-generic type or namespace,
-  // and are otherwise marked as an unresolved identifier.
-  //
-  // Unresolved identifiers are valid if and only if they are the last step of a
-  // qualified name; all resolved qualifiers must resolve to an entity with
-  // members, such as a namespace. Resolved identifiers in the last step will
-  // occur for both out-of-line definitions and new declarations, depending on
-  // context.
-  //
-  // Example state transitions:
-  //
-  // ```
-  // // New -> Unresolved, because `MyNamespace` is newly declared.
-  // namespace MyNamespace;
-  //
-  // // New -> Resolved -> Unresolved, because `MyType` is newly declared.
-  // class MyNamespace.MyType;
-  //
-  // // New -> Resolved -> Resolved, because `MyType` was forward declared.
-  // class MyNamespace.MyType {
-  //   // New -> Unresolved, because `DoSomething` is newly declared.
-  //   fn DoSomething();
-  // }
-  //
-  // // New -> Resolved -> Resolved -> ResolvedNonScope, because `DoSomething`
-  // // is forward declared in `MyType`, but is not a scope itself.
-  // fn MyNamespace.MyType.DoSomething() { ... }
-  // ```
-  struct DeclarationNameContext {
-    enum class State {
-      // A new context which has not processed any parts of the qualifier.
-      New,
-
-      // A node ID has been resolved, whether through an identifier or
-      // expression. This provided a new scope, such as a type.
-      Resolved,
-
-      // A node ID has been resolved, whether through an identifier or
-      // expression. It did not provide a new scope, so must be the final part,
-      // such as an out-of-line function definition.
-      ResolvedNonScope,
-
-      // An identifier didn't resolve.
-      Unresolved,
-
-      // An error has occurred, such as an additional qualifier past an
-      // unresolved name. No new diagnostics should be emitted.
-      Error,
-    };
-
-    State state = State::New;
-
-    // The scope which qualified names are added to. For unqualified names,
-    // this will be Invalid to indicate the current scope should be used.
-    SemanticsNameScopeId target_scope_id = SemanticsNameScopeId::Invalid;
-
-    // The last parse node used.
-    ParseTree::Node parse_node = ParseTree::Node::Invalid;
-
-    union {
-      // The ID of a resolved qualifier, including both identifiers and
-      // expressions. Invalid indicates resolution failed.
-      SemanticsNodeId resolved_node_id = SemanticsNodeId::Invalid;
-
-      // The ID of an unresolved identifier.
-      SemanticsStringId unresolved_name_id;
-    };
-  };
-
   // Stores references for work.
   explicit SemanticsContext(const TokenizedBuffer& tokens,
                             DiagnosticEmitter<ParseTree::Node>& emitter,
@@ -124,16 +48,20 @@ class SemanticsContext {
   auto AddNameToLookup(ParseTree::Node name_node, SemanticsStringId name_id,
                        SemanticsNodeId target_id) -> void;
 
-  // Adds a name to name lookup. Prints a diagnostic for name conflicts.
-  auto AddNameToLookup(DeclarationNameContext name_context,
-                       SemanticsNodeId target_id) -> void;
-
   // Performs name lookup in a specified scope, returning the referenced node.
   // If scope_id is invalid, uses the current contextual scope.
   auto LookupName(ParseTree::Node parse_node, SemanticsStringId name_id,
                   SemanticsNameScopeId scope_id, bool print_diagnostics)
       -> SemanticsNodeId;
 
+  // Prints a diagnostic for a duplicate name.
+  auto DiagnoseDuplicateName(ParseTree::Node parse_node,
+                             SemanticsNodeId prev_def_id) -> void;
+
+  // Prints a diagnostic for a missing name.
+  auto DiagnoseNameNotFound(ParseTree::Node parse_node,
+                            SemanticsStringId name_id) -> void;
+
   // Pushes a new scope onto scope_stack_.
   auto PushScope() -> void;
 
@@ -182,20 +110,6 @@ class SemanticsContext {
   // Returns whether the current position in the current block is reachable.
   auto is_current_position_reachable() -> bool;
 
-  // Pushes processing of a new declaration name, which will be used
-  // contextually.
-  auto PushDeclarationName() -> void;
-
-  // Pops the current declaration name processing, returning the final context
-  // for adding the name to lookup. This also pops the final name node from the
-  // node stack, which will be applied to the declaration name if appropriate.
-  auto PopDeclarationName() -> DeclarationNameContext;
-
-  // Applies an entry from the node stack to the top of the declaration name
-  // stack.
-  auto ApplyDeclarationNameQualifier(ParseTree::Node parse_node,
-                                     SemanticsNodeId node_or_name_id) -> void;
-
   // Runs ImplicitAsImpl for a set of arguments and parameters.
   //
   // This will eventually need to support checking against multiple possible
@@ -286,6 +200,10 @@ class SemanticsContext {
     return return_scope_stack_;
   }
 
+  auto declaration_name_stack() -> SemanticsDeclarationNameStack& {
+    return declaration_name_stack_;
+  }
+
  private:
   // For CanImplicitAs, the detected conversion to apply.
   enum ImplicitAsKind {
@@ -372,9 +290,8 @@ class SemanticsContext {
   // A stack for scope context.
   llvm::SmallVector<ScopeStackEntry> scope_stack_;
 
-  // A stack for declaration name context. See DeclarationNameContext for
-  // details.
-  llvm::SmallVector<DeclarationNameContext> declaration_name_stack_;
+  // The stack used for qualified declaration name construction.
+  SemanticsDeclarationNameStack declaration_name_stack_;
 
   // Maps identifiers to name lookup results. Values are a stack of name lookup
   // results in the ancestor scopes. This offers constant-time lookup of names,

toolchain/semantics/semantics_declaration_name_stack.cpp

@@ -0,0 +1,170 @@
+// 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/semantics/semantics_declaration_name_stack.h"
+
+#include "toolchain/semantics/semantics_context.h"
+
+namespace Carbon {
+
+auto SemanticsDeclarationNameStack::Push() -> void {
+  declaration_name_stack_.push_back(
+      {.state = Context::State::New,
+       .target_scope_id = SemanticsNameScopeId::Invalid,
+       .resolved_node_id = SemanticsNodeId::Invalid});
+}
+
+auto SemanticsDeclarationNameStack::Pop() -> Context {
+  if (context_->parse_tree().node_kind(
+          context_->node_stack().PeekParseNode()) ==
+      ParseNodeKind::QualifiedDeclaration) {
+    // Any parts from a QualifiedDeclaration will already have been processed
+    // into the name.
+    context_->node_stack()
+        .PopAndDiscardSoloParseNode<ParseNodeKind::QualifiedDeclaration>();
+  } else {
+    // The name had no qualifiers, so we need to process the node now.
+    auto [parse_node, name_id] =
+        context_->node_stack().PopWithParseNode<ParseNodeKind::Name>();
+    ApplyNameQualifier(parse_node, name_id);
+  }
+
+  return declaration_name_stack_.pop_back_val();
+}
+
+auto SemanticsDeclarationNameStack::AddNameToLookup(Context name_context,
+                                                    SemanticsNodeId target_id)
+    -> void {
+  switch (name_context.state) {
+    case Context::State::Error:
+      // The name is invalid and a diagnostic has already been emitted.
+      return;
+
+    case Context::State::New:
+      CARBON_FATAL() << "Name is missing, not expected to call AddNameToLookup "
+                        "(but that may change based on error handling).";
+
+    case Context::State::Resolved:
+    case Context::State::ResolvedNonScope: {
+      context_->DiagnoseDuplicateName(name_context.parse_node,
+                                      name_context.resolved_node_id);
+      return;
+    }
+
+    case Context::State::Unresolved:
+      if (name_context.target_scope_id == SemanticsNameScopeId::Invalid) {
+        context_->AddNameToLookup(name_context.parse_node,
+                                  name_context.unresolved_name_id, target_id);
+      } else {
+        bool success = context_->semantics_ir().AddNameScopeEntry(
+            name_context.target_scope_id, name_context.unresolved_name_id,
+            target_id);
+        CARBON_CHECK(success)
+            << "Duplicate names should have been resolved previously: "
+            << name_context.unresolved_name_id << " in "
+            << name_context.target_scope_id;
+      }
+      return;
+  }
+}
+
+auto SemanticsDeclarationNameStack::ApplyExpressionQualifier(
+    ParseTree::Node parse_node, SemanticsNodeId node_id) -> void {
+  auto& name_context = declaration_name_stack_.back();
+  if (CanResolveQualifier(name_context, parse_node)) {
+    if (node_id == SemanticsNodeId::BuiltinError) {
+      // The input node is an error, so error the context.
+      name_context.state = Context::State::Error;
+      return;
+    }
+
+    // For other nodes, we expect a regular resolved node, for example a
+    // namespace or generic type. Store it and continue for the target scope
+    // update.
+    name_context.resolved_node_id = node_id;
+
+    UpdateScopeIfNeeded(name_context);
+  }
+}
+
+auto SemanticsDeclarationNameStack::ApplyNameQualifier(
+    ParseTree::Node parse_node, SemanticsStringId name_id) -> void {
+  auto& name_context = declaration_name_stack_.back();
+  if (CanResolveQualifier(name_context, parse_node)) {
+    // For identifier nodes, we need to perform a lookup on the identifier.
+    // This means the input node_id is actually a string ID.
+    auto resolved_node_id = context_->LookupName(
+        name_context.parse_node, name_id, name_context.target_scope_id,
+        /*print_diagnostics=*/false);
+    if (resolved_node_id == SemanticsNodeId::BuiltinError) {
+      // Invalid indicates an unresolved node. Store it and return.
+      name_context.state = Context::State::Unresolved;
+      name_context.unresolved_name_id = name_id;
+      return;
+    } else {
+      // Store the resolved node and continue for the target scope update.
+      name_context.resolved_node_id = resolved_node_id;
+    }
+
+    UpdateScopeIfNeeded(name_context);
+  }
+}
+
+auto SemanticsDeclarationNameStack::UpdateScopeIfNeeded(Context& name_context)
+    -> void {
+  // This will only be reached for resolved nodes. We update the target
+  // scope based on the resolved type.
+  auto resolved_node =
+      context_->semantics_ir().GetNode(name_context.resolved_node_id);
+  switch (resolved_node.kind()) {
+    case SemanticsNodeKind::Namespace:
+      name_context.state = Context::State::Resolved;
+      name_context.target_scope_id = resolved_node.GetAsNamespace();
+      break;
+    default:
+      name_context.state = Context::State::ResolvedNonScope;
+      break;
+  }
+}
+
+auto SemanticsDeclarationNameStack::CanResolveQualifier(
+    Context& name_context, ParseTree::Node parse_node) -> bool {
+  switch (name_context.state) {
+    case Context::State::Error:
+      // Already in an error state, so return without examining.
+      return false;
+
+    case Context::State::Unresolved:
+      // Because more qualifiers were found, we diagnose that the earlier
+      // qualifier failed to resolve.
+      name_context.state = Context::State::Error;
+      context_->DiagnoseNameNotFound(name_context.parse_node,
+                                     name_context.unresolved_name_id);
+      return false;
+
+    case Context::State::ResolvedNonScope: {
+      // Because more qualifiers were found, we diagnose that the earlier
+      // qualifier didn't resolve to a scoped entity.
+      name_context.state = Context::State::Error;
+      CARBON_DIAGNOSTIC(QualifiedDeclarationInNonScope, Error,
+                        "Declaration qualifiers are only allowed for entities "
+                        "that provide a scope.");
+      CARBON_DIAGNOSTIC(QualifiedDeclarationNonScopeEntity, Note,
+                        "Non-scope entity referenced here.");
+      context_->emitter()
+          .Build(parse_node, QualifiedDeclarationInNonScope)
+          .Note(name_context.parse_node, QualifiedDeclarationNonScopeEntity)
+          .Emit();
+      return false;
+    }
+
+    case Context::State::New:
+    case Context::State::Resolved: {
+      name_context.parse_node = parse_node;
+      return true;
+    }
+  }
+}
+
+}  // namespace Carbon

toolchain/semantics/semantics_declaration_name_stack.h

@@ -0,0 +1,139 @@
+// 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_SEMANTICS_SEMANTICS_DECLARATION_NAME_STACK_H_
+#define CARBON_TOOLCHAIN_SEMANTICS_SEMANTICS_DECLARATION_NAME_STACK_H_
+
+#include "llvm/ADT/SmallVector.h"
+#include "toolchain/parser/parse_tree.h"
+#include "toolchain/semantics/semantics_node.h"
+
+namespace Carbon {
+
+class SemanticsContext;
+
+// Provides support and stacking for qualified declaration name handling.
+//
+// A qualified declaration name will consist of entries which are either
+// Identifiers or full expressions. Expressions are expected to resolve to
+// types, such as how `fn Vector(i32).Clear() { ... }` uses the expression
+// `Vector(i32)` to indicate the type whose member is being declared.
+// Identifiers such as `Clear` will be resolved to a name if possible, for
+// example when declaring things that are in a non-generic type or namespace,
+// and are otherwise marked as an unresolved identifier.
+//
+// Unresolved identifiers are valid if and only if they are the last step of a
+// qualified name; all resolved qualifiers must resolve to an entity with
+// members, such as a namespace. Resolved identifiers in the last step will
+// occur for both out-of-line definitions and new declarations, depending on
+// context.
+//
+// Example state transitions:
+//
+// ```
+// // New -> Unresolved, because `MyNamespace` is newly declared.
+// namespace MyNamespace;
+//
+// // New -> Resolved -> Unresolved, because `MyType` is newly declared.
+// class MyNamespace.MyType;
+//
+// // New -> Resolved -> Resolved, because `MyType` was forward declared.
+// class MyNamespace.MyType {
+//   // New -> Unresolved, because `DoSomething` is newly declared.
+//   fn DoSomething();
+// }
+//
+// // New -> Resolved -> Resolved -> ResolvedNonScope, because `DoSomething`
+// // is forward declared in `MyType`, but is not a scope itself.
+// fn MyNamespace.MyType.DoSomething() { ... }
+// ```
+class SemanticsDeclarationNameStack {
+ public:
+  // Context for declaration name construction.
+  struct Context {
+    enum class State {
+      // A new context which has not processed any parts of the qualifier.
+      New,
+
+      // A node ID has been resolved, whether through an identifier or
+      // expression. This provided a new scope, such as a type.
+      Resolved,
+
+      // A node ID has been resolved, whether through an identifier or
+      // expression. It did not provide a new scope, so must be the final part,
+      // such as an out-of-line function definition.
+      ResolvedNonScope,
+
+      // An identifier didn't resolve.
+      Unresolved,
+
+      // An error has occurred, such as an additional qualifier past an
+      // unresolved name. No new diagnostics should be emitted.
+      Error,
+    };
+
+    State state = State::New;
+
+    // The scope which qualified names are added to. For unqualified names,
+    // this will be Invalid to indicate the current scope should be used.
+    SemanticsNameScopeId target_scope_id = SemanticsNameScopeId::Invalid;
+
+    // The last parse node used.
+    ParseTree::Node parse_node = ParseTree::Node::Invalid;
+
+    union {
+      // The ID of a resolved qualifier, including both identifiers and
+      // expressions. Invalid indicates resolution failed.
+      SemanticsNodeId resolved_node_id = SemanticsNodeId::Invalid;
+
+      // The ID of an unresolved identifier.
+      SemanticsStringId unresolved_name_id;
+    };
+  };
+
+  explicit SemanticsDeclarationNameStack(SemanticsContext* context)
+      : context_(context) {}
+
+  // Pushes processing of a new declaration name, which will be used
+  // contextually.
+  auto Push() -> void;
+
+  // Pops the current declaration name processing, returning the final context
+  // for adding the name to lookup. This also pops the final name node from the
+  // node stack, which will be applied to the declaration name if appropriate.
+  auto Pop() -> Context;
+
+  // Applies an expression from the node stack to the top of the declaration
+  // name stack.
+  auto ApplyExpressionQualifier(ParseTree::Node parse_node,
+                                SemanticsNodeId node_id) -> void;
+
+  // Applies a Name from the node stack to the top of the declaration name
+  // stack.
+  auto ApplyNameQualifier(ParseTree::Node parse_node, SemanticsStringId name_id)
+      -> void;
+
+  // Adds a name to name lookup. Prints a diagnostic for name conflicts.
+  auto AddNameToLookup(Context name_context, SemanticsNodeId target_id) -> void;
+
+ private:
+  // Returns true if the context is in a state where it can resolve qualifiers.
+  // Updates name_context as needed.
+  auto CanResolveQualifier(Context& name_context, ParseTree::Node parse_node)
+      -> bool;
+
+  // Updates the scope on name_context as needed. This is called after
+  // resolution is complete, whether for Name or expression.
+  auto UpdateScopeIfNeeded(Context& name_context) -> void;
+
+  // The linked context.
+  SemanticsContext* context_;
+
+  // Provides nesting for construction.
+  llvm::SmallVector<Context> declaration_name_stack_;
+};
+
+}  // namespace Carbon
+
+#endif  // CARBON_TOOLCHAIN_SEMANTICS_SEMANTICS_DECLARATION_NAME_STACK_H_

toolchain/semantics/semantics_handle_function.cpp

@@ -21,7 +21,7 @@ static auto BuildFunctionDeclaration(SemanticsContext& context)
   }
   SemanticsNodeBlockId param_refs_id =
       context.node_stack().Pop<ParseNodeKind::ParameterList>();
-  auto name_context = context.PopDeclarationName();
+  auto name_context = context.declaration_name_stack().Pop();
   auto fn_node = context.node_stack()
                      .PopForSoloParseNode<ParseNodeKind::FunctionIntroducer>();
 
@@ -36,7 +36,7 @@ static auto BuildFunctionDeclaration(SemanticsContext& context)
        .body_block_ids = {}});
   auto decl_id = context.AddNode(
       SemanticsNode::FunctionDeclaration::Make(fn_node, function_id));
-  context.AddNameToLookup(name_context, decl_id);
+  context.declaration_name_stack().AddNameToLookup(name_context, decl_id);
   return {function_id, decl_id};
 }
 
@@ -103,7 +103,7 @@ auto SemanticsHandleFunctionIntroducer(SemanticsContext& context,
   // Push the bracketing node.
   context.node_stack().Push(parse_node);
   // A name should always follow.
-  context.PushDeclarationName();
+  context.declaration_name_stack().Push();
   return true;
 }
 

toolchain/semantics/semantics_handle_name.cpp

@@ -87,25 +87,34 @@ auto SemanticsHandleNameExpression(SemanticsContext& context,
 
 auto SemanticsHandleQualifiedDeclaration(SemanticsContext& context,
                                          ParseTree::Node parse_node) -> bool {
-  // The first two qualifiers in a chain will be a QualifiedDeclaration with two
-  // Identifier or expression children. Later qualifiers will have a
-  // QualifiedDeclaration as the first child, and an Identifier or expression as
-  // the second child.
-  auto [parse_node2, node_or_name_id2] =
-      context.node_stack().PopExpressionWithParseNode();
-  if (context.parse_tree().node_kind(context.node_stack().PeekParseNode()) !=
-      ParseNodeKind::QualifiedDeclaration) {
-    // First QualifiedDeclaration in a chain.
-    auto [parse_node1, node_or_name_id1] =
-        context.node_stack().PopExpressionWithParseNode();
-    context.ApplyDeclarationNameQualifier(parse_node1, node_or_name_id1);
-    // Add the QualifiedDeclaration so that it can be used for bracketing.
-    context.node_stack().Push(parse_node);
+  auto pop_and_apply_first_child = [&]() {
+    if (context.parse_tree().node_kind(context.node_stack().PeekParseNode()) !=
+        ParseNodeKind::QualifiedDeclaration) {
+      // First QualifiedDeclaration in a chain.
+      auto [parse_node1, node_id1] =
+          context.node_stack().PopExpressionWithParseNode();
+      context.declaration_name_stack().ApplyExpressionQualifier(parse_node1,
+                                                                node_id1);
+      // Add the QualifiedDeclaration so that it can be used for bracketing.
+      context.node_stack().Push(parse_node);
+    } else {
+      // Nothing to do: the QualifiedDeclaration remains as a bracketing node
+      // for later QualifiedDeclarations.
+    }
+  };
+
+  ParseTree::Node parse_node2 = context.node_stack().PeekParseNode();
+  if (context.parse_tree().node_kind(parse_node2) == ParseNodeKind::Name) {
+    SemanticsStringId name_id2 =
+        context.node_stack().Pop<ParseNodeKind::Name>();
+    pop_and_apply_first_child();
+    context.declaration_name_stack().ApplyNameQualifier(parse_node2, name_id2);
   } else {
-    // Nothing to do: the QualifiedDeclaration remains as a bracketing node for
-    // later QualifiedDeclarations.
+    SemanticsNodeId node_id2 = context.node_stack().PopExpression();
+    pop_and_apply_first_child();
+    context.declaration_name_stack().ApplyExpressionQualifier(parse_node2,
+                                                              node_id2);
   }
-  context.ApplyDeclarationNameQualifier(parse_node2, node_or_name_id2);
 
   return true;
 }

toolchain/semantics/semantics_handle_namespace.cpp

@@ -9,16 +9,16 @@ namespace Carbon {
 
 auto SemanticsHandleNamespaceStart(SemanticsContext& context,
                                    ParseTree::Node /*parse_node*/) -> bool {
-  context.PushDeclarationName();
+  context.declaration_name_stack().Push();
   return true;
 }
 
 auto SemanticsHandleNamespace(SemanticsContext& context,
                               ParseTree::Node parse_node) -> bool {
-  auto name_context = context.PopDeclarationName();
+  auto name_context = context.declaration_name_stack().Pop();
   auto namespace_id = context.AddNode(SemanticsNode::Namespace::Make(
       parse_node, context.semantics_ir().AddNameScope()));
-  context.AddNameToLookup(name_context, namespace_id);
+  context.declaration_name_stack().AddNameToLookup(name_context, namespace_id);
   return true;
 }
 

toolchain/semantics/semantics_node_stack.h

@@ -82,7 +82,8 @@ class SemanticsNodeStack {
     PopForSoloParseNode<RequiredParseKind>();
   }
 
-  // Pops the top of the stack and returns the parse_node and the ID.
+  // Pops an expression from the top of the stack and returns the parse_node and
+  // the ID.
   auto PopExpressionWithParseNode()
       -> std::pair<ParseTree::Node, SemanticsNodeId> {
     return PopWithParseNode<SemanticsNodeId>();
@@ -325,12 +326,6 @@ class SemanticsNodeStack {
 
   // Require a ParseNodeKind be mapped to a particular IdKind.
   auto RequireIdKind(ParseNodeKind parse_kind, IdKind id_kind) -> void {
-    // TODO: Name can be popped as a node_id by declaration name handling. Will
-    // refactor to remove this quirk.
-    if (parse_kind == ParseNodeKind::Name &&
-        id_kind == IdKind::SemanticsNodeId) {
-      return;
-    }
     CARBON_CHECK(ParseNodeKindToIdKind(parse_kind) == id_kind)
         << "Unexpected IdKind mapping for " << parse_kind;
   }

toolchain/semantics/testdata/namespace/fail_unresolved_scope.carbon

@@ -5,7 +5,7 @@
 // AUTOUPDATE
 // CHECK:STDOUT: cross_reference_irs_size: 1
 // CHECK:STDOUT: functions: [
-// CHECK:STDOUT:   {name: str0, param_refs: block0, body: {block2}}},
+// CHECK:STDOUT:   {name: str<invalid>, param_refs: block0, body: {block2}}},
 // CHECK:STDOUT: ]
 // CHECK:STDOUT: integer_literals: [
 // CHECK:STDOUT: ]