Rework name lookup to handle non-lexical scoping. (#3354)

When declaring a name such as `fn Ns.Class.F() { ... }`, enter the
scopes of `Ns` and `Ns.Class` as we form the name, and remain in those
non-lexical scopes until the end of the declaration.

When performing an unqualified lookup, look in any enclosing non-lexical
scopes in addition to looking into the lexical name table.

We now track a scope index with each lookup result in the lexical name
lookup table. This is used to determine whether a lexical or non-lexcial
result is the innermost result and whether a declared name is in the
same scope as some previous introduction of that name or in a nested
scope. For now, this could just be the index into the scope_stack, but
the intent is to also use this to detect names being declared after they
are first looked up, which requires the indexes to outlive their scopes,
so we use a persistent numbering of all scopes instead. The persistent
numbering also permits more invariant checking.

toolchain/check/context.cpp

@@ -115,52 +115,144 @@ auto Context::NoteIncompleteClass(SemIR::ClassId class_id,
 auto Context::AddNameToLookup(Parse::Node name_node, IdentifierId name_id,
                               SemIR::InstId target_id) -> void {
   if (current_scope().names.insert(name_id).second) {
-    name_lookup_[name_id].push_back(target_id);
+    // TODO: Reject if we previously performed a failed lookup for this name in
+    // this scope or a scope nested within it.
+    auto& lexical_results = name_lookup_[name_id];
+    CARBON_CHECK(lexical_results.empty() ||
+                 lexical_results.back().scope_index < current_scope_index())
+        << "Failed to clean up after scope nested within the current scope";
+    lexical_results.push_back(
+        {.node_id = target_id, .scope_index = current_scope_index()});
   } else {
-    DiagnoseDuplicateName(name_node, name_lookup_[name_id].back());
+    DiagnoseDuplicateName(name_node, name_lookup_[name_id].back().node_id);
   }
 }
 
-auto Context::LookupName(Parse::Node parse_node, IdentifierId name_id,
-                         SemIR::NameScopeId scope_id, bool print_diagnostics)
+auto Context::LookupNameInDeclaration(Parse::Node parse_node,
+                                      IdentifierId name_id,
+                                      SemIR::NameScopeId scope_id)
     -> SemIR::InstId {
   if (scope_id == SemIR::NameScopeId::Invalid) {
-    auto it = name_lookup_.find(name_id);
-    if (it == name_lookup_.end()) {
-      if (print_diagnostics) {
-        DiagnoseNameNotFound(parse_node, name_id);
+    // Look for a name in the current scope only. There are two cases where the
+    // name would be in an outer scope:
+    //
+    //  - The name is the sole component of the declared name:
+    //
+    //    class A;
+    //    fn F() {
+    //      class A;
+    //    }
+    //
+    //    In this case, the inner A is not the same class as the outer A, so
+    //    lookup should not find the outer A.
+    //
+    //  - The name is a qualifier of some larger declared name:
+    //
+    //    class A { class B; }
+    //    fn F() {
+    //      class A.B {}
+    //    }
+    //
+    //    In this case, we're not in the correct scope to define a member of
+    //    class A, so we should reject, and we achieve this by not finding the
+    //    name A from the outer scope.
+    if (auto name_it = name_lookup_.find(name_id);
+        name_it != name_lookup_.end()) {
+      CARBON_CHECK(!name_it->second.empty())
+          << "Should have been erased: " << identifiers().Get(name_id);
+      auto result = name_it->second.back();
+      if (result.scope_index == current_scope_index()) {
+        return result.node_id;
       }
-      return SemIR::InstId::BuiltinError;
     }
-    CARBON_CHECK(!it->second.empty())
-        << "Should have been erased: " << identifiers().Get(name_id);
-
-    // TODO: Check for ambiguous lookups.
-    return it->second.back();
+    return SemIR::InstId::Invalid;
   } else {
-    const auto& scope = name_scopes().Get(scope_id);
-    auto it = scope.find(name_id);
-    if (it == scope.end()) {
-      if (print_diagnostics) {
-        DiagnoseNameNotFound(parse_node, name_id);
-      }
-      return SemIR::InstId::BuiltinError;
+    // TODO: Once we support `extend`, do not look into `extend`ed scopes here,
+    // following the same logic as above.
+    return LookupQualifiedName(parse_node, name_id, scope_id,
+                               /*required=*/false);
+  }
+}
+
+auto Context::LookupUnqualifiedName(Parse::Node parse_node,
+                                    IdentifierId name_id) -> SemIR::InstId {
+  // TODO: Check for shadowed lookup results.
+
+  // Find the results from enclosing lexical scopes. These will be combined with
+  // results from non-lexical scopes such as namespaces and classes.
+  llvm::ArrayRef<LexicalLookupResult> lexical_results;
+  if (auto name_it = name_lookup_.find(name_id);
+      name_it != name_lookup_.end()) {
+    lexical_results = name_it->second;
+    CARBON_CHECK(!lexical_results.empty())
+        << "Should have been erased: " << identifiers().Get(name_id);
+  }
+
+  // Walk the non-lexical scopes and perform lookups into each of them.
+  for (auto [index, name_scope_id] : llvm::reverse(non_lexical_scope_stack_)) {
+    // If the innermost lexical result is within this non-lexical scope, then
+    // it shadows all further non-lexical results and we're done.
+    if (!lexical_results.empty() &&
+        lexical_results.back().scope_index > index) {
+      return lexical_results.back().node_id;
     }
 
-    return it->second;
+    auto non_lexical_result =
+        LookupQualifiedName(parse_node, name_id, name_scope_id,
+                            /*required=*/false);
+    if (non_lexical_result.is_valid()) {
+      return non_lexical_result;
+    }
   }
+
+  if (!lexical_results.empty()) {
+    return lexical_results.back().node_id;
+  }
+
+  // We didn't find anything at all.
+  DiagnoseNameNotFound(parse_node, name_id);
+  return SemIR::InstId::BuiltinError;
+}
+
+auto Context::LookupQualifiedName(Parse::Node parse_node, IdentifierId name_id,
+                                  SemIR::NameScopeId scope_id, bool required)
+    -> SemIR::InstId {
+  CARBON_CHECK(scope_id.is_valid()) << "No scope to perform lookup into";
+  const auto& scope = name_scopes().Get(scope_id);
+  auto it = scope.find(name_id);
+  if (it == scope.end()) {
+    // TODO: Also perform lookups into `extend`ed scopes.
+    if (required) {
+      DiagnoseNameNotFound(parse_node, name_id);
+      return SemIR::InstId::BuiltinError;
+    }
+    return SemIR::InstId::Invalid;
+  }
+
+  return it->second;
 }
 
 auto Context::PushScope(SemIR::InstId scope_inst_id,
                         SemIR::NameScopeId scope_id) -> void {
-  scope_stack_.push_back(
-      {.scope_inst_id = scope_inst_id, .scope_id = scope_id});
+  scope_stack_.push_back({.index = next_scope_index_,
+                          .scope_inst_id = scope_inst_id,
+                          .scope_id = scope_id});
+  if (scope_id.is_valid()) {
+    non_lexical_scope_stack_.push_back({next_scope_index_, scope_id});
+  }
+
+  // TODO: Handle this case more gracefully.
+  CARBON_CHECK(next_scope_index_.index != std::numeric_limits<int32_t>::max())
+      << "Ran out of scopes";
+  ++next_scope_index_.index;
 }
 
 auto Context::PopScope() -> void {
   auto scope = scope_stack_.pop_back_val();
   for (const auto& str_id : scope.names) {
     auto it = name_lookup_.find(str_id);
+    CARBON_CHECK(it->second.back().scope_index == scope.index)
+        << "Inconsistent scope index for name " << identifiers().Get(str_id);
     if (it->second.size() == 1) {
       // Erase names that no longer resolve.
       name_lookup_.erase(it);
@@ -168,6 +260,20 @@ auto Context::PopScope() -> void {
       it->second.pop_back();
     }
   }
+
+  if (scope.scope_id.is_valid()) {
+    CARBON_CHECK(non_lexical_scope_stack_.back().first == scope.index);
+    non_lexical_scope_stack_.pop_back();
+  }
+}
+
+auto Context::PopToScope(ScopeIndex index) -> void {
+  while (current_scope_index() > index) {
+    PopScope();
+  }
+  CARBON_CHECK(current_scope_index() == index)
+      << "Scope index " << index << " does not enclose the current scope "
+      << current_scope_index();
 }
 
 auto Context::FollowNameReferences(SemIR::InstId inst_id) -> SemIR::InstId {

toolchain/check/context.h

@@ -55,10 +55,20 @@ class Context {
   auto AddNameToLookup(Parse::Node name_node, IdentifierId name_id,
                        SemIR::InstId target_id) -> void;
 
-  // Performs name lookup in a specified scope, returning the referenced
-  // instruction. If scope_id is invalid, uses the current contextual scope.
-  auto LookupName(Parse::Node parse_node, IdentifierId name_id,
-                  SemIR::NameScopeId scope_id, bool print_diagnostics)
+  // Performs name lookup in a specified scope for a name appearing in a
+  // declaration, returning the referenced instruction. If scope_id is invalid,
+  // uses the current contextual scope.
+  auto LookupNameInDeclaration(Parse::Node parse_node, IdentifierId name_id,
+                               SemIR::NameScopeId scope_id) -> SemIR::InstId;
+
+  // Performs an unqualified name lookup, returning the referenced instruction.
+  auto LookupUnqualifiedName(Parse::Node parse_node, IdentifierId name_id)
+      -> SemIR::InstId;
+
+  // Performs a qualified name lookup in a specified scope and in scopes that
+  // it extends, returning the referenced instruction.
+  auto LookupQualifiedName(Parse::Node parse_node, IdentifierId name_id,
+                           SemIR::NameScopeId scope_id, bool required = true)
       -> SemIR::InstId;
 
   // Prints a diagnostic for a duplicate name.
@@ -81,6 +91,14 @@ class Context {
   // Pops the top scope from scope_stack_, cleaning up names from name_lookup_.
   auto PopScope() -> void;
 
+  // Pops scopes until we return to the specified scope index.
+  auto PopToScope(ScopeIndex index) -> void;
+
+  // Returns the scope index associated with the current scope.
+  auto current_scope_index() const -> ScopeIndex {
+    return current_scope().index;
+  }
+
   // Returns the name scope associated with the current lexical scope, if any.
   auto current_scope_id() const -> SemIR::NameScopeId {
     return current_scope().scope_id;
@@ -296,6 +314,9 @@ class Context {
 
   // An entry in scope_stack_.
   struct ScopeStackEntry {
+    // The sequential index of this scope entry within the file.
+    ScopeIndex index;
+
     // The instruction associated with this entry, if any. This can be one of:
     //
     // - A `ClassDeclaration`, for a class definition scope.
@@ -314,6 +335,14 @@ class Context {
     // TODO: This likely needs to track things which need to be destructed.
   };
 
+  // A lookup result in the lexical lookup table `name_lookup_`.
+  struct LexicalLookupResult {
+    // The node that was added to lookup.
+    SemIR::InstId node_id;
+    // The scope in which the node was added.
+    ScopeIndex scope_index;
+  };
+
   // Forms a canonical type ID for a type. This function is given two
   // callbacks:
   //
@@ -383,16 +412,27 @@ class Context {
   // A stack for scope context.
   llvm::SmallVector<ScopeStackEntry> scope_stack_;
 
+  // Information about non-lexical scopes. This is a subset of the entries and
+  // the information in scope_stack_.
+  llvm::SmallVector<std::pair<ScopeIndex, SemIR::NameScopeId>>
+      non_lexical_scope_stack_;
+
+  // The index of the next scope that will be pushed onto scope_stack_.
+  ScopeIndex next_scope_index_ = ScopeIndex(0);
+
   // The stack used for qualified declaration name construction.
   DeclarationNameStack 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,
   // regardless of how many scopes exist between the name declaration and
-  // reference.
+  // reference. The corresponding scope for each lookup result is tracked, so
+  // that lexical lookup results can be interleaved with lookup results from
+  // non-lexical scopes such as classes.
   //
   // Names which no longer have lookup results are erased.
-  llvm::DenseMap<IdentifierId, llvm::SmallVector<SemIR::InstId>> name_lookup_;
+  llvm::DenseMap<IdentifierId, llvm::SmallVector<LexicalLookupResult>>
+      name_lookup_;
 
   // Cache of the mapping from instructions to types, to avoid recomputing the
   // folding set ID.

toolchain/check/declaration_name_stack.cpp

@@ -9,7 +9,8 @@
 namespace Carbon::Check {
 
 auto DeclarationNameStack::MakeEmptyNameContext() -> NameContext {
-  return NameContext{.target_scope_id = context_->current_scope_id()};
+  return NameContext{.enclosing_scope = context_->current_scope_index(),
+                     .target_scope_id = context_->current_scope_id()};
 }
 
 auto DeclarationNameStack::MakeUnqualifiedName(Parse::Node parse_node,
@@ -20,11 +21,14 @@ auto DeclarationNameStack::MakeUnqualifiedName(Parse::Node parse_node,
   return context;
 }
 
-auto DeclarationNameStack::Push() -> void {
+auto DeclarationNameStack::PushScopeAndStartName() -> void {
   declaration_name_stack_.push_back(MakeEmptyNameContext());
 }
 
-auto DeclarationNameStack::Pop() -> NameContext {
+auto DeclarationNameStack::FinishName() -> NameContext {
+  CARBON_CHECK(declaration_name_stack_.back().state !=
+               NameContext::State::Finished)
+      << "Finished name twice";
   if (context_->parse_tree().node_kind(
           context_->node_stack().PeekParseNode()) ==
       Parse::NodeKind::QualifiedDeclaration) {
@@ -39,7 +43,17 @@ auto DeclarationNameStack::Pop() -> NameContext {
     ApplyNameQualifier(parse_node, name_id);
   }
 
-  return declaration_name_stack_.pop_back_val();
+  NameContext result = declaration_name_stack_.back();
+  declaration_name_stack_.back().state = NameContext::State::Finished;
+  return result;
+}
+
+auto DeclarationNameStack::PopScope() -> void {
+  CARBON_CHECK(declaration_name_stack_.back().state ==
+               NameContext::State::Finished)
+      << "Missing call to FinishName before PopScope";
+  context_->PopToScope(declaration_name_stack_.back().enclosing_scope);
+  declaration_name_stack_.pop_back();
 }
 
 auto DeclarationNameStack::LookupOrAddName(NameContext name_context,
@@ -74,6 +88,9 @@ auto DeclarationNameStack::LookupOrAddName(NameContext name_context,
             << name_context.target_scope_id;
       }
       return SemIR::InstId::Invalid;
+
+    case NameContext::State::Finished:
+      CARBON_FATAL() << "Finished state should only be used internally";
   }
 }
 
@@ -95,16 +112,10 @@ auto DeclarationNameStack::ApplyNameQualifierTo(NameContext& name_context,
                                                 IdentifierId name_id) -> void {
   if (CanResolveQualifier(name_context, parse_node)) {
     // For identifier nodes, we need to perform a lookup on the identifier.
-    // This means the input instruction name_id is actually a string ID.
-    //
-    // TODO: This doesn't perform the right kind of lookup. We will find names
-    // from enclosing lexical scopes here, in the case where `target_scope_id`
-    // is invalid.
-    auto resolved_inst_id = context_->LookupName(
-        name_context.parse_node, name_id, name_context.target_scope_id,
-        /*print_diagnostics=*/false);
-    if (resolved_inst_id == SemIR::InstId::BuiltinError) {
-      // Invalid indicates an unresolved instruction. Store it and return.
+    auto resolved_inst_id = context_->LookupNameInDeclaration(
+        name_context.parse_node, name_id, name_context.target_scope_id);
+    if (!resolved_inst_id.is_valid()) {
+      // Invalid indicates an unresolved name. Store it and return.
       name_context.state = NameContext::State::Unresolved;
       name_context.unresolved_name_id = name_id;
       return;
@@ -130,16 +141,19 @@ auto DeclarationNameStack::UpdateScopeIfNeeded(NameContext& name_context)
       if (class_info.is_defined()) {
         name_context.state = NameContext::State::Resolved;
         name_context.target_scope_id = class_info.scope_id;
+        context_->PushScope(name_context.resolved_inst_id, class_info.scope_id);
       } else {
         name_context.state = NameContext::State::ResolvedNonScope;
       }
       break;
     }
-    case SemIR::Namespace::Kind:
+    case SemIR::Namespace::Kind: {
+      auto scope_id = resolved_inst.As<SemIR::Namespace>().name_scope_id;
       name_context.state = NameContext::State::Resolved;
-      name_context.target_scope_id =
-          resolved_inst.As<SemIR::Namespace>().name_scope_id;
+      name_context.target_scope_id = scope_id;
+      context_->PushScope(name_context.resolved_inst_id, scope_id);
       break;
+    }
     default:
       name_context.state = NameContext::State::ResolvedNonScope;
       break;
@@ -198,6 +212,9 @@ auto DeclarationNameStack::CanResolveQualifier(NameContext& name_context,
       name_context.parse_node = parse_node;
       return true;
     }
+
+    case NameContext::State::Finished:
+      CARBON_FATAL() << "Added a qualifier after calling FinishName";
   }
 }
 

toolchain/check/declaration_name_stack.h

@@ -11,6 +11,20 @@
 
 namespace Carbon::Check {
 
+// An index for a pushed scope. This may correspond to a permanent scope with a
+// corresponding `NameScope`, in which case a different index will be assigned
+// each time the scope is entered. Alternatively, it may be a temporary scope
+// such as is created for a block, and will only be entered once.
+//
+// `ScopeIndex` values are comparable. Lower `ScopeIndex` values correspond to
+// scopes entered earlier in the file.
+//
+// TODO: Move this struct and the name lookup code in context.h to a separate
+// file.
+struct ScopeIndex : public ComparableIndexBase, public Printable<ScopeIndex> {
+  using ComparableIndexBase::ComparableIndexBase;
+};
+
 class Context;
 
 // Provides support and stacking for qualified declaration name handling.
@@ -28,6 +42,19 @@ class Context;
 // occur for both out-of-line definitions and new declarations, depending on
 // context.
 //
+// For each name component that is processed and denotes a scope, the
+// corresponding scope is also entered. This is important for unqualified name
+// lookup both in the definition of the entity being declared, and for names
+// appearing later in the declaration name itself. For example, in:
+//
+// ```
+// fn ClassA.ClassB(T:! U).Fn() { var x: V; }
+// ```
+//
+// the lookup for `U` looks in `ClassA`, and the lookup for `V` looks in
+// `ClassA.ClassB` then in its enclosing scope `ClassA`. Scopes entered as part
+// of processing the name are exited when the name is popped from the stack.
+//
 // Example state transitions:
 //
 // ```
@@ -67,11 +94,20 @@ class DeclarationNameStack {
       // An identifier didn't resolve.
       Unresolved,
 
+      // The name has already been finished. This is not set in the name
+      // returned by `FinishName`, but is used internally to track that
+      // `FinishName` has already been called.
+      Finished,
+
       // An error has occurred, such as an additional qualifier past an
       // unresolved name. No new diagnostics should be emitted.
       Error,
     };
 
+    // The current scope when this name began. This is the scope that we will
+    // return to at the end of the declaration.
+    ScopeIndex enclosing_scope;
+
     State state = State::Empty;
 
     // The scope which qualified names are added to. For unqualified names in
@@ -95,13 +131,24 @@ class DeclarationNameStack {
   explicit DeclarationNameStack(Context* context) : context_(context) {}
 
   // Pushes processing of a new declaration name, which will be used
-  // contextually.
-  auto Push() -> void;
+  // contextually, and prepares to enter scopes for that name. To pop this
+  // state, `FinishName` and `PopScope` must be called, in that order.
+  auto PushScopeAndStartName() -> 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
-  // name stack, which will be applied to the declaration name if appropriate.
-  auto Pop() -> NameContext;
+  // 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;
+
+  // Pops the declaration name from the declaration name stack, and pops all
+  // scopes that were entered as part of handling the declaration name. These
+  // are the scopes corresponding to name qualifiers in the name, for example
+  // the `A.B` in `fn A.B.F()`.
+  //
+  // This should be called at the end of the declaration.
+  auto PopScope() -> void;
 
   // Creates and returns a name context corresponding to declaring an
   // unqualified name in the current context. This is suitable for adding to
@@ -111,7 +158,8 @@ class DeclarationNameStack {
       -> NameContext;
 
   // Applies a Name from the name stack to the top of the declaration name
-  // stack.
+  // 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(Parse::Node parse_node, IdentifierId name_id) -> void;
 
   // Adds a name to name lookup. Prints a diagnostic for name conflicts.

toolchain/check/handle_class.cpp

@@ -14,13 +14,13 @@ auto HandleClassIntroducer(Context& context, Parse::Node parse_node) -> bool {
   // Push the bracketing node.
   context.node_stack().Push(parse_node);
   // A name should always follow.
-  context.declaration_name_stack().Push();
+  context.declaration_name_stack().PushScopeAndStartName();
   return true;
 }
 
 static auto BuildClassDeclaration(Context& context)
     -> std::tuple<SemIR::ClassId, SemIR::InstId> {
-  auto name_context = context.declaration_name_stack().Pop();
+  auto name_context = context.declaration_name_stack().FinishName();
   auto class_keyword =
       context.node_stack()
           .PopForSoloParseNode<Parse::NodeKind::ClassIntroducer>();
@@ -76,6 +76,7 @@ static auto BuildClassDeclaration(Context& context)
 auto HandleClassDeclaration(Context& context, Parse::Node /*parse_node*/)
     -> bool {
   BuildClassDeclaration(context);
+  context.declaration_name_stack().PopScope();
   return true;
 }
 
@@ -137,6 +138,7 @@ auto HandleClassDefinition(Context& context, Parse::Node parse_node) -> bool {
       context.node_stack().Pop<Parse::NodeKind::ClassDefinitionStart>();
   context.inst_block_stack().Pop();
   context.PopScope();
+  context.declaration_name_stack().PopScope();
 
   // The class type is now fully defined.
   auto& class_info = context.classes().Get(class_id);

toolchain/check/handle_function.cpp

@@ -55,7 +55,7 @@ static auto BuildFunctionDeclaration(Context& context, bool is_definition)
       context.node_stack()
           .PopIf<Parse::NodeKind::ImplicitParameterList>()
           .value_or(SemIR::InstBlockId::Empty);
-  auto name_context = context.declaration_name_stack().Pop();
+  auto name_context = context.declaration_name_stack().FinishName();
   auto fn_node =
       context.node_stack()
           .PopForSoloParseNode<Parse::NodeKind::FunctionIntroducer>();
@@ -132,6 +132,7 @@ static auto BuildFunctionDeclaration(Context& context, bool is_definition)
 auto HandleFunctionDeclaration(Context& context, Parse::Node /*parse_node*/)
     -> bool {
   BuildFunctionDeclaration(context, /*is_definition=*/false);
+  context.declaration_name_stack().PopScope();
   return true;
 }
 
@@ -156,6 +157,7 @@ auto HandleFunctionDefinition(Context& context, Parse::Node parse_node)
   context.PopScope();
   context.inst_block_stack().Pop();
   context.return_scope_stack().pop_back();
+  context.declaration_name_stack().PopScope();
   return true;
 }
 
@@ -233,7 +235,7 @@ auto HandleFunctionIntroducer(Context& context, Parse::Node parse_node)
   // Push the bracketing node.
   context.node_stack().Push(parse_node);
   // A name should always follow.
-  context.declaration_name_stack().Push();
+  context.declaration_name_stack().PushScopeAndStartName();
   return true;
 }
 

toolchain/check/handle_name.cpp

@@ -62,10 +62,10 @@ auto HandleMemberAccessExpression(Context& context, Parse::Node parse_node)
   // If the base is a name scope, such as a class or namespace, perform lookup
   // into that scope.
   if (auto name_scope_id = GetAsNameScope(context, base_id)) {
-    auto inst_id = name_scope_id->is_valid()
-                       ? context.LookupName(parse_node, name_id, *name_scope_id,
-                                            /*print_diagnostics=*/true)
-                       : SemIR::InstId::BuiltinError;
+    auto inst_id =
+        name_scope_id->is_valid()
+            ? context.LookupQualifiedName(parse_node, name_id, *name_scope_id)
+            : SemIR::InstId::BuiltinError;
     inst_id = GetExpressionValueForLookupResult(context, inst_id);
     auto inst = context.insts().Get(inst_id);
     // TODO: Track that this instruction was named within `base_id`.
@@ -103,8 +103,8 @@ auto HandleMemberAccessExpression(Context& context, Parse::Node parse_node)
       auto class_scope_id = context.classes()
                                 .Get(base_type.As<SemIR::ClassType>().class_id)
                                 .scope_id;
-      auto member_id = context.LookupName(parse_node, name_id, class_scope_id,
-                                          /*print_diagnostics=*/true);
+      auto member_id =
+          context.LookupQualifiedName(parse_node, name_id, class_scope_id);
       member_id = GetExpressionValueForLookupResult(context, member_id);
 
       // Perform instance binding if we found an instance member.
@@ -234,10 +234,7 @@ auto HandleName(Context& context, Parse::Node parse_node) -> bool {
 auto HandleNameExpression(Context& context, Parse::Node parse_node) -> bool {
   auto name_id = context.tokens().GetIdentifier(
       context.parse_tree().node_token(parse_node));
-  auto value_id =
-      context.LookupName(parse_node, name_id, SemIR::NameScopeId::Invalid,
-                         /*print_diagnostics=*/true);
-
+  auto value_id = context.LookupUnqualifiedName(parse_node, name_id);
   value_id = GetExpressionValueForLookupResult(context, value_id);
   auto value = context.insts().Get(value_id);
   context.AddInstAndPush(
@@ -285,9 +282,7 @@ auto HandleSelfTypeNameExpression(Context& context, Parse::Node parse_node)
   // HandleClassDefinitionStart.
   auto name_id = context.identifiers().Add(
       Lex::TokenKind::SelfTypeIdentifier.fixed_spelling());
-  auto value_id =
-      context.LookupName(parse_node, name_id, SemIR::NameScopeId::Invalid,
-                         /*print_diagnostics=*/true);
+  auto value_id = context.LookupUnqualifiedName(parse_node, name_id);
   auto value = context.insts().Get(value_id);
   context.AddInstAndPush(
       parse_node,
@@ -306,9 +301,7 @@ auto HandleSelfValueNameExpression(Context& context, Parse::Node parse_node)
   // should not. See #2984 and the corresponding code in
   // HandleFunctionDefinitionStart.
   auto name_id = context.identifiers().Add(SemIR::SelfParameter::Name);
-  auto value_id =
-      context.LookupName(parse_node, name_id, SemIR::NameScopeId::Invalid,
-                         /*print_diagnostics=*/true);
+  auto value_id = context.LookupUnqualifiedName(parse_node, name_id);
   auto value = context.insts().Get(value_id);
   context.AddInstAndPush(
       parse_node,

toolchain/check/handle_namespace.cpp

@@ -9,16 +9,17 @@ namespace Carbon::Check {
 
 auto HandleNamespaceStart(Context& context, Parse::Node /*parse_node*/)
     -> bool {
-  context.declaration_name_stack().Push();
+  context.declaration_name_stack().PushScopeAndStartName();
   return true;
 }
 
 auto HandleNamespace(Context& context, Parse::Node parse_node) -> bool {
-  auto name_context = context.declaration_name_stack().Pop();
+  auto name_context = context.declaration_name_stack().FinishName();
   auto namespace_id = context.AddInst(SemIR::Namespace{
       parse_node, context.GetBuiltinType(SemIR::BuiltinKind::NamespaceType),
       context.name_scopes().Add()});
   context.declaration_name_stack().AddNameToLookup(name_context, namespace_id);
+  context.declaration_name_stack().PopScope();
   return true;
 }
 

toolchain/check/testdata/class/fail_redeclaration_scope.carbon

@@ -0,0 +1,80 @@
+// 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
+
+class A;
+
+class X {
+  // OK, a different A.
+  class A { class B; }
+  class A.B {}
+}
+
+class A { class B; }
+
+class Y {
+  // CHECK:STDERR: fail_redeclaration_scope.carbon:[[@LINE+3]]:9: ERROR: Name `A` not found.
+  // CHECK:STDERR:   class A.B {}
+  // CHECK:STDERR:         ^
+  class A.B {}
+}
+
+// CHECK:STDOUT: constants {
+// CHECK:STDOUT:   %.loc11: type = struct_type {}
+// CHECK:STDOUT: }
+// CHECK:STDOUT:
+// CHECK:STDOUT: file "fail_redeclaration_scope.carbon" {
+// CHECK:STDOUT:   class_declaration @A.1, ()
+// CHECK:STDOUT:   %A: type = class_type @A.1
+// CHECK:STDOUT:   class_declaration @X, ()
+// CHECK:STDOUT:   %X: type = class_type @X
+// CHECK:STDOUT:   class_declaration @A.1, ()
+// CHECK:STDOUT:   class_declaration @Y, ()
+// CHECK:STDOUT:   %Y: type = class_type @Y
+// CHECK:STDOUT: }
+// CHECK:STDOUT:
+// CHECK:STDOUT: class @A.1 {
+// CHECK:STDOUT:   class_declaration @B.2, ()
+// CHECK:STDOUT:   %B: type = class_type @B.2
+// CHECK:STDOUT:
+// CHECK:STDOUT: !members:
+// CHECK:STDOUT:   .B = <unexpected instref 20>
+// CHECK:STDOUT: }
+// CHECK:STDOUT:
+// CHECK:STDOUT: class @X {
+// CHECK:STDOUT:   class_declaration @A.2, ()
+// CHECK:STDOUT:   %A: type = class_type @A.2
+// CHECK:STDOUT:   class_declaration @B.1, ()
+// CHECK:STDOUT:
+// CHECK:STDOUT: !members:
+// CHECK:STDOUT:   .A = <unexpected instref 13>
+// CHECK:STDOUT: }
+// CHECK:STDOUT:
+// CHECK:STDOUT: class @A.2 {
+// CHECK:STDOUT:   class_declaration @B.1, ()
+// CHECK:STDOUT:   %B: type = class_type @B.1
+// CHECK:STDOUT:
+// CHECK:STDOUT: !members:
+// CHECK:STDOUT:   .B = <unexpected instref 15>
+// CHECK:STDOUT: }
+// CHECK:STDOUT:
+// CHECK:STDOUT: class @B.1 {
+// CHECK:STDOUT:
+// CHECK:STDOUT: !members:
+// CHECK:STDOUT: }
+// CHECK:STDOUT:
+// CHECK:STDOUT: class @B.2;
+// CHECK:STDOUT:
+// CHECK:STDOUT: class @Y {
+// CHECK:STDOUT:   class_declaration @.1, ()
+// CHECK:STDOUT:   %.loc21: type = class_type @.1
+// CHECK:STDOUT:
+// CHECK:STDOUT: !members:
+// CHECK:STDOUT: }
+// CHECK:STDOUT:
+// CHECK:STDOUT: class @.1 {
+// CHECK:STDOUT:
+// CHECK:STDOUT: !members:
+// CHECK:STDOUT: }

toolchain/check/testdata/class/fail_scope.carbon

@@ -8,32 +8,30 @@ class Class {
   fn F() -> i32 {
     return 1;
   }
+}
 
-  // TODO: This `F()` should find `Class.F`.
-  fn G() -> i32 {
-    // CHECK:STDERR: fail_scope.carbon:[[@LINE+3]]:12: ERROR: Name `F` not found.
-    // CHECK:STDERR:     return F();
-    // CHECK:STDERR:            ^
-    return F();
-  }
+fn G() -> i32 {
+  // CHECK:STDERR: fail_scope.carbon:[[@LINE+3]]:10: ERROR: Name `F` not found.
+  // CHECK:STDERR:   return F();
+  // CHECK:STDERR:          ^
+  return F();
 }
 
 // CHECK:STDOUT: constants {
-// CHECK:STDOUT:   %.loc19: type = struct_type {}
+// CHECK:STDOUT:   %.loc11: type = struct_type {}
 // CHECK:STDOUT: }
 // CHECK:STDOUT:
 // CHECK:STDOUT: file "fail_scope.carbon" {
 // CHECK:STDOUT:   class_declaration @Class, ()
 // CHECK:STDOUT:   %Class: type = class_type @Class
+// CHECK:STDOUT:   %G: <function> = fn_decl @G
 // CHECK:STDOUT: }
 // CHECK:STDOUT:
 // CHECK:STDOUT: class @Class {
 // CHECK:STDOUT:   %F: <function> = fn_decl @F
-// CHECK:STDOUT:   %G: <function> = fn_decl @G
 // CHECK:STDOUT:
 // CHECK:STDOUT: !members:
 // CHECK:STDOUT:   .F = %F
-// CHECK:STDOUT:   .G = %G
 // CHECK:STDOUT: }
 // CHECK:STDOUT:
 // CHECK:STDOUT: fn @F() -> i32 {

toolchain/check/testdata/class/fail_unbound_field.carbon

@@ -7,8 +7,7 @@
 class Class {
   var field: i32;
   fn F() -> i32 {
-    // TODO: Unqualified lookup should find this name.
-    // CHECK:STDERR: fail_unbound_field.carbon:[[@LINE+3]]:12: ERROR: Name `field` not found.
+    // CHECK:STDERR: fail_unbound_field.carbon:[[@LINE+3]]:12: ERROR: Expression cannot be used as a value.
     // CHECK:STDERR:     return field;
     // CHECK:STDERR:            ^
     return field;
@@ -23,7 +22,7 @@ fn G() -> i32 {
 }
 
 // CHECK:STDOUT: constants {
-// CHECK:STDOUT:   %.loc16: type = struct_type {.field: i32}
+// CHECK:STDOUT:   %.loc15: type = struct_type {.field: i32}
 // CHECK:STDOUT: }
 // CHECK:STDOUT:
 // CHECK:STDOUT: file "fail_unbound_field.carbon" {
@@ -45,7 +44,7 @@ fn G() -> i32 {
 // CHECK:STDOUT:
 // CHECK:STDOUT: fn @F() -> i32 {
 // CHECK:STDOUT: !entry:
-// CHECK:STDOUT:   %field.ref: <error> = name_reference "field", <error>
+// CHECK:STDOUT:   %field.ref: <unbound field of class Class> = name_reference "field", @Class.%field
 // CHECK:STDOUT:   return <error>
 // CHECK:STDOUT: }
 // CHECK:STDOUT:

toolchain/check/testdata/class/nested.carbon

@@ -0,0 +1,144 @@
+// 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
+
+class Outer {
+  class Inner {
+    var pi: Self*;
+    var po: Outer*;
+    var qi: Inner*;
+  }
+
+  var po: Self*;
+  var qo: Outer*;
+  var pi: Inner*;
+}
+
+fn F(a: Outer*) {
+  // TODO: Simplify this once `Outer.Inner` works.
+  // let b: Outer.Inner* = (*a).pi;
+
+  (*a).po = a;
+  (*a).qo = a;
+  (*a).pi = (*a).pi;
+  (*(*a).pi).po = a;
+  (*(*a).pi).pi = (*a).pi;
+  (*(*a).pi).qi = (*a).pi;
+}
+
+// CHECK:STDOUT: constants {
+// CHECK:STDOUT:   %.loc12_3.1: type = struct_type {.pi: Inner*, .po: Outer*, .qi: Inner*}
+// CHECK:STDOUT:   %.loc17_1.1: type = struct_type {.po: Outer*, .qo: Outer*, .pi: Inner*}
+// CHECK:STDOUT:   %.loc17_1.2: type = ptr_type {.po: Outer*, .qo: Outer*, .pi: Inner*}
+// CHECK:STDOUT:   %.loc12_3.2: type = ptr_type {.pi: Inner*, .po: Outer*, .qi: Inner*}
+// CHECK:STDOUT: }
+// CHECK:STDOUT:
+// CHECK:STDOUT: file "nested.carbon" {
+// CHECK:STDOUT:   class_declaration @Outer, ()
+// CHECK:STDOUT:   %Outer: type = class_type @Outer
+// CHECK:STDOUT:   %F: <function> = fn_decl @F
+// CHECK:STDOUT: }
+// CHECK:STDOUT:
+// CHECK:STDOUT: class @Outer {
+// CHECK:STDOUT:   class_declaration @Inner, ()
+// CHECK:STDOUT:   %Inner: type = class_type @Inner
+// CHECK:STDOUT:   %Self.ref: type = name_reference "Self", file.%Outer
+// CHECK:STDOUT:   %.loc14_15: type = ptr_type Outer
+// CHECK:STDOUT:   %.loc14_9.1: type = unbound_field_type Outer, Outer*
+// CHECK:STDOUT:   %.loc14_9.2: <unbound field of class Outer> = field "po", member0
+// CHECK:STDOUT:   %po: <unbound field of class Outer> = bind_name "po", %.loc14_9.2
+// CHECK:STDOUT:   %Outer.ref: type = name_reference "Outer", file.%Outer
+// CHECK:STDOUT:   %.loc15_16: type = ptr_type Outer
+// CHECK:STDOUT:   %.loc15_9.1: type = unbound_field_type Outer, Outer*
+// CHECK:STDOUT:   %.loc15_9.2: <unbound field of class Outer> = field "qo", member1
+// CHECK:STDOUT:   %qo: <unbound field of class Outer> = bind_name "qo", %.loc15_9.2
+// CHECK:STDOUT:   %Inner.ref: type = name_reference "Inner", %Inner
+// CHECK:STDOUT:   %.loc16_16: type = ptr_type Inner
+// CHECK:STDOUT:   %.loc16_9.1: type = unbound_field_type Outer, Inner*
+// CHECK:STDOUT:   %.loc16_9.2: <unbound field of class Outer> = field "pi", member2
+// CHECK:STDOUT:   %pi: <unbound field of class Outer> = bind_name "pi", %.loc16_9.2
+// CHECK:STDOUT:
+// CHECK:STDOUT: !members:
+// CHECK:STDOUT:   .Inner = <unexpected instref 11>
+// CHECK:STDOUT:   .po = %po
+// CHECK:STDOUT:   .qo = %qo
+// CHECK:STDOUT:   .pi = %pi
+// CHECK:STDOUT: }
+// CHECK:STDOUT:
+// CHECK:STDOUT: class @Inner {
+// CHECK:STDOUT:   %Self.ref: type = name_reference "Self", @Outer.%Inner
+// CHECK:STDOUT:   %.loc9_17: type = ptr_type Inner
+// CHECK:STDOUT:   %.loc9_11.1: type = unbound_field_type Inner, Inner*
+// CHECK:STDOUT:   %.loc9_11.2: <unbound field of class Inner> = field "pi", member0
+// CHECK:STDOUT:   %pi: <unbound field of class Inner> = bind_name "pi", %.loc9_11.2
+// CHECK:STDOUT:   %Outer.ref: type = name_reference "Outer", file.%Outer
+// CHECK:STDOUT:   %.loc10_18: type = ptr_type Outer
+// CHECK:STDOUT:   %.loc10_11.1: type = unbound_field_type Inner, Outer*
+// CHECK:STDOUT:   %.loc10_11.2: <unbound field of class Inner> = field "po", member1
+// CHECK:STDOUT:   %po: <unbound field of class Inner> = bind_name "po", %.loc10_11.2
+// CHECK:STDOUT:   %Inner.ref: type = name_reference "Inner", @Outer.%Inner
+// CHECK:STDOUT:   %.loc11_18: type = ptr_type Inner
+// CHECK:STDOUT:   %.loc11_11.1: type = unbound_field_type Inner, Inner*
+// CHECK:STDOUT:   %.loc11_11.2: <unbound field of class Inner> = field "qi", member2
+// CHECK:STDOUT:   %qi: <unbound field of class Inner> = bind_name "qi", %.loc11_11.2
+// CHECK:STDOUT:
+// CHECK:STDOUT: !members:
+// CHECK:STDOUT:   .pi = %pi
+// CHECK:STDOUT:   .po = %po
+// CHECK:STDOUT:   .qi = %qi
+// CHECK:STDOUT: }
+// CHECK:STDOUT:
+// CHECK:STDOUT: fn @F(%a: Outer*) {
+// CHECK:STDOUT: !entry:
+// CHECK:STDOUT:   %a.ref.loc23_5: Outer* = name_reference "a", %a
+// CHECK:STDOUT:   %.loc23_4: ref Outer = dereference %a.ref.loc23_5
+// CHECK:STDOUT:   %.loc23_7: ref Outer* = class_field_access %.loc23_4, member0
+// CHECK:STDOUT:   %a.ref.loc23_13: Outer* = name_reference "a", %a
+// CHECK:STDOUT:   assign %.loc23_7, %a.ref.loc23_13
+// CHECK:STDOUT:   %a.ref.loc24_5: Outer* = name_reference "a", %a
+// CHECK:STDOUT:   %.loc24_4: ref Outer = dereference %a.ref.loc24_5
+// CHECK:STDOUT:   %.loc24_7: ref Outer* = class_field_access %.loc24_4, member1
+// CHECK:STDOUT:   %a.ref.loc24_13: Outer* = name_reference "a", %a
+// CHECK:STDOUT:   assign %.loc24_7, %a.ref.loc24_13
+// CHECK:STDOUT:   %a.ref.loc25_5: Outer* = name_reference "a", %a
+// CHECK:STDOUT:   %.loc25_4: ref Outer = dereference %a.ref.loc25_5
+// CHECK:STDOUT:   %.loc25_7: ref Inner* = class_field_access %.loc25_4, member2
+// CHECK:STDOUT:   %a.ref.loc25_15: Outer* = name_reference "a", %a
+// CHECK:STDOUT:   %.loc25_14: ref Outer = dereference %a.ref.loc25_15
+// CHECK:STDOUT:   %.loc25_17.1: ref Inner* = class_field_access %.loc25_14, member2
+// CHECK:STDOUT:   %.loc25_17.2: Inner* = bind_value %.loc25_17.1
+// CHECK:STDOUT:   assign %.loc25_7, %.loc25_17.2
+// CHECK:STDOUT:   %a.ref.loc26_7: Outer* = name_reference "a", %a
+// CHECK:STDOUT:   %.loc26_6: ref Outer = dereference %a.ref.loc26_7
+// CHECK:STDOUT:   %.loc26_9.1: ref Inner* = class_field_access %.loc26_6, member2
+// CHECK:STDOUT:   %.loc26_9.2: Inner* = bind_value %.loc26_9.1
+// CHECK:STDOUT:   %.loc26_4: ref Inner = dereference %.loc26_9.2
+// CHECK:STDOUT:   %.loc26_13: ref Outer* = class_field_access %.loc26_4, member1
+// CHECK:STDOUT:   %a.ref.loc26_19: Outer* = name_reference "a", %a
+// CHECK:STDOUT:   assign %.loc26_13, %a.ref.loc26_19
+// CHECK:STDOUT:   %a.ref.loc27_7: Outer* = name_reference "a", %a
+// CHECK:STDOUT:   %.loc27_6: ref Outer = dereference %a.ref.loc27_7
+// CHECK:STDOUT:   %.loc27_9.1: ref Inner* = class_field_access %.loc27_6, member2
+// CHECK:STDOUT:   %.loc27_9.2: Inner* = bind_value %.loc27_9.1
+// CHECK:STDOUT:   %.loc27_4: ref Inner = dereference %.loc27_9.2
+// CHECK:STDOUT:   %.loc27_13: ref Inner* = class_field_access %.loc27_4, member0
+// CHECK:STDOUT:   %a.ref.loc27_21: Outer* = name_reference "a", %a
+// CHECK:STDOUT:   %.loc27_20: ref Outer = dereference %a.ref.loc27_21
+// CHECK:STDOUT:   %.loc27_23.1: ref Inner* = class_field_access %.loc27_20, member2
+// CHECK:STDOUT:   %.loc27_23.2: Inner* = bind_value %.loc27_23.1
+// CHECK:STDOUT:   assign %.loc27_13, %.loc27_23.2
+// CHECK:STDOUT:   %a.ref.loc28_7: Outer* = name_reference "a", %a
+// CHECK:STDOUT:   %.loc28_6: ref Outer = dereference %a.ref.loc28_7
+// CHECK:STDOUT:   %.loc28_9.1: ref Inner* = class_field_access %.loc28_6, member2
+// CHECK:STDOUT:   %.loc28_9.2: Inner* = bind_value %.loc28_9.1
+// CHECK:STDOUT:   %.loc28_4: ref Inner = dereference %.loc28_9.2
+// CHECK:STDOUT:   %.loc28_13: ref Inner* = class_field_access %.loc28_4, member2
+// CHECK:STDOUT:   %a.ref.loc28_21: Outer* = name_reference "a", %a
+// CHECK:STDOUT:   %.loc28_20: ref Outer = dereference %a.ref.loc28_21
+// CHECK:STDOUT:   %.loc28_23.1: ref Inner* = class_field_access %.loc28_20, member2
+// CHECK:STDOUT:   %.loc28_23.2: Inner* = bind_value %.loc28_23.1
+// CHECK:STDOUT:   assign %.loc28_13, %.loc28_23.2
+// CHECK:STDOUT:   return
+// CHECK:STDOUT: }

toolchain/check/testdata/class/reenter_scope.carbon

@@ -0,0 +1,43 @@
+// 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
+
+class Class {
+  fn F() -> i32;
+  fn G() -> i32;
+}
+
+fn Class.F() -> i32 {
+  return G();
+}
+
+// CHECK:STDOUT: constants {
+// CHECK:STDOUT:   %.loc10: type = struct_type {}
+// CHECK:STDOUT: }
+// CHECK:STDOUT:
+// CHECK:STDOUT: file "reenter_scope.carbon" {
+// CHECK:STDOUT:   class_declaration @Class, ()
+// CHECK:STDOUT:   %Class: type = class_type @Class
+// CHECK:STDOUT:   %F: <function> = fn_decl @F
+// CHECK:STDOUT: }
+// CHECK:STDOUT:
+// CHECK:STDOUT: class @Class {
+// CHECK:STDOUT:   %F: <function> = fn_decl @F
+// CHECK:STDOUT:   %G: <function> = fn_decl @G
+// CHECK:STDOUT:
+// CHECK:STDOUT: !members:
+// CHECK:STDOUT:   .F = %F
+// CHECK:STDOUT:   .G = %G
+// CHECK:STDOUT: }
+// CHECK:STDOUT:
+// CHECK:STDOUT: fn @F() -> i32 {
+// CHECK:STDOUT: !entry:
+// CHECK:STDOUT:   %G.ref: <function> = name_reference "G", @Class.%G
+// CHECK:STDOUT:   %.loc13_11: init i32 = call %G.ref()
+// CHECK:STDOUT:   %.loc13_13: i32 = value_of_initializer %.loc13_11
+// CHECK:STDOUT:   return %.loc13_13
+// CHECK:STDOUT: }
+// CHECK:STDOUT:
+// CHECK:STDOUT: fn @G() -> i32;

toolchain/check/testdata/class/scope.carbon

@@ -8,6 +8,10 @@ class Class {
   fn F() -> i32 {
     return 1;
   }
+
+  fn G() -> i32 {
+    return F();
+  }
 }
 
 fn F() -> i32 {
@@ -19,7 +23,7 @@ fn Run() -> i32 {
 }
 
 // CHECK:STDOUT: constants {
-// CHECK:STDOUT:   %.loc11: type = struct_type {}
+// CHECK:STDOUT:   %.loc15: type = struct_type {}
 // CHECK:STDOUT: }
 // CHECK:STDOUT:
 // CHECK:STDOUT: file "scope.carbon" {
@@ -31,9 +35,11 @@ fn Run() -> i32 {
 // CHECK:STDOUT:
 // CHECK:STDOUT: class @Class {
 // CHECK:STDOUT:   %F: <function> = fn_decl @F.1
+// CHECK:STDOUT:   %G: <function> = fn_decl @G
 // CHECK:STDOUT:
 // CHECK:STDOUT: !members:
 // CHECK:STDOUT:   .F = %F
+// CHECK:STDOUT:   .G = %G
 // CHECK:STDOUT: }
 // CHECK:STDOUT:
 // CHECK:STDOUT: fn @F.1() -> i32 {
@@ -42,21 +48,29 @@ fn Run() -> i32 {
 // CHECK:STDOUT:   return %.loc9
 // CHECK:STDOUT: }
 // CHECK:STDOUT:
+// CHECK:STDOUT: fn @G() -> i32 {
+// CHECK:STDOUT: !entry:
+// CHECK:STDOUT:   %F.ref: <function> = name_reference "F", @Class.%F
+// CHECK:STDOUT:   %.loc13_13: init i32 = call %F.ref()
+// CHECK:STDOUT:   %.loc13_15: i32 = value_of_initializer %.loc13_13
+// CHECK:STDOUT:   return %.loc13_15
+// CHECK:STDOUT: }
+// CHECK:STDOUT:
 // CHECK:STDOUT: fn @F.2() -> i32 {
 // CHECK:STDOUT: !entry:
-// CHECK:STDOUT:   %.loc14: i32 = int_literal 2
-// CHECK:STDOUT:   return %.loc14
+// CHECK:STDOUT:   %.loc18: i32 = int_literal 2
+// CHECK:STDOUT:   return %.loc18
 // CHECK:STDOUT: }
 // CHECK:STDOUT:
 // CHECK:STDOUT: fn @Run() -> i32 {
 // CHECK:STDOUT: !entry:
-// CHECK:STDOUT:   %F.ref.loc18_10: <function> = name_reference "F", file.%F
-// CHECK:STDOUT:   %.loc18_11: init i32 = call %F.ref.loc18_10()
+// CHECK:STDOUT:   %F.ref.loc22_10: <function> = name_reference "F", file.%F
+// CHECK:STDOUT:   %.loc22_11: init i32 = call %F.ref.loc22_10()
 // CHECK:STDOUT:   %Class.ref: type = name_reference "Class", file.%Class
-// CHECK:STDOUT:   %F.ref.loc18_21: <function> = name_reference "F", @Class.%F
-// CHECK:STDOUT:   %.loc18_23.1: init i32 = call %F.ref.loc18_21()
-// CHECK:STDOUT:   %.loc18_14.1: i32 = value_of_initializer %.loc18_11
-// CHECK:STDOUT:   %.loc18_23.2: i32 = value_of_initializer %.loc18_23.1
-// CHECK:STDOUT:   %.loc18_14.2: i32 = add %.loc18_14.1, %.loc18_23.2
-// CHECK:STDOUT:   return %.loc18_14.2
+// CHECK:STDOUT:   %F.ref.loc22_21: <function> = name_reference "F", @Class.%F
+// CHECK:STDOUT:   %.loc22_23.1: init i32 = call %F.ref.loc22_21()
+// CHECK:STDOUT:   %.loc22_14.1: i32 = value_of_initializer %.loc22_11
+// CHECK:STDOUT:   %.loc22_23.2: i32 = value_of_initializer %.loc22_23.1
+// CHECK:STDOUT:   %.loc22_14.2: i32 = add %.loc22_14.1, %.loc22_23.2
+// CHECK:STDOUT:   return %.loc22_14.2
 // CHECK:STDOUT: }

toolchain/check/testdata/namespace/shadow.carbon

@@ -0,0 +1,61 @@
+// 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
+
+fn A();
+
+namespace N;
+fn N.A();
+
+namespace N.M;
+
+fn N.M.B() -> i32 {
+  // This is N.A, not package.A.
+  A();
+  if (true) {
+    var A: i32 = 0;
+    // This is the local A.
+    // TODO: Decide if we should warn or error on the shadowing of N.A here.
+    return A;
+  }
+  return 0;
+}
+
+// CHECK:STDOUT: constants {
+// CHECK:STDOUT:   %.loc16: type = tuple_type ()
+// CHECK:STDOUT: }
+// CHECK:STDOUT:
+// CHECK:STDOUT: file "shadow.carbon" {
+// CHECK:STDOUT:   %A.loc7: <function> = fn_decl @A.1
+// CHECK:STDOUT:   %.loc9: <namespace> = namespace {.A = %A.loc10, .M = %.loc12}
+// CHECK:STDOUT:   %A.loc10: <function> = fn_decl @A.2
+// CHECK:STDOUT:   %.loc12: <namespace> = namespace {.B = %B}
+// CHECK:STDOUT:   %B: <function> = fn_decl @B
+// CHECK:STDOUT: }
+// CHECK:STDOUT:
+// CHECK:STDOUT: fn @A.1();
+// CHECK:STDOUT:
+// CHECK:STDOUT: fn @A.2();
+// CHECK:STDOUT:
+// CHECK:STDOUT: fn @B() -> i32 {
+// CHECK:STDOUT: !entry:
+// CHECK:STDOUT:   %A.ref.loc16: <function> = name_reference "A", file.%A.loc10
+// CHECK:STDOUT:   %.loc16: init () = call %A.ref.loc16()
+// CHECK:STDOUT:   %.loc17: bool = bool_literal true
+// CHECK:STDOUT:   if %.loc17 br !if.then else br !if.else
+// CHECK:STDOUT:
+// CHECK:STDOUT: !if.then:
+// CHECK:STDOUT:   %A.var: ref i32 = var "A"
+// CHECK:STDOUT:   %A: ref i32 = bind_name "A", %A.var
+// CHECK:STDOUT:   %.loc18: i32 = int_literal 0
+// CHECK:STDOUT:   assign %A.var, %.loc18
+// CHECK:STDOUT:   %A.ref.loc21: ref i32 = name_reference "A", %A
+// CHECK:STDOUT:   %.loc21: i32 = bind_value %A.ref.loc21
+// CHECK:STDOUT:   return %.loc21
+// CHECK:STDOUT:
+// CHECK:STDOUT: !if.else:
+// CHECK:STDOUT:   %.loc23: i32 = int_literal 0
+// CHECK:STDOUT:   return %.loc23
+// CHECK:STDOUT: }

toolchain/check/testdata/namespace/unqualified_lookup.carbon

@@ -0,0 +1,75 @@
+// 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
+
+namespace OuterN;
+namespace OuterN.InnerN;
+
+fn A();
+fn OuterN.B();
+fn OuterN.InnerN.C();
+
+fn CallA() {
+  A();
+}
+
+fn OuterN.CallAB() {
+  A();
+  B();
+}
+
+fn OuterN.InnerN.CallABC() {
+  A();
+  B();
+  C();
+}
+
+// CHECK:STDOUT: constants {
+// CHECK:STDOUT:   %.loc15: type = tuple_type ()
+// CHECK:STDOUT: }
+// CHECK:STDOUT:
+// CHECK:STDOUT: file "unqualified_lookup.carbon" {
+// CHECK:STDOUT:   %.loc7: <namespace> = namespace {.InnerN = %.loc8, .B = %B, .CallAB = %CallAB}
+// CHECK:STDOUT:   %.loc8: <namespace> = namespace {.C = %C, .CallABC = %CallABC}
+// CHECK:STDOUT:   %A: <function> = fn_decl @A
+// CHECK:STDOUT:   %B: <function> = fn_decl @B
+// CHECK:STDOUT:   %C: <function> = fn_decl @C
+// CHECK:STDOUT:   %CallA: <function> = fn_decl @CallA
+// CHECK:STDOUT:   %CallAB: <function> = fn_decl @CallAB
+// CHECK:STDOUT:   %CallABC: <function> = fn_decl @CallABC
+// CHECK:STDOUT: }
+// CHECK:STDOUT:
+// CHECK:STDOUT: fn @A();
+// CHECK:STDOUT:
+// CHECK:STDOUT: fn @B();
+// CHECK:STDOUT:
+// CHECK:STDOUT: fn @C();
+// CHECK:STDOUT:
+// CHECK:STDOUT: fn @CallA() {
+// CHECK:STDOUT: !entry:
+// CHECK:STDOUT:   %A.ref: <function> = name_reference "A", file.%A
+// CHECK:STDOUT:   %.loc15: init () = call %A.ref()
+// CHECK:STDOUT:   return
+// CHECK:STDOUT: }
+// CHECK:STDOUT:
+// CHECK:STDOUT: fn @CallAB() {
+// CHECK:STDOUT: !entry:
+// CHECK:STDOUT:   %A.ref: <function> = name_reference "A", file.%A
+// CHECK:STDOUT:   %.loc19: init () = call %A.ref()
+// CHECK:STDOUT:   %B.ref: <function> = name_reference "B", file.%B
+// CHECK:STDOUT:   %.loc20: init () = call %B.ref()
+// CHECK:STDOUT:   return
+// CHECK:STDOUT: }
+// CHECK:STDOUT:
+// CHECK:STDOUT: fn @CallABC() {
+// CHECK:STDOUT: !entry:
+// CHECK:STDOUT:   %A.ref: <function> = name_reference "A", file.%A
+// CHECK:STDOUT:   %.loc24: init () = call %A.ref()
+// CHECK:STDOUT:   %B.ref: <function> = name_reference "B", file.%B
+// CHECK:STDOUT:   %.loc25: init () = call %B.ref()
+// CHECK:STDOUT:   %C.ref: <function> = name_reference "C", file.%C
+// CHECK:STDOUT:   %.loc26: init () = call %C.ref()
+// CHECK:STDOUT:   return
+// CHECK:STDOUT: }

toolchain/check/testdata/var/fail_shadowing.carbon

@@ -1,49 +0,0 @@
-// 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
-
-fn Main() {
-  var x: i32 = 0;
-  if (true) {
-    // TODO: We should reject the use of the shadowed variable `x`, not this
-    // shadowing declaration. Also the diagnostic text is wrong ("same scope").
-    // CHECK:STDERR: fail_shadowing.carbon:[[@LINE+6]]:9: ERROR: Duplicate name being declared in the same scope.
-    // CHECK:STDERR:     var x: i32 = 0;
-    // CHECK:STDERR:         ^
-    // CHECK:STDERR: fail_shadowing.carbon:[[@LINE-7]]:7: Name is previously declared here.
-    // CHECK:STDERR:   var x: i32 = 0;
-    // CHECK:STDERR:       ^
-    var x: i32 = 0;
-
-    x = 1;
-  }
-}
-
-// CHECK:STDOUT: file "fail_shadowing.carbon" {
-// CHECK:STDOUT:   %Main: <function> = fn_decl @Main
-// CHECK:STDOUT: }
-// CHECK:STDOUT:
-// CHECK:STDOUT: fn @Main() {
-// CHECK:STDOUT: !entry:
-// CHECK:STDOUT:   %x.var.loc8: ref i32 = var "x"
-// CHECK:STDOUT:   %x.loc8: ref i32 = bind_name "x", %x.var.loc8
-// CHECK:STDOUT:   %.loc8: i32 = int_literal 0
-// CHECK:STDOUT:   assign %x.var.loc8, %.loc8
-// CHECK:STDOUT:   %.loc9: bool = bool_literal true
-// CHECK:STDOUT:   if %.loc9 br !if.then else br !if.else
-// CHECK:STDOUT:
-// CHECK:STDOUT: !if.then:
-// CHECK:STDOUT:   %x.var.loc18: ref i32 = var "x"
-// CHECK:STDOUT:   %x.loc18: ref i32 = bind_name "x", %x.var.loc18
-// CHECK:STDOUT:   %.loc18: i32 = int_literal 0
-// CHECK:STDOUT:   assign %x.var.loc18, %.loc18
-// CHECK:STDOUT:   %x.ref: ref i32 = name_reference "x", %x.loc8
-// CHECK:STDOUT:   %.loc20: i32 = int_literal 1
-// CHECK:STDOUT:   assign %x.ref, %.loc20
-// CHECK:STDOUT:   br !if.else
-// CHECK:STDOUT:
-// CHECK:STDOUT: !if.else:
-// CHECK:STDOUT:   return
-// CHECK:STDOUT: }

toolchain/check/testdata/var/shadowing.carbon

@@ -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
+//
+// AUTOUPDATE
+
+fn Main() {
+  var x: i32 = 0;
+  if (true) {
+    var x: i32 = 0;
+
+    // TODO: We should reject this use of the shadowed variable `x`.
+    x = 1;
+  }
+}
+
+// CHECK:STDOUT: file "shadowing.carbon" {
+// CHECK:STDOUT:   %Main: <function> = fn_decl @Main
+// CHECK:STDOUT: }
+// CHECK:STDOUT:
+// CHECK:STDOUT: fn @Main() {
+// CHECK:STDOUT: !entry:
+// CHECK:STDOUT:   %x.var.loc8: ref i32 = var "x"
+// CHECK:STDOUT:   %x.loc8: ref i32 = bind_name "x", %x.var.loc8
+// CHECK:STDOUT:   %.loc8: i32 = int_literal 0
+// CHECK:STDOUT:   assign %x.var.loc8, %.loc8
+// CHECK:STDOUT:   %.loc9: bool = bool_literal true
+// CHECK:STDOUT:   if %.loc9 br !if.then else br !if.else
+// CHECK:STDOUT:
+// CHECK:STDOUT: !if.then:
+// CHECK:STDOUT:   %x.var.loc10: ref i32 = var "x"
+// CHECK:STDOUT:   %x.loc10: ref i32 = bind_name "x", %x.var.loc10
+// CHECK:STDOUT:   %.loc10: i32 = int_literal 0
+// CHECK:STDOUT:   assign %x.var.loc10, %.loc10
+// CHECK:STDOUT:   %x.ref: ref i32 = name_reference "x", %x.loc10
+// CHECK:STDOUT:   %.loc13: i32 = int_literal 1
+// CHECK:STDOUT:   assign %x.ref, %.loc13
+// CHECK:STDOUT:   br !if.else
+// CHECK:STDOUT:
+// CHECK:STDOUT: !if.else:
+// CHECK:STDOUT:   return
+// CHECK:STDOUT: }