Jelajahi Sumber

Start pulling in names from cross-package imports. (#3789)

Note, I'm annotating the lookup partly so that the reason the conflict
comes up is clear, partly so that there's actually a diagnostic line
associated with the root cause as more tests get packed into a single
file.
Jon Ross-Perkins 2 tahun lalu
induk
melakukan
957f11587d

+ 12 - 0
toolchain/base/value_store.h

@@ -200,6 +200,14 @@ class ValueStore<StringId> : public Yaml::Printable<ValueStore<StringId>> {
     return values_[id.index];
   }
 
+  // Returns an ID for the value, or Invalid if not found.
+  auto Lookup(llvm::StringRef value) const -> StringId {
+    if (auto it = map_.find(value); it != map_.end()) {
+      return it->second;
+    }
+    return StringId::Invalid;
+  }
+
   auto OutputYaml() const -> Yaml::OutputMapping {
     return Yaml::OutputMapping([&](Yaml::OutputMapping::Map map) {
       for (auto [i, val] : llvm::enumerate(values_)) {
@@ -233,6 +241,10 @@ class StringStoreWrapper : public Printable<StringStoreWrapper<IdT>> {
     return values_->Get(StringId(id.index));
   }
 
+  auto Lookup(llvm::StringRef value) const -> IdT {
+    return IdT(values_->Lookup(value).index);
+  }
+
   auto Print(llvm::raw_ostream& out) const -> void { out << *values_; }
 
   auto size() const -> size_t { return values_->size(); }

+ 1 - 0
toolchain/check/BUILD

@@ -78,6 +78,7 @@ cc_library(
         "//common:vlog",
         "//toolchain/base:index_base",
         "//toolchain/check:scope_stack",
+        "//toolchain/diagnostics:diagnostic_emitter",
         "//toolchain/lex:tokenized_buffer",
         "//toolchain/parse:node_kind",
         "//toolchain/parse:tree",

+ 63 - 4
toolchain/check/context.cpp

@@ -14,6 +14,7 @@
 #include "toolchain/check/eval.h"
 #include "toolchain/check/import_ref.h"
 #include "toolchain/check/inst_block_stack.h"
+#include "toolchain/diagnostics/diagnostic_emitter.h"
 #include "toolchain/lex/tokenized_buffer.h"
 #include "toolchain/parse/node_ids.h"
 #include "toolchain/parse/node_kind.h"
@@ -190,7 +191,7 @@ auto Context::AddNameToLookup(SemIR::NameId name_id, SemIR::InstId target_id)
   }
 }
 
-auto Context::LookupNameInDecl(Parse::NodeId /*node_id*/, SemIR::NameId name_id,
+auto Context::LookupNameInDecl(Parse::NodeId node_id, SemIR::NameId name_id,
                                SemIR::NameScopeId scope_id) -> SemIR::InstId {
   if (!scope_id.is_valid()) {
     // Look for a name in the current scope only. There are two cases where the
@@ -227,7 +228,8 @@ auto Context::LookupNameInDecl(Parse::NodeId /*node_id*/, SemIR::NameId name_id,
     //
     //    // Error, no `F` in `B`.
     //    fn B.F() {}
-    return LookupNameInExactScope(name_id, name_scopes().Get(scope_id));
+    return LookupNameInExactScope(node_id, name_id,
+                                  name_scopes().Get(scope_id));
   }
 }
 
@@ -259,13 +261,70 @@ auto Context::LookupUnqualifiedName(Parse::NodeId node_id,
   return SemIR::InstId::BuiltinError;
 }
 
-auto Context::LookupNameInExactScope(SemIR::NameId name_id,
+// Handles lookup through the import_ir_scopes for LookupNameInExactScope.
+static auto LookupInImportIRScopes(Context& context, SemIRLocation loc,
+                                   SemIR::NameId name_id,
+                                   const SemIR::NameScope& scope)
+    -> SemIR::InstId {
+  auto identifier_id = name_id.AsIdentifierId();
+  llvm::StringRef identifier;
+  if (identifier_id.is_valid()) {
+    identifier = context.identifiers().Get(identifier_id);
+  }
+
+  DiagnosticAnnotationScope annotate_diagnostics(
+      &context.emitter(), [&](auto& builder) {
+        CARBON_DIAGNOSTIC(InNameLookup, Note, "In name lookup for `{0}`.",
+                          SemIR::NameId);
+        builder.Note(loc, InNameLookup, name_id);
+      });
+
+  auto result_id = SemIR::InstId::Invalid;
+  for (auto [import_ir_id, import_scope_id] : scope.import_ir_scopes) {
+    auto& import_ir = context.import_irs().Get(import_ir_id);
+
+    // Determine the NameId in the import IR.
+    SemIR::NameId import_name_id = name_id;
+    if (identifier_id.is_valid()) {
+      auto import_identifier_id = import_ir->identifiers().Lookup(identifier);
+      if (!import_identifier_id.is_valid()) {
+        // Name doesn't exist in the import IR.
+        continue;
+      }
+      import_name_id = SemIR::NameId::ForIdentifier(import_identifier_id);
+    }
+
+    // Look up the name in the import scope.
+    const auto& import_scope = import_ir->name_scopes().Get(import_scope_id);
+    auto it = import_scope.names.find(import_name_id);
+    if (it == import_scope.names.end()) {
+      // Name doesn't exist in the import scope.
+      continue;
+    }
+    auto import_inst_id = context.AddPlaceholderInst(
+        {SemIR::ImportRefUnused{import_ir_id, it->second}});
+    TryResolveImportRefUnused(context, import_inst_id);
+    if (result_id.is_valid()) {
+      // TODO: Add generalized merge functionality (merge_decls.h?).
+      context.DiagnoseDuplicateName(import_inst_id, result_id);
+    } else {
+      result_id = import_inst_id;
+    }
+  }
+
+  return result_id;
+}
+
+auto Context::LookupNameInExactScope(SemIRLocation loc, SemIR::NameId name_id,
                                      const SemIR::NameScope& scope)
     -> SemIR::InstId {
   if (auto it = scope.names.find(name_id); it != scope.names.end()) {
     TryResolveImportRefUnused(*this, it->second);
     return it->second;
   }
+  if (!scope.import_ir_scopes.empty()) {
+    return LookupInImportIRScopes(*this, loc, name_id, scope);
+  }
   return SemIR::InstId::Invalid;
 }
 
@@ -281,7 +340,7 @@ auto Context::LookupQualifiedName(Parse::NodeId node_id, SemIR::NameId name_id,
     const auto& scope = name_scopes().Get(scope_ids.pop_back_val());
     has_error |= scope.has_error;
 
-    auto scope_result_id = LookupNameInExactScope(name_id, scope);
+    auto scope_result_id = LookupNameInExactScope(node_id, name_id, scope);
     if (!scope_result_id.is_valid()) {
       // Nothing found in this scope: also look in its extended scopes.
       auto extended = llvm::reverse(scope.extended_scopes);

+ 1 - 1
toolchain/check/context.h

@@ -122,7 +122,7 @@ class Context {
   // Performs a name lookup in a specified scope, returning the referenced
   // instruction. Does not look into extended scopes. Returns an invalid
   // instruction if the name is not found.
-  auto LookupNameInExactScope(SemIR::NameId name_id,
+  auto LookupNameInExactScope(SemIRLocation loc, SemIR::NameId name_id,
                               const SemIR::NameScope& scope) -> SemIR::InstId;
 
   // Performs a qualified name lookup in a specified scope and in scopes that

+ 1 - 1
toolchain/check/impl.cpp

@@ -90,7 +90,7 @@ static auto BuildInterfaceWitness(
     if (auto fn_decl = decl.TryAs<SemIR::FunctionDecl>()) {
       auto& fn = context.functions().Get(fn_decl->function_id);
       auto impl_decl_id =
-          context.LookupNameInExactScope(fn.name_id, impl_scope);
+          context.LookupNameInExactScope(decl_id, fn.name_id, impl_scope);
       if (impl_decl_id.is_valid()) {
         used_decl_ids.push_back(impl_decl_id);
         table.push_back(CheckAssociatedFunctionImplementation(

+ 295 - 0
toolchain/check/testdata/class/cross_package_import.carbon

@@ -0,0 +1,295 @@
+// 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
+
+// ============================================================================
+// Setup files
+// ============================================================================
+
+// --- other_define.carbon
+
+package Other library "define" api;
+
+class C {}
+
+// --- other_extern.carbon
+
+package Other library "extern" api;
+
+// TODO: Mark extern
+class C;
+
+// --- other_conflict.carbon
+
+package Other library "conflict" api;
+
+fn C() {}
+
+// ============================================================================
+// Test files
+// ============================================================================
+
+// --- define.carbon
+
+library "define" api;
+
+import Other library "define";
+
+var c: Other.C = {};
+
+// --- fail_extern.carbon
+
+library "extern" api;
+
+import Other library "extern";
+
+// CHECK:STDERR: fail_extern.carbon:[[@LINE+10]]:8: ERROR: Variable has incomplete type `C`.
+// CHECK:STDERR: var c: Other.C = {};
+// CHECK:STDERR:        ^~~~~~~
+// CHECK:STDERR: fail_extern.carbon: Class was forward declared here.
+// CHECK:STDERR: other_extern.carbon:5:1: ERROR: Duplicate name being declared in the same scope.
+// CHECK:STDERR: class C;
+// CHECK:STDERR: ^~~~~~~~
+// CHECK:STDERR: other_define.carbon:4:1: Name is previously declared here.
+// CHECK:STDERR: class C {}
+// CHECK:STDERR: ^~~~~~~~~
+var c: Other.C = {};
+
+// --- fail_merge_define_extern.carbon
+
+library "fail_merge_define_extern" api;
+
+import Other library "define";
+import Other library "extern";
+
+// CHECK:STDERR: fail_merge_define_extern.carbon:[[@LINE+9]]:8: In name lookup for `C`.
+// CHECK:STDERR: var c: Other.C = {};
+// CHECK:STDERR:        ^~~~~~~
+// CHECK:STDERR: other_conflict.carbon:4:1: ERROR: Duplicate name being declared in the same scope.
+// CHECK:STDERR: fn C() {}
+// CHECK:STDERR: ^~~~~~~~
+// CHECK:STDERR: other_define.carbon:4:1: Name is previously declared here.
+// CHECK:STDERR: class C {}
+// CHECK:STDERR: ^~~~~~~~~
+var c: Other.C = {};
+
+// --- fail_conflict.carbon
+
+library "conflict" api;
+
+import Other library "define";
+import Other library "conflict";
+
+// CHECK:STDERR: fail_conflict.carbon:[[@LINE+3]]:8: In name lookup for `C`.
+// CHECK:STDERR: var c: Other.C = {};
+// CHECK:STDERR:        ^~~~~~~
+var c: Other.C = {};
+
+// CHECK:STDOUT: --- other_define.carbon
+// CHECK:STDOUT:
+// CHECK:STDOUT: constants {
+// CHECK:STDOUT:   %C: type = class_type @C [template]
+// CHECK:STDOUT:   %.1: type = struct_type {} [template]
+// CHECK:STDOUT: }
+// CHECK:STDOUT:
+// CHECK:STDOUT: file {
+// CHECK:STDOUT:   package: <namespace> = namespace [template] {
+// CHECK:STDOUT:     .C = %C.decl
+// CHECK:STDOUT:   }
+// CHECK:STDOUT:   %C.decl: type = class_decl @C [template = constants.%C] {}
+// CHECK:STDOUT: }
+// CHECK:STDOUT:
+// CHECK:STDOUT: class @C {
+// CHECK:STDOUT: !members:
+// CHECK:STDOUT:   .Self = constants.%C
+// CHECK:STDOUT: }
+// CHECK:STDOUT:
+// CHECK:STDOUT: --- other_extern.carbon
+// CHECK:STDOUT:
+// CHECK:STDOUT: constants {
+// CHECK:STDOUT:   %C: type = class_type @C [template]
+// CHECK:STDOUT: }
+// CHECK:STDOUT:
+// CHECK:STDOUT: file {
+// CHECK:STDOUT:   package: <namespace> = namespace [template] {
+// CHECK:STDOUT:     .C = %C.decl
+// CHECK:STDOUT:   }
+// CHECK:STDOUT:   %C.decl: type = class_decl @C [template = constants.%C] {}
+// CHECK:STDOUT: }
+// CHECK:STDOUT:
+// CHECK:STDOUT: class @C;
+// CHECK:STDOUT:
+// CHECK:STDOUT: --- other_conflict.carbon
+// CHECK:STDOUT:
+// CHECK:STDOUT: file {
+// CHECK:STDOUT:   package: <namespace> = namespace [template] {
+// CHECK:STDOUT:     .C = %C
+// CHECK:STDOUT:   }
+// CHECK:STDOUT:   %C: <function> = fn_decl @C [template] {}
+// CHECK:STDOUT: }
+// CHECK:STDOUT:
+// CHECK:STDOUT: fn @C() {
+// CHECK:STDOUT: !entry:
+// CHECK:STDOUT:   return
+// CHECK:STDOUT: }
+// CHECK:STDOUT:
+// CHECK:STDOUT: --- define.carbon
+// CHECK:STDOUT:
+// CHECK:STDOUT: constants {
+// CHECK:STDOUT:   %C: type = class_type @C [template]
+// CHECK:STDOUT:   %.1: type = struct_type {} [template]
+// CHECK:STDOUT:   %.2: type = tuple_type () [template]
+// CHECK:STDOUT:   %.3: type = ptr_type {} [template]
+// CHECK:STDOUT:   %.4: C = struct_value () [template]
+// CHECK:STDOUT: }
+// CHECK:STDOUT:
+// CHECK:STDOUT: file {
+// CHECK:STDOUT:   package: <namespace> = namespace [template] {
+// CHECK:STDOUT:     .Other = %Other
+// CHECK:STDOUT:     .c = %c
+// CHECK:STDOUT:   }
+// CHECK:STDOUT:   %Other: <namespace> = namespace [template] {}
+// CHECK:STDOUT:   %Other.ref: <namespace> = name_ref Other, %Other [template = %Other]
+// CHECK:STDOUT:   %import_ref: type = import_ref ir1, inst+1, used [template = constants.%C]
+// CHECK:STDOUT:   %C.decl: invalid = class_decl @C [template = constants.%C] {}
+// CHECK:STDOUT:   %C.ref: type = name_ref C, %import_ref [template = constants.%C]
+// CHECK:STDOUT:   %c.var: ref C = var c
+// CHECK:STDOUT:   %c: ref C = bind_name c, %c.var
+// CHECK:STDOUT: }
+// CHECK:STDOUT:
+// CHECK:STDOUT: class @C {
+// CHECK:STDOUT:   %import_ref = import_ref ir1, inst+2, unused
+// CHECK:STDOUT:
+// CHECK:STDOUT: !members:
+// CHECK:STDOUT:   .Self = %import_ref
+// CHECK:STDOUT: }
+// CHECK:STDOUT:
+// CHECK:STDOUT: fn @__global_init() {
+// CHECK:STDOUT: !entry:
+// CHECK:STDOUT:   %.loc6_19.1: {} = struct_literal ()
+// CHECK:STDOUT:   %.loc6_19.2: init C = class_init (), file.%c.var [template = constants.%.4]
+// CHECK:STDOUT:   %.loc6_19.3: init C = converted %.loc6_19.1, %.loc6_19.2 [template = constants.%.4]
+// CHECK:STDOUT:   assign file.%c.var, %.loc6_19.3
+// CHECK:STDOUT:   return
+// CHECK:STDOUT: }
+// CHECK:STDOUT:
+// CHECK:STDOUT: --- fail_extern.carbon
+// CHECK:STDOUT:
+// CHECK:STDOUT: constants {
+// CHECK:STDOUT:   %C: type = class_type @C [template]
+// CHECK:STDOUT:   %.1: type = struct_type {} [template]
+// CHECK:STDOUT: }
+// CHECK:STDOUT:
+// CHECK:STDOUT: file {
+// CHECK:STDOUT:   package: <namespace> = namespace [template] {
+// CHECK:STDOUT:     .Other = %Other
+// CHECK:STDOUT:     .c = %c
+// CHECK:STDOUT:   }
+// CHECK:STDOUT:   %Other: <namespace> = namespace [template] {}
+// CHECK:STDOUT:   %Other.ref: <namespace> = name_ref Other, %Other [template = %Other]
+// CHECK:STDOUT:   %import_ref: type = import_ref ir1, inst+1, used [template = constants.%C]
+// CHECK:STDOUT:   %C.decl: invalid = class_decl @C [template = constants.%C] {}
+// CHECK:STDOUT:   %C.ref: type = name_ref C, %import_ref [template = constants.%C]
+// CHECK:STDOUT:   %c.var: ref <error> = var c
+// CHECK:STDOUT:   %c: ref <error> = bind_name c, %c.var
+// CHECK:STDOUT: }
+// CHECK:STDOUT:
+// CHECK:STDOUT: class @C;
+// CHECK:STDOUT:
+// CHECK:STDOUT: fn @__global_init() {
+// CHECK:STDOUT: !entry:
+// CHECK:STDOUT:   %.loc16: {} = struct_literal ()
+// CHECK:STDOUT:   assign file.%c.var, <error>
+// CHECK:STDOUT:   return
+// CHECK:STDOUT: }
+// CHECK:STDOUT:
+// CHECK:STDOUT: --- fail_merge_define_extern.carbon
+// CHECK:STDOUT:
+// CHECK:STDOUT: constants {
+// CHECK:STDOUT:   %C.1: type = class_type @C.1 [template]
+// CHECK:STDOUT:   %.1: type = struct_type {} [template]
+// CHECK:STDOUT:   %C.2: type = class_type @C.2 [template]
+// CHECK:STDOUT:   %.2: type = tuple_type () [template]
+// CHECK:STDOUT:   %.3: type = ptr_type {} [template]
+// CHECK:STDOUT:   %.4: C = struct_value () [template]
+// CHECK:STDOUT: }
+// CHECK:STDOUT:
+// CHECK:STDOUT: file {
+// CHECK:STDOUT:   package: <namespace> = namespace [template] {
+// CHECK:STDOUT:     .Other = %Other
+// CHECK:STDOUT:     .c = %c
+// CHECK:STDOUT:   }
+// CHECK:STDOUT:   %Other: <namespace> = namespace [template] {}
+// CHECK:STDOUT:   %Other.ref: <namespace> = name_ref Other, %Other [template = %Other]
+// CHECK:STDOUT:   %import_ref.1: type = import_ref ir1, inst+1, used [template = constants.%C.1]
+// CHECK:STDOUT:   %C.decl.1: invalid = class_decl @C.1 [template = constants.%C.1] {}
+// CHECK:STDOUT:   %import_ref.2: type = import_ref ir2, inst+1, used [template = constants.%C.2]
+// CHECK:STDOUT:   %C.decl.2: invalid = class_decl @C.2 [template = constants.%C.2] {}
+// CHECK:STDOUT:   %C.ref: type = name_ref C, %import_ref.1 [template = constants.%C.1]
+// CHECK:STDOUT:   %c.var: ref C = var c
+// CHECK:STDOUT:   %c: ref C = bind_name c, %c.var
+// CHECK:STDOUT: }
+// CHECK:STDOUT:
+// CHECK:STDOUT: class @C.1 {
+// CHECK:STDOUT:   %import_ref = import_ref ir1, inst+2, unused
+// CHECK:STDOUT:
+// CHECK:STDOUT: !members:
+// CHECK:STDOUT:   .Self = %import_ref
+// CHECK:STDOUT: }
+// CHECK:STDOUT:
+// CHECK:STDOUT: class @C.2;
+// CHECK:STDOUT:
+// CHECK:STDOUT: fn @__global_init() {
+// CHECK:STDOUT: !entry:
+// CHECK:STDOUT:   %.loc16_19.1: {} = struct_literal ()
+// CHECK:STDOUT:   %.loc16_19.2: init C = class_init (), file.%c.var [template = constants.%.4]
+// CHECK:STDOUT:   %.loc16_19.3: init C = converted %.loc16_19.1, %.loc16_19.2 [template = constants.%.4]
+// CHECK:STDOUT:   assign file.%c.var, %.loc16_19.3
+// CHECK:STDOUT:   return
+// CHECK:STDOUT: }
+// CHECK:STDOUT:
+// CHECK:STDOUT: --- fail_conflict.carbon
+// CHECK:STDOUT:
+// CHECK:STDOUT: constants {
+// CHECK:STDOUT:   %C: type = class_type @C.2 [template]
+// CHECK:STDOUT:   %.1: type = struct_type {} [template]
+// CHECK:STDOUT:   %.2: type = tuple_type () [template]
+// CHECK:STDOUT:   %.3: type = ptr_type {} [template]
+// CHECK:STDOUT:   %.4: C = struct_value () [template]
+// CHECK:STDOUT: }
+// CHECK:STDOUT:
+// CHECK:STDOUT: file {
+// CHECK:STDOUT:   package: <namespace> = namespace [template] {
+// CHECK:STDOUT:     .Other = %Other
+// CHECK:STDOUT:     .c = %c
+// CHECK:STDOUT:   }
+// CHECK:STDOUT:   %Other: <namespace> = namespace [template] {}
+// CHECK:STDOUT:   %Other.ref: <namespace> = name_ref Other, %Other [template = %Other]
+// CHECK:STDOUT:   %import_ref.1: type = import_ref ir1, inst+1, used [template = constants.%C]
+// CHECK:STDOUT:   %C.decl: invalid = class_decl @C.2 [template = constants.%C] {}
+// CHECK:STDOUT:   %import_ref.2: <function> = import_ref ir2, inst+1, used [template = imports.%C]
+// CHECK:STDOUT:   %C.ref: type = name_ref C, %import_ref.1 [template = constants.%C]
+// CHECK:STDOUT:   %c.var: ref C = var c
+// CHECK:STDOUT:   %c: ref C = bind_name c, %c.var
+// CHECK:STDOUT: }
+// CHECK:STDOUT:
+// CHECK:STDOUT: class @C.2 {
+// CHECK:STDOUT:   %import_ref = import_ref ir1, inst+2, unused
+// CHECK:STDOUT:
+// CHECK:STDOUT: !members:
+// CHECK:STDOUT:   .Self = %import_ref
+// CHECK:STDOUT: }
+// CHECK:STDOUT:
+// CHECK:STDOUT: fn @C.1();
+// CHECK:STDOUT:
+// CHECK:STDOUT: fn @__global_init() {
+// CHECK:STDOUT: !entry:
+// CHECK:STDOUT:   %.loc10_19.1: {} = struct_literal ()
+// CHECK:STDOUT:   %.loc10_19.2: init C = class_init (), file.%c.var [template = constants.%.4]
+// CHECK:STDOUT:   %.loc10_19.3: init C = converted %.loc10_19.1, %.loc10_19.2 [template = constants.%.4]
+// CHECK:STDOUT:   assign file.%c.var, %.loc10_19.3
+// CHECK:STDOUT:   return
+// CHECK:STDOUT: }
+// CHECK:STDOUT:

+ 122 - 23
toolchain/check/testdata/packages/cross_package_import.carbon

@@ -14,11 +14,24 @@ package Other library "fn" api;
 
 fn F() {}
 
+// --- other_fn_extern.carbon
+
+package Other library "fn_extern" api;
+
+// TODO: Mark extern
+// CHECK:STDERR: other_fn_extern.carbon:[[@LINE+6]]:1: ERROR: Duplicate name being declared in the same scope.
+// CHECK:STDERR: fn F();
+// CHECK:STDERR: ^~~~~~~
+// CHECK:STDERR: other_fn.carbon:4:1: Name is previously declared here.
+// CHECK:STDERR: fn F() {}
+// CHECK:STDERR: ^~~~~~~~
+fn F();
+
 // --- other_fn_conflict.carbon
 
 package Other library "fn_conflict" api;
 
-fn F() {}
+fn F(x: i32) {}
 
 // --- other_fn2.carbon
 
@@ -36,8 +49,7 @@ namespace Other;
 // Test files
 // ============================================================================
 
-// --- fail_main_use_other.carbon
-// TODO: Once name lookup works, nothing should fail here.
+// --- main_use_other.carbon
 
 library "use_other" api;
 
@@ -45,14 +57,28 @@ import Other library "fn";
 import Other library "fn2";
 
 fn Run() {
-  // CHECK:STDERR: fail_main_use_other.carbon:[[@LINE+3]]:3: ERROR: Name `F` not found.
+  Other.F();
+  Other.F2();
+}
+
+// --- fail_main_use_other_extern.carbon
+
+library "use_other_extern" api;
+
+import Other library "fn";
+import Other library "fn_extern";
+
+fn Run() {
+  // CHECK:STDERR: fail_main_use_other_extern.carbon:[[@LINE+9]]:3: In name lookup for `F`.
   // CHECK:STDERR:   Other.F();
   // CHECK:STDERR:   ^~~~~~~
+  // CHECK:STDERR: other_fn_conflict.carbon:4:1: ERROR: Duplicate name being declared in the same scope.
+  // CHECK:STDERR: fn F(x: i32) {}
+  // CHECK:STDERR: ^~~~~~~~~~~~~~
+  // CHECK:STDERR: other_fn.carbon:4:1: Name is previously declared here.
+  // CHECK:STDERR: fn F() {}
+  // CHECK:STDERR: ^~~~~~~~
   Other.F();
-  // CHECK:STDERR: fail_main_use_other.carbon:[[@LINE+3]]:3: ERROR: Name `F2` not found.
-  // CHECK:STDERR:   Other.F2();
-  // CHECK:STDERR:   ^~~~~~~~
-  Other.F2();
 }
 
 // --- main_unused_other_ambiguous.carbon
@@ -63,7 +89,6 @@ import Other library "fn";
 import Other library "fn_conflict";
 
 // --- fail_main_use_other_ambiguous.carbon
-// TODO: Once name lookup works, this should be ambiguous.
 
 library "use_other_ambiguous" api;
 
@@ -71,7 +96,7 @@ import Other library "fn";
 import Other library "fn_conflict";
 
 fn Run() {
-  // CHECK:STDERR: fail_main_use_other_ambiguous.carbon:[[@LINE+3]]:3: ERROR: Name `F` not found.
+  // CHECK:STDERR: fail_main_use_other_ambiguous.carbon:[[@LINE+3]]:3: In name lookup for `F`.
   // CHECK:STDERR:   Other.F();
   // CHECK:STDERR:   ^~~~~~~
   Other.F();
@@ -90,6 +115,12 @@ import library "other_ns";
 // CHECK:STDERR: ^~~~~~~~~~~~~~~~
 import Other library "fn";
 
+// CHECK:STDERR: fail_main_namespace_conflict.carbon:[[@LINE+6]]:1: ERROR: Duplicate name being declared in the same scope.
+// CHECK:STDERR: fn Other.F() {}
+// CHECK:STDERR: ^~~~~~~~~~~~~~
+// CHECK:STDERR: other_fn.carbon:4:1: Name is previously declared here.
+// CHECK:STDERR: fn F() {}
+// CHECK:STDERR: ^~~~~~~~
 fn Other.F() {}
 
 // --- fail_main_reopen_other.carbon
@@ -137,7 +168,7 @@ fn Other.G() {}
 // CHECK:STDOUT:   return
 // CHECK:STDOUT: }
 // CHECK:STDOUT:
-// CHECK:STDOUT: --- other_fn_conflict.carbon
+// CHECK:STDOUT: --- other_fn_extern.carbon
 // CHECK:STDOUT:
 // CHECK:STDOUT: file {
 // CHECK:STDOUT:   package: <namespace> = namespace [template] {
@@ -146,7 +177,21 @@ fn Other.G() {}
 // CHECK:STDOUT:   %F: <function> = fn_decl @F [template] {}
 // CHECK:STDOUT: }
 // CHECK:STDOUT:
-// CHECK:STDOUT: fn @F() {
+// CHECK:STDOUT: fn @F();
+// CHECK:STDOUT:
+// CHECK:STDOUT: --- other_fn_conflict.carbon
+// CHECK:STDOUT:
+// CHECK:STDOUT: file {
+// CHECK:STDOUT:   package: <namespace> = namespace [template] {
+// CHECK:STDOUT:     .F = %F
+// CHECK:STDOUT:   }
+// CHECK:STDOUT:   %F: <function> = fn_decl @F [template] {
+// CHECK:STDOUT:     %x.loc4_6.1: i32 = param x
+// CHECK:STDOUT:     @F.%x: i32 = bind_name x, %x.loc4_6.1
+// CHECK:STDOUT:   }
+// CHECK:STDOUT: }
+// CHECK:STDOUT:
+// CHECK:STDOUT: fn @F(%x: i32) {
 // CHECK:STDOUT: !entry:
 // CHECK:STDOUT:   return
 // CHECK:STDOUT: }
@@ -174,7 +219,11 @@ fn Other.G() {}
 // CHECK:STDOUT:   %Other: <namespace> = namespace [template] {}
 // CHECK:STDOUT: }
 // CHECK:STDOUT:
-// CHECK:STDOUT: --- fail_main_use_other.carbon
+// CHECK:STDOUT: --- main_use_other.carbon
+// CHECK:STDOUT:
+// CHECK:STDOUT: constants {
+// CHECK:STDOUT:   %.1: type = tuple_type () [template]
+// CHECK:STDOUT: }
 // CHECK:STDOUT:
 // CHECK:STDOUT: file {
 // CHECK:STDOUT:   package: <namespace> = namespace [template] {
@@ -187,13 +236,50 @@ fn Other.G() {}
 // CHECK:STDOUT:
 // CHECK:STDOUT: fn @Run() {
 // CHECK:STDOUT: !entry:
-// CHECK:STDOUT:   %Other.ref.loc12: <namespace> = name_ref Other, file.%Other [template = file.%Other]
-// CHECK:STDOUT:   %F.ref: <error> = name_ref F, <error> [template = <error>]
-// CHECK:STDOUT:   %Other.ref.loc16: <namespace> = name_ref Other, file.%Other [template = file.%Other]
-// CHECK:STDOUT:   %F2.ref: <error> = name_ref F2, <error> [template = <error>]
+// CHECK:STDOUT:   %Other.ref.loc8: <namespace> = name_ref Other, file.%Other [template = file.%Other]
+// CHECK:STDOUT:   %import_ref.1: <function> = import_ref ir1, inst+1, used [template = imports.%F]
+// CHECK:STDOUT:   %F.ref: <function> = name_ref F, %import_ref.1 [template = imports.%F]
+// CHECK:STDOUT:   %.loc8: init () = call %F.ref()
+// CHECK:STDOUT:   %Other.ref.loc9: <namespace> = name_ref Other, file.%Other [template = file.%Other]
+// CHECK:STDOUT:   %import_ref.2: <function> = import_ref ir2, inst+1, used [template = imports.%F2]
+// CHECK:STDOUT:   %F2.ref: <function> = name_ref F2, %import_ref.2 [template = imports.%F2]
+// CHECK:STDOUT:   %.loc9: init () = call %F2.ref()
 // CHECK:STDOUT:   return
 // CHECK:STDOUT: }
 // CHECK:STDOUT:
+// CHECK:STDOUT: fn @F();
+// CHECK:STDOUT:
+// CHECK:STDOUT: fn @F2();
+// CHECK:STDOUT:
+// CHECK:STDOUT: --- fail_main_use_other_extern.carbon
+// CHECK:STDOUT:
+// CHECK:STDOUT: constants {
+// CHECK:STDOUT:   %.1: type = tuple_type () [template]
+// CHECK:STDOUT: }
+// CHECK:STDOUT:
+// CHECK:STDOUT: file {
+// CHECK:STDOUT:   package: <namespace> = namespace [template] {
+// CHECK:STDOUT:     .Other = %Other
+// CHECK:STDOUT:     .Run = %Run
+// CHECK:STDOUT:   }
+// CHECK:STDOUT:   %Other: <namespace> = namespace [template] {}
+// CHECK:STDOUT:   %Run: <function> = fn_decl @Run [template] {}
+// CHECK:STDOUT: }
+// CHECK:STDOUT:
+// CHECK:STDOUT: fn @Run() {
+// CHECK:STDOUT: !entry:
+// CHECK:STDOUT:   %Other.ref: <namespace> = name_ref Other, file.%Other [template = file.%Other]
+// CHECK:STDOUT:   %import_ref.1: <function> = import_ref ir1, inst+1, used [template = imports.%F.1]
+// CHECK:STDOUT:   %import_ref.2: <function> = import_ref ir2, inst+1, used [template = imports.%F.2]
+// CHECK:STDOUT:   %F.ref: <function> = name_ref F, %import_ref.1 [template = imports.%F.1]
+// CHECK:STDOUT:   %.loc17: init () = call %F.ref()
+// CHECK:STDOUT:   return
+// CHECK:STDOUT: }
+// CHECK:STDOUT:
+// CHECK:STDOUT: fn @F.1();
+// CHECK:STDOUT:
+// CHECK:STDOUT: fn @F.2();
+// CHECK:STDOUT:
 // CHECK:STDOUT: --- main_unused_other_ambiguous.carbon
 // CHECK:STDOUT:
 // CHECK:STDOUT: file {
@@ -205,6 +291,10 @@ fn Other.G() {}
 // CHECK:STDOUT:
 // CHECK:STDOUT: --- fail_main_use_other_ambiguous.carbon
 // CHECK:STDOUT:
+// CHECK:STDOUT: constants {
+// CHECK:STDOUT:   %.1: type = tuple_type () [template]
+// CHECK:STDOUT: }
+// CHECK:STDOUT:
 // CHECK:STDOUT: file {
 // CHECK:STDOUT:   package: <namespace> = namespace [template] {
 // CHECK:STDOUT:     .Other = %Other
@@ -217,24 +307,33 @@ fn Other.G() {}
 // CHECK:STDOUT: fn @Run() {
 // CHECK:STDOUT: !entry:
 // CHECK:STDOUT:   %Other.ref: <namespace> = name_ref Other, file.%Other [template = file.%Other]
-// CHECK:STDOUT:   %F.ref: <error> = name_ref F, <error> [template = <error>]
+// CHECK:STDOUT:   %import_ref.1: <function> = import_ref ir1, inst+1, used [template = imports.%F.1]
+// CHECK:STDOUT:   %import_ref.2: <function> = import_ref ir2, inst+3, used [template = imports.%F.2]
+// CHECK:STDOUT:   %F.ref: <function> = name_ref F, %import_ref.1 [template = imports.%F.1]
+// CHECK:STDOUT:   %.loc11: init () = call %F.ref()
 // CHECK:STDOUT:   return
 // CHECK:STDOUT: }
 // CHECK:STDOUT:
+// CHECK:STDOUT: fn @F.1();
+// CHECK:STDOUT:
+// CHECK:STDOUT: fn @F.2(%x: i32);
+// CHECK:STDOUT:
 // CHECK:STDOUT: --- fail_main_namespace_conflict.carbon
 // CHECK:STDOUT:
 // CHECK:STDOUT: file {
 // CHECK:STDOUT:   package: <namespace> = namespace [template] {
 // CHECK:STDOUT:     .Other = %Other
 // CHECK:STDOUT:   }
-// CHECK:STDOUT:   %import_ref: <namespace> = import_ref ir1, inst+1, used
-// CHECK:STDOUT:   %Other: <namespace> = namespace %import_ref, [template] {
-// CHECK:STDOUT:     .F = %F
+// CHECK:STDOUT:   %import_ref.1: <namespace> = import_ref ir1, inst+1, used
+// CHECK:STDOUT:   %Other: <namespace> = namespace %import_ref.1, [template] {}
+// CHECK:STDOUT:   %.loc19: <function> = fn_decl @.1 [template] {
+// CHECK:STDOUT:     %import_ref.2: <function> = import_ref ir2, inst+1, used [template = imports.%F]
 // CHECK:STDOUT:   }
-// CHECK:STDOUT:   %F: <function> = fn_decl @F [template] {}
 // CHECK:STDOUT: }
 // CHECK:STDOUT:
-// CHECK:STDOUT: fn @F() {
+// CHECK:STDOUT: fn @F();
+// CHECK:STDOUT:
+// CHECK:STDOUT: fn @.1() {
 // CHECK:STDOUT: !entry:
 // CHECK:STDOUT:   return
 // CHECK:STDOUT: }

+ 3 - 0
toolchain/diagnostics/diagnostic_kind.def

@@ -217,6 +217,9 @@ CARBON_DIAGNOSTIC_KIND(QualifiedDeclOutsideScopeEntity)
 CARBON_DIAGNOSTIC_KIND(QualifiedDeclInIncompleteClassScope)
 CARBON_DIAGNOSTIC_KIND(QualifiedDeclInUndefinedInterfaceScope)
 
+// Name lookup.
+CARBON_DIAGNOSTIC_KIND(InNameLookup)
+
 CARBON_DIAGNOSTIC_KIND(AddrOfEphemeralRef)
 CARBON_DIAGNOSTIC_KIND(AddrOfNonRef)
 CARBON_DIAGNOSTIC_KIND(AddrOnNonSelfParam)