Fix member accesses that name a nested class. (#3356)

The value in the name lookup table is the class declaration. Map it to
the class type when it's found by lookup in an expression. This differs
from the behavior in a declaration name, where we want to find the class
declaration itself.

toolchain/check/handle_name.cpp

@@ -36,6 +36,24 @@ static auto GetAsNameScope(Context& context, SemIR::NodeId base_id)
   return std::nullopt;
 }
 
+// Given a node produced by a name lookup, get the value to use for that result
+// in an expression.
+static auto GetExpressionValueForLookupResult(Context& context,
+                                              SemIR::NodeId lookup_result_id)
+    -> SemIR::NodeId {
+  // If lookup finds a class declaration, the value is its `Self` type.
+  auto lookup_result = context.nodes().Get(lookup_result_id);
+  if (auto class_decl = lookup_result.TryAs<SemIR::ClassDeclaration>()) {
+    return context.sem_ir().GetTypeAllowBuiltinTypes(
+        context.classes().Get(class_decl->class_id).self_type_id);
+  }
+
+  // Anything else should be a typed value already.
+  CARBON_CHECK(lookup_result.kind().value_kind() == SemIR::NodeValueKind::Typed)
+      << "Unexpected kind for lookup result";
+  return lookup_result_id;
+}
+
 auto HandleMemberAccessExpression(Context& context, Parse::Node parse_node)
     -> bool {
   StringId name_id = context.node_stack().Pop<Parse::NodeKind::Name>();
@@ -48,6 +66,7 @@ auto HandleMemberAccessExpression(Context& context, Parse::Node parse_node)
                        ? context.LookupName(parse_node, name_id, *name_scope_id,
                                             /*print_diagnostics=*/true)
                        : SemIR::NodeId::BuiltinError;
+    node_id = GetExpressionValueForLookupResult(context, node_id);
     auto node = context.nodes().Get(node_id);
     // TODO: Track that this node was named within `base_id`.
     context.AddNodeAndPush(
@@ -86,9 +105,7 @@ auto HandleMemberAccessExpression(Context& context, Parse::Node parse_node)
                                 .scope_id;
       auto member_id = context.LookupName(parse_node, name_id, class_scope_id,
                                           /*print_diagnostics=*/true);
-      if (!member_id.is_valid()) {
-        break;
-      }
+      member_id = GetExpressionValueForLookupResult(context, member_id);
 
       // Perform instance binding if we found an instance member.
       auto member_type_id = context.nodes().Get(member_id).type_id();
@@ -209,16 +226,8 @@ auto HandleNameExpression(Context& context, Parse::Node parse_node) -> bool {
   auto value_id =
       context.LookupName(parse_node, name_id, SemIR::NameScopeId::Invalid,
                          /*print_diagnostics=*/true);
+  value_id = GetExpressionValueForLookupResult(context, value_id);
   auto value = context.nodes().Get(value_id);
-
-  // If lookup finds a class declaration, the value is its `Self` type.
-  if (auto class_decl = value.TryAs<SemIR::ClassDeclaration>()) {
-    value_id = context.sem_ir().GetTypeAllowBuiltinTypes(
-        context.classes().Get(class_decl->class_id).self_type_id);
-    value = context.nodes().Get(value_id);
-  }
-
-  CARBON_CHECK(value.kind().value_kind() == SemIR::NodeValueKind::Typed);
   context.AddNodeAndPush(
       parse_node,
       SemIR::NameReference{parse_node, value.type_id(), name_id, value_id});

toolchain/check/testdata/class/nested_name.carbon

@@ -0,0 +1,65 @@
+// 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 n: i32;
+  }
+}
+
+fn F(oi: Outer.Inner) -> i32 {
+  return oi.n;
+}
+
+fn G(o: Outer) {
+  var i: o.Inner;
+}
+
+// CHECK:STDOUT: file "nested_name.carbon" {
+// CHECK:STDOUT:   class_declaration @Outer, ()
+// CHECK:STDOUT:   %Outer: type = class_type @Outer
+// CHECK:STDOUT:   %.loc11: type = struct_type {}
+// CHECK:STDOUT:   %F: <function> = fn_decl @F
+// CHECK:STDOUT:   %G: <function> = fn_decl @G
+// CHECK:STDOUT: }
+// CHECK:STDOUT:
+// CHECK:STDOUT: class @Outer {
+// CHECK:STDOUT:   class_declaration @Inner, ()
+// CHECK:STDOUT:   %Inner: type = class_type @Inner
+// CHECK:STDOUT:   %.loc10: type = struct_type {.n: i32}
+// CHECK:STDOUT:
+// CHECK:STDOUT: !members:
+// CHECK:STDOUT:   .Inner = <unexpected noderef 11>
+// CHECK:STDOUT: }
+// CHECK:STDOUT:
+// CHECK:STDOUT: class @Inner {
+// CHECK:STDOUT:   %.loc9_10.1: type = unbound_field_type Inner, i32
+// CHECK:STDOUT:   %.loc9_10.2: <unbound field of class Inner> = field "n", member0
+// CHECK:STDOUT:   %n: <unbound field of class Inner> = bind_name "n", %.loc9_10.2
+// CHECK:STDOUT:
+// CHECK:STDOUT: !members:
+// CHECK:STDOUT:   .n = %n
+// CHECK:STDOUT: }
+// CHECK:STDOUT:
+// CHECK:STDOUT: fn @F(%oi: Inner) -> i32 {
+// CHECK:STDOUT: !entry:
+// CHECK:STDOUT:   %.loc8: type = ptr_type {.n: i32}
+// CHECK:STDOUT:   %oi.ref: Inner = name_reference "oi", %oi
+// CHECK:STDOUT:   %.loc14_12.1: ref i32 = class_field_access %oi.ref, member0
+// CHECK:STDOUT:   %.loc14_12.2: i32 = bind_value %.loc14_12.1
+// CHECK:STDOUT:   return %.loc14_12.2
+// CHECK:STDOUT: }
+// CHECK:STDOUT:
+// CHECK:STDOUT: fn @G(%o: Outer) {
+// CHECK:STDOUT: !entry:
+// CHECK:STDOUT:   %.loc11: type = tuple_type ()
+// CHECK:STDOUT:   %.loc7: type = ptr_type {}
+// CHECK:STDOUT:   %o.ref: Outer = name_reference "o", %o
+// CHECK:STDOUT:   %Inner.ref: type = name_reference "Inner", @Outer.%Inner
+// CHECK:STDOUT:   %i.var: ref Inner = var "i"
+// CHECK:STDOUT:   %i: ref Inner = bind_name "i", %i.var
+// CHECK:STDOUT:   return
+// CHECK:STDOUT: }