diff --git a/toolchain/check/check.cpp b/toolchain/check/check.cpp index ce5577cd9707..f1835605fe97 100644 --- a/toolchain/check/check.cpp +++ b/toolchain/check/check.cpp @@ -36,10 +36,11 @@ class SemIRDiagnosticConverter : public DiagnosticConverter { const SemIR::File* sem_ir) : node_converters_(node_converters), sem_ir_(sem_ir) {} - auto ConvertLocation(SemIRLocation loc) const -> DiagnosticLocation override { + auto ConvertLocation(SemIRLocation loc, ContextFnT context_fn) const + -> DiagnosticLocation override { // Parse nodes always refer to the current IR. if (!loc.is_inst_id) { - return ConvertLocationInFile(sem_ir_, loc.node_location); + return ConvertLocationInFile(sem_ir_, loc.node_location, context_fn); } const auto* cursor_ir = sem_ir_; @@ -48,14 +49,19 @@ class SemIRDiagnosticConverter : public DiagnosticConverter { // If the parse node is valid, use it for the location. if (auto node_id = cursor_ir->insts().GetNodeId(cursor_inst_id); node_id.is_valid()) { - return ConvertLocationInFile(cursor_ir, node_id); + return ConvertLocationInFile(cursor_ir, node_id, context_fn); } // If the parse node was invalid, recurse through import references when // possible. if (auto import_ref = cursor_ir->insts().TryGetAs( cursor_inst_id)) { - cursor_ir = cursor_ir->import_irs().Get(import_ref->ir_id); + const auto& import_ir = cursor_ir->import_irs().Get(import_ref->ir_id); + auto context_loc = + ConvertLocationInFile(cursor_ir, import_ir.node_id, context_fn); + CARBON_DIAGNOSTIC(InImport, Note, "In import."); + context_fn(context_loc, InImport); + cursor_ir = import_ir.sem_ir; cursor_inst_id = import_ref->inst_id; continue; } @@ -71,7 +77,8 @@ class SemIRDiagnosticConverter : public DiagnosticConverter { } // Invalid parse node but not an import; just nothing to point at. - return ConvertLocationInFile(cursor_ir, Parse::NodeId::Invalid); + return ConvertLocationInFile(cursor_ir, Parse::NodeId::Invalid, + context_fn); } } @@ -91,11 +98,12 @@ class SemIRDiagnosticConverter : public DiagnosticConverter { private: auto ConvertLocationInFile(const SemIR::File* sem_ir, - Parse::NodeLocation node_location) const + Parse::NodeLocation node_location, + ContextFnT context_fn) const -> DiagnosticLocation { auto it = node_converters_->find(sem_ir); CARBON_CHECK(it != node_converters_->end()); - return it->second->ConvertLocation(node_location); + return it->second->ConvertLocation(node_location, context_fn); } const llvm::DenseMap* @@ -191,7 +199,7 @@ static auto InitPackageScopeAndImports(Context& context, UnitInfo& unit_info) for (const auto& import : self_import->second.imports) { const auto& import_sem_ir = **import.unit_info->unit->sem_ir; ImportLibraryFromCurrentPackage(context, namespace_type_id, - import_sem_ir); + import.names.node_id, import_sem_ir); error_in_import |= import_sem_ir.name_scopes() .Get(SemIR::NameScopeId::Package) .has_error; @@ -216,13 +224,14 @@ static auto InitPackageScopeAndImports(Context& context, UnitInfo& unit_info) continue; } - llvm::SmallVector sem_irs; + llvm::SmallVector import_irs; for (auto import : package_imports.imports) { - sem_irs.push_back(&**import.unit_info->unit->sem_ir); + import_irs.push_back({.node_id = import.names.node_id, + .sem_ir = &**import.unit_info->unit->sem_ir}); } ImportLibrariesFromOtherPackage(context, namespace_type_id, package_imports.node_id, package_id, - sem_irs, package_imports.has_load_error); + import_irs, package_imports.has_load_error); } CARBON_CHECK(context.import_irs().size() == num_irs) diff --git a/toolchain/check/context.cpp b/toolchain/check/context.cpp index 8a055777d7c7..bdb3a61c2542 100644 --- a/toolchain/check/context.cpp +++ b/toolchain/check/context.cpp @@ -137,7 +137,7 @@ auto Context::ReplaceInstBeforeConstantUse( auto Context::AddImportRef(SemIR::ImportIRId ir_id, SemIR::InstId inst_id) -> SemIR::InstId { auto import_ref_id = - AddPlaceholderInstInNoBlock(SemIR::ImportRefUnused{ir_id, inst_id}); + AddPlaceholderInstInNoBlock({SemIR::ImportRefUnused{ir_id, inst_id}}); // We can't insert this instruction into whatever block we happen to be in, // because this function is typically called by name lookup in the middle of @@ -303,7 +303,8 @@ static auto LookupInImportIRScopes(Context& context, SemIRLocation loc, // 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); + auto import_identifier_id = + import_ir.sem_ir->identifiers().Lookup(identifier); if (!import_identifier_id.is_valid()) { // Name doesn't exist in the import IR. continue; @@ -312,7 +313,8 @@ static auto LookupInImportIRScopes(Context& context, SemIRLocation loc, } // Look up the name in the import scope. - const auto& import_scope = import_ir->name_scopes().Get(import_scope_id); + const auto& import_scope = + import_ir.sem_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. @@ -742,7 +744,7 @@ class TypeCompleter { auto BuildImportRefUsedValueRepr(SemIR::TypeId type_id, SemIR::ImportRefUsed import_ref) const -> SemIR::ValueRepr { - const auto& import_ir = context_.import_irs().Get(import_ref.ir_id); + const auto& import_ir = context_.import_irs().Get(import_ref.ir_id).sem_ir; auto import_inst = import_ir->insts().Get(import_ref.inst_id); CARBON_CHECK(import_inst.kind() != SemIR::InstKind::ImportRefUsed) << "If ImportRefUsed can point at another, this would be recursive."; diff --git a/toolchain/check/import.cpp b/toolchain/check/import.cpp index 1425e640436e..68ae266d76df 100644 --- a/toolchain/check/import.cpp +++ b/toolchain/check/import.cpp @@ -130,12 +130,11 @@ static auto CacheCopiedNamespace( // name conflicts, but that won't change the result because namespaces supersede // other names in conflicts. static auto CopySingleNameScopeFromImportIR( - Context& context, + Context& context, SemIR::TypeId namespace_type_id, llvm::DenseMap& copied_namespaces, SemIR::ImportIRId ir_id, SemIR::InstId import_inst_id, SemIR::NameScopeId import_scope_id, SemIR::NameScopeId enclosing_scope_id, - SemIR::NameId name_id, SemIR::TypeId namespace_type_id) - -> SemIR::NameScopeId { + SemIR::NameId name_id) -> SemIR::NameScopeId { // Produce the namespace for the entry. auto make_import_id = [&]() { return context.AddInst(SemIR::ImportRefUsed{.type_id = namespace_type_id, @@ -199,8 +198,8 @@ static auto CopyEnclosingNameScopesFromImportIR( auto name_id = CopyNameFromImportIR(context, import_sem_ir, import_scope.name_id); scope_cursor = CopySingleNameScopeFromImportIR( - context, copied_namespaces, ir_id, import_scope.inst_id, - import_scope_id, scope_cursor, name_id, namespace_type_id); + context, namespace_type_id, copied_namespaces, ir_id, + import_scope.inst_id, import_scope_id, scope_cursor, name_id); } return scope_cursor; @@ -208,8 +207,10 @@ static auto CopyEnclosingNameScopesFromImportIR( auto ImportLibraryFromCurrentPackage(Context& context, SemIR::TypeId namespace_type_id, + Parse::ImportDirectiveId node_id, const SemIR::File& import_sem_ir) -> void { - auto ir_id = context.import_irs().Add(&import_sem_ir); + auto ir_id = + context.import_irs().Add({.node_id = node_id, .sem_ir = &import_sem_ir}); context.import_ir_constant_values()[ir_id.index].Set( SemIR::InstId::PackageNamespace, context.constant_values().Get(SemIR::InstId::PackageNamespace)); @@ -236,9 +237,8 @@ auto ImportLibraryFromCurrentPackage(Context& context, // Namespaces are always imported because they're essential for // qualifiers, and the type is simple. CopySingleNameScopeFromImportIR( - context, copied_namespaces, ir_id, import_inst_id, - import_namespace_inst->name_scope_id, enclosing_scope_id, name_id, - namespace_type_id); + context, namespace_type_id, copied_namespaces, ir_id, import_inst_id, + import_namespace_inst->name_scope_id, enclosing_scope_id, name_id); } else { // Leave a placeholder that the inst comes from the other IR. auto target_id = context.AddImportRef(ir_id, import_inst_id); @@ -260,9 +260,9 @@ auto ImportLibrariesFromOtherPackage(Context& context, SemIR::TypeId namespace_type_id, Parse::ImportDirectiveId node_id, IdentifierId package_id, - llvm::ArrayRef sem_irs, + llvm::ArrayRef import_irs, bool has_load_error) -> void { - CARBON_CHECK(has_load_error || !sem_irs.empty()) + CARBON_CHECK(has_load_error || !import_irs.empty()) << "There should be either a load error or at least one IR."; auto name_id = SemIR::NameId::ForIdentifier(package_id); @@ -273,8 +273,8 @@ auto ImportLibrariesFromOtherPackage(Context& context, auto& scope = context.name_scopes().Get(namespace_scope_id); scope.is_closed_import = !is_duplicate; - for (const auto* sem_ir : sem_irs) { - auto ir_id = context.import_irs().Add(sem_ir); + for (auto import_ir : import_irs) { + auto ir_id = context.import_irs().Add(import_ir); scope.import_ir_scopes.push_back({ir_id, SemIR::NameScopeId::Package}); context.import_ir_constant_values()[ir_id.index].Set( SemIR::InstId::PackageNamespace, namespace_const_id); diff --git a/toolchain/check/import.h b/toolchain/check/import.h index 5f4849d92650..e680a3f31545 100644 --- a/toolchain/check/import.h +++ b/toolchain/check/import.h @@ -6,6 +6,7 @@ #define CARBON_TOOLCHAIN_CHECK_IMPORT_H_ #include "toolchain/check/context.h" +#include "toolchain/parse/node_ids.h" #include "toolchain/sem_ir/file.h" namespace Carbon::Check { @@ -15,6 +16,7 @@ namespace Carbon::Check { // though they are several layers deep. auto ImportLibraryFromCurrentPackage(Context& context, SemIR::TypeId namespace_type_id, + Parse::ImportDirectiveId node_id, const SemIR::File& import_sem_ir) -> void; // Adds another package's imports to name lookup, with all libraries together. @@ -22,13 +24,13 @@ auto ImportLibraryFromCurrentPackage(Context& context, // will resolve, and will provide a name scope that can be used for further // qualified name lookups. // -// sem_irs will all be non-null, and may be empty. has_load_error is used to -// indicate if any library in the package failed to import correctly. +// import_irs may be empty. has_load_error is used to indicate if any library in +// the package failed to import correctly. auto ImportLibrariesFromOtherPackage(Context& context, SemIR::TypeId namespace_type_id, Parse::ImportDirectiveId node_id, IdentifierId package_id, - llvm::ArrayRef sem_irs, + llvm::ArrayRef import_irs, bool has_load_error) -> void; } // namespace Carbon::Check diff --git a/toolchain/check/import_ref.cpp b/toolchain/check/import_ref.cpp index e77c4757f179..492bdff7f526 100644 --- a/toolchain/check/import_ref.cpp +++ b/toolchain/check/import_ref.cpp @@ -57,7 +57,7 @@ class ImportRefResolver { explicit ImportRefResolver(Context& context, SemIR::ImportIRId import_ir_id) : context_(context), import_ir_id_(import_ir_id), - import_ir_(*context_.import_irs().Get(import_ir_id)), + import_ir_(*context_.import_irs().Get(import_ir_id).sem_ir), import_ir_constant_values_( context_.import_ir_constant_values()[import_ir_id.index]) {} @@ -912,7 +912,8 @@ auto TryResolveImportRefUnused(Context& context, SemIR::InstId inst_id) return; } - const SemIR::File& import_ir = *context.import_irs().Get(import_ref->ir_id); + const SemIR::File& import_ir = + *context.import_irs().Get(import_ref->ir_id).sem_ir; auto import_inst = import_ir.insts().Get(import_ref->inst_id); ImportRefResolver resolver(context, import_ref->ir_id); diff --git a/toolchain/check/testdata/class/cross_package_import.carbon b/toolchain/check/testdata/class/cross_package_import.carbon index b8844eaaf66a..b185436797e1 100644 --- a/toolchain/check/testdata/class/cross_package_import.carbon +++ b/toolchain/check/testdata/class/cross_package_import.carbon @@ -45,17 +45,11 @@ library "extern" api; import Other library "extern"; -// CHECK:STDERR: fail_extern.carbon:[[@LINE+11]]:8: ERROR: Variable has incomplete type `C`. +// CHECK:STDERR: fail_extern.carbon:[[@LINE+5]]: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: -// 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 @@ -63,18 +57,24 @@ var c: Other.C = {}; library "fail_merge_define_extern" api; import Other library "define"; -import Other library "extern"; - -// CHECK:STDERR: fail_merge_define_extern.carbon:[[@LINE+10]]:8: In name lookup for `C`. -// CHECK:STDERR: var c: Other.C = {}; -// CHECK:STDERR: ^~~~~~~ -// CHECK:STDERR: -// CHECK:STDERR: other_conflict.carbon:4:1: ERROR: Duplicate name being declared in the same scope. -// CHECK:STDERR: fn C() {} +// CHECK:STDERR: fail_merge_define_extern.carbon:[[@LINE+12]]:1: In import. +// CHECK:STDERR: import Other library "extern"; +// CHECK:STDERR: ^~~~~~ +// CHECK:STDERR: other_extern.carbon:5:1: ERROR: Duplicate name being declared in the same scope. +// CHECK:STDERR: class C; // CHECK:STDERR: ^~~~~~~~ +// CHECK:STDERR: fail_merge_define_extern.carbon:[[@LINE-7]]:1: In import. +// CHECK:STDERR: import Other library "define"; +// CHECK:STDERR: ^~~~~~ // CHECK:STDERR: other_define.carbon:4:1: Name is previously declared here. // CHECK:STDERR: class C {} // CHECK:STDERR: ^~~~~~~~~ +import Other library "extern"; + +// CHECK:STDERR: fail_merge_define_extern.carbon:[[@LINE+4]]:8: In name lookup for `C`. +// CHECK:STDERR: var c: Other.C = {}; +// CHECK:STDERR: ^~~~~~~ +// CHECK:STDERR: var c: Other.C = {}; // --- fail_conflict.carbon @@ -82,6 +82,18 @@ var c: Other.C = {}; library "conflict" api; import Other library "define"; +// CHECK:STDERR: fail_conflict.carbon:[[@LINE+12]]:1: In import. +// CHECK:STDERR: import Other library "conflict"; +// 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: fail_conflict.carbon:[[@LINE-7]]:1: In import. +// CHECK:STDERR: import Other library "define"; +// CHECK:STDERR: ^~~~~~ +// CHECK:STDERR: other_define.carbon:4:1: Name is previously declared here. +// CHECK:STDERR: class C {} +// CHECK:STDERR: ^~~~~~~~~ import Other library "conflict"; // CHECK:STDERR: fail_conflict.carbon:[[@LINE+3]]:8: In name lookup for `C`. @@ -201,7 +213,7 @@ var c: Other.C = {}; // CHECK:STDOUT: // CHECK:STDOUT: fn @__global_init() { // CHECK:STDOUT: !entry: -// CHECK:STDOUT: %.loc17: {} = struct_literal () +// CHECK:STDOUT: %.loc11: {} = struct_literal () // CHECK:STDOUT: assign file.%c.var, // CHECK:STDOUT: return // CHECK:STDOUT: } @@ -243,10 +255,10 @@ var c: Other.C = {}; // CHECK:STDOUT: // CHECK:STDOUT: fn @__global_init() { // CHECK:STDOUT: !entry: -// CHECK:STDOUT: %.loc17_19.1: {} = struct_literal () -// CHECK:STDOUT: %.loc17_19.2: init C = class_init (), file.%c.var [template = constants.%.4] -// CHECK:STDOUT: %.loc17_19.3: init C = converted %.loc17_19.1, %.loc17_19.2 [template = constants.%.4] -// CHECK:STDOUT: assign file.%c.var, %.loc17_19.3 +// CHECK:STDOUT: %.loc23_19.1: {} = struct_literal () +// CHECK:STDOUT: %.loc23_19.2: init C = class_init (), file.%c.var [template = constants.%.4] +// CHECK:STDOUT: %.loc23_19.3: init C = converted %.loc23_19.1, %.loc23_19.2 [template = constants.%.4] +// CHECK:STDOUT: assign file.%c.var, %.loc23_19.3 // CHECK:STDOUT: return // CHECK:STDOUT: } // CHECK:STDOUT: @@ -285,10 +297,10 @@ var c: Other.C = {}; // 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: %.loc22_19.1: {} = struct_literal () +// CHECK:STDOUT: %.loc22_19.2: init C = class_init (), file.%c.var [template = constants.%.4] +// CHECK:STDOUT: %.loc22_19.3: init C = converted %.loc22_19.1, %.loc22_19.2 [template = constants.%.4] +// CHECK:STDOUT: assign file.%c.var, %.loc22_19.3 // CHECK:STDOUT: return // CHECK:STDOUT: } // CHECK:STDOUT: diff --git a/toolchain/check/testdata/class/fail_import_misuses.carbon b/toolchain/check/testdata/class/fail_import_misuses.carbon index 62dee713bced..2edb61eec4eb 100644 --- a/toolchain/check/testdata/class/fail_import_misuses.carbon +++ b/toolchain/check/testdata/class/fail_import_misuses.carbon @@ -19,9 +19,12 @@ library "b" api; import library "a"; -// CHECK:STDERR: fail_b.carbon:[[@LINE+7]]:1: ERROR: Duplicate name being declared in the same scope. +// CHECK:STDERR: fail_b.carbon:[[@LINE+10]]:1: ERROR: Duplicate name being declared in the same scope. // CHECK:STDERR: class Empty { // CHECK:STDERR: ^~~~~~~~~~~~~ +// CHECK:STDERR: fail_b.carbon:[[@LINE-5]]:1: In import. +// CHECK:STDERR: import library "a"; +// CHECK:STDERR: ^~~~~~ // CHECK:STDERR: a.carbon:4:1: Name is previously declared here. // CHECK:STDERR: class Empty { // CHECK:STDERR: ^~~~~~~~~~~~~ diff --git a/toolchain/check/testdata/class/fail_todo_import_forward_decl.carbon b/toolchain/check/testdata/class/fail_todo_import_forward_decl.carbon index 005a5863393c..f33152fdcc7f 100644 --- a/toolchain/check/testdata/class/fail_todo_import_forward_decl.carbon +++ b/toolchain/check/testdata/class/fail_todo_import_forward_decl.carbon @@ -17,9 +17,12 @@ library "b" api; import library "a"; // TODO: This should probably have a valid form. -// CHECK:STDERR: fail_b.carbon:[[@LINE+6]]:1: ERROR: Duplicate name being declared in the same scope. +// CHECK:STDERR: fail_b.carbon:[[@LINE+9]]:1: ERROR: Duplicate name being declared in the same scope. // CHECK:STDERR: class ForwardDecl { // CHECK:STDERR: ^~~~~~~~~~~~~~~~~~~ +// CHECK:STDERR: fail_b.carbon:[[@LINE-6]]:1: In import. +// CHECK:STDERR: import library "a"; +// CHECK:STDERR: ^~~~~~ // CHECK:STDERR: a.carbon:4:1: Name is previously declared here. // CHECK:STDERR: class ForwardDecl; // CHECK:STDERR: ^~~~~~~~~~~~~~~~~~ diff --git a/toolchain/check/testdata/function/definition/fail_todo_import_forward_decl.carbon b/toolchain/check/testdata/function/definition/fail_todo_import_forward_decl.carbon index 1d89ea3f3a03..06008179cc44 100644 --- a/toolchain/check/testdata/function/definition/fail_todo_import_forward_decl.carbon +++ b/toolchain/check/testdata/function/definition/fail_todo_import_forward_decl.carbon @@ -18,9 +18,12 @@ import library "a"; // TODO: This should be an error until we can be clear whether `a` or `b` should // own the definition, as there might be a separate definition in `a`'s impl. -// CHECK:STDERR: fail_b.carbon:[[@LINE+6]]:1: ERROR: Duplicate name being declared in the same scope. +// CHECK:STDERR: fail_b.carbon:[[@LINE+9]]:1: ERROR: Duplicate name being declared in the same scope. // CHECK:STDERR: fn Foo() {} // CHECK:STDERR: ^~~~~~~~~~ +// CHECK:STDERR: fail_b.carbon:[[@LINE-7]]:1: In import. +// CHECK:STDERR: import library "a"; +// CHECK:STDERR: ^~~~~~ // CHECK:STDERR: a.carbon:4:1: Name is previously declared here. // CHECK:STDERR: fn Foo(); // CHECK:STDERR: ^~~~~~~~~ @@ -44,7 +47,7 @@ fn Foo() {} // CHECK:STDOUT: .Foo = %import_ref // CHECK:STDOUT: } // CHECK:STDOUT: %import_ref: = import_ref ir1, inst+1, used [template = imports.%Foo] -// CHECK:STDOUT: %.loc14: = fn_decl @.1 [template] {} +// CHECK:STDOUT: %.loc17: = fn_decl @.1 [template] {} // CHECK:STDOUT: } // CHECK:STDOUT: // CHECK:STDOUT: fn @Foo(); diff --git a/toolchain/check/testdata/namespace/fail_conflict_imported_namespace_first.carbon b/toolchain/check/testdata/namespace/fail_conflict_imported_namespace_first.carbon index 9b91dd6ebbdf..86a481fa02ef 100644 --- a/toolchain/check/testdata/namespace/fail_conflict_imported_namespace_first.carbon +++ b/toolchain/check/testdata/namespace/fail_conflict_imported_namespace_first.carbon @@ -16,9 +16,12 @@ package Example api; import library "namespace"; -// CHECK:STDERR: fail_conflict.carbon:[[@LINE+6]]:1: ERROR: Duplicate name being declared in the same scope. +// CHECK:STDERR: fail_conflict.carbon:[[@LINE+9]]:1: ERROR: Duplicate name being declared in the same scope. // CHECK:STDERR: fn NS(); // CHECK:STDERR: ^~~~~~~~ +// CHECK:STDERR: fail_conflict.carbon:[[@LINE-5]]:1: In import. +// CHECK:STDERR: import library "namespace"; +// CHECK:STDERR: ^~~~~~ // CHECK:STDERR: namespace.carbon:4:1: Name is previously declared here. // CHECK:STDERR: namespace NS; // CHECK:STDERR: ^~~~~~~~~~~~~ @@ -45,7 +48,7 @@ fn NS.Foo(); // CHECK:STDOUT: %NS: = namespace %import_ref, [template] { // CHECK:STDOUT: .Foo = %Foo // CHECK:STDOUT: } -// CHECK:STDOUT: %.loc12: = fn_decl @.1 [template] {} +// CHECK:STDOUT: %.loc15: = fn_decl @.1 [template] {} // CHECK:STDOUT: %Foo: = fn_decl @Foo [template] {} // CHECK:STDOUT: } // CHECK:STDOUT: diff --git a/toolchain/check/testdata/namespace/fail_conflict_imported_namespace_second.carbon b/toolchain/check/testdata/namespace/fail_conflict_imported_namespace_second.carbon index 879b9e1d0dec..69a34bc18c04 100644 --- a/toolchain/check/testdata/namespace/fail_conflict_imported_namespace_second.carbon +++ b/toolchain/check/testdata/namespace/fail_conflict_imported_namespace_second.carbon @@ -16,9 +16,12 @@ package Example api; import library "fn"; -// CHECK:STDERR: fail_conflict.carbon:[[@LINE+7]]:1: ERROR: Duplicate name being declared in the same scope. +// CHECK:STDERR: fail_conflict.carbon:[[@LINE+10]]:1: ERROR: Duplicate name being declared in the same scope. // CHECK:STDERR: namespace NS; // CHECK:STDERR: ^~~~~~~~~~~~~ +// CHECK:STDERR: fail_conflict.carbon:[[@LINE-5]]:1: In import. +// CHECK:STDERR: import library "fn"; +// CHECK:STDERR: ^~~~~~ // CHECK:STDERR: fn.carbon:4:1: Name is previously declared here. // CHECK:STDERR: fn NS(); // CHECK:STDERR: ^~~~~~~~ @@ -51,8 +54,8 @@ fn NS.Foo(); // CHECK:STDOUT: .NS = %import_ref // CHECK:STDOUT: } // CHECK:STDOUT: %import_ref: = import_ref ir1, inst+1, used [template = imports.%NS] -// CHECK:STDOUT: %.loc13: = namespace [template] {} -// CHECK:STDOUT: %.loc21: = fn_decl @.1 [template] {} +// CHECK:STDOUT: %.loc16: = namespace [template] {} +// CHECK:STDOUT: %.loc24: = fn_decl @.1 [template] {} // CHECK:STDOUT: } // CHECK:STDOUT: // CHECK:STDOUT: fn @NS(); diff --git a/toolchain/check/testdata/namespace/fail_conflict_in_imports_namespace_first.carbon b/toolchain/check/testdata/namespace/fail_conflict_in_imports_namespace_first.carbon index d520e5be005d..9913595042dc 100644 --- a/toolchain/check/testdata/namespace/fail_conflict_in_imports_namespace_first.carbon +++ b/toolchain/check/testdata/namespace/fail_conflict_in_imports_namespace_first.carbon @@ -15,12 +15,6 @@ fn NS.Foo() {} package Example library "fn" api; -// CHECK:STDERR: fn.carbon:[[@LINE+6]]:1: ERROR: Duplicate name being declared in the same scope. -// CHECK:STDERR: fn NS() {} -// CHECK:STDERR: ^~~~~~~~~ -// CHECK:STDERR: namespace.carbon:4:1: Name is previously declared here. -// CHECK:STDERR: namespace NS; -// CHECK:STDERR: ^~~~~~~~~~~~~ fn NS() {} // --- fail_conflict.carbon @@ -28,6 +22,18 @@ fn NS() {} package Example library "namespace_then_fn" api; import library "namespace"; +// CHECK:STDERR: fail_conflict.carbon:[[@LINE+12]]:1: In import. +// CHECK:STDERR: import library "fn"; +// CHECK:STDERR: ^~~~~~ +// CHECK:STDERR: fn.carbon:4:1: ERROR: Duplicate name being declared in the same scope. +// CHECK:STDERR: fn NS() {} +// CHECK:STDERR: ^~~~~~~~~ +// CHECK:STDERR: fail_conflict.carbon:[[@LINE-7]]:1: In import. +// CHECK:STDERR: import library "namespace"; +// CHECK:STDERR: ^~~~~~ +// CHECK:STDERR: namespace.carbon:4:1: Name is previously declared here. +// CHECK:STDERR: namespace NS; +// CHECK:STDERR: ^~~~~~~~~~~~~ import library "fn"; fn NS.Bar() {} diff --git a/toolchain/check/testdata/namespace/fail_conflict_in_imports_namespace_second.carbon b/toolchain/check/testdata/namespace/fail_conflict_in_imports_namespace_second.carbon index c43c641f1cef..0b013c38dcdb 100644 --- a/toolchain/check/testdata/namespace/fail_conflict_in_imports_namespace_second.carbon +++ b/toolchain/check/testdata/namespace/fail_conflict_in_imports_namespace_second.carbon @@ -14,12 +14,6 @@ fn NS() {} package Example library "namespace" api; -// CHECK:STDERR: namespace.carbon:[[@LINE+6]]:1: ERROR: Duplicate name being declared in the same scope. -// CHECK:STDERR: namespace NS; -// CHECK:STDERR: ^~~~~~~~~~~~~ -// CHECK:STDERR: fn.carbon:4:1: Name is previously declared here. -// CHECK:STDERR: fn NS() {} -// CHECK:STDERR: ^~~~~~~~~ namespace NS; fn NS.Foo() {} @@ -28,6 +22,18 @@ fn NS.Foo() {} package Example library "fn_then_namespace" api; import library "fn"; +// CHECK:STDERR: fail_conflict.carbon:[[@LINE+12]]:1: In import. +// CHECK:STDERR: import library "namespace"; +// CHECK:STDERR: ^~~~~~ +// CHECK:STDERR: namespace.carbon:4:1: ERROR: Duplicate name being declared in the same scope. +// CHECK:STDERR: namespace NS; +// CHECK:STDERR: ^~~~~~~~~~~~~ +// CHECK:STDERR: fail_conflict.carbon:[[@LINE-7]]:1: In import. +// CHECK:STDERR: import library "fn"; +// CHECK:STDERR: ^~~~~~ +// CHECK:STDERR: fn.carbon:4:1: Name is previously declared here. +// CHECK:STDERR: fn NS() {} +// CHECK:STDERR: ^~~~~~~~~ import library "namespace"; fn NS.Bar() {} diff --git a/toolchain/check/testdata/packages/cross_package_import.carbon b/toolchain/check/testdata/packages/cross_package_import.carbon index f36bda608968..1d9e28be561a 100644 --- a/toolchain/check/testdata/packages/cross_package_import.carbon +++ b/toolchain/check/testdata/packages/cross_package_import.carbon @@ -19,12 +19,6 @@ fn F() {} 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 @@ -66,19 +60,25 @@ fn Run() { library "use_other_extern" api; import Other library "fn"; +// CHECK:STDERR: fail_main_use_other_extern.carbon:[[@LINE+12]]:1: In import. +// CHECK:STDERR: import Other library "fn_extern"; +// CHECK:STDERR: ^~~~~~ +// CHECK:STDERR: other_fn_extern.carbon:5:1: ERROR: Duplicate name being declared in the same scope. +// CHECK:STDERR: fn F(); +// CHECK:STDERR: ^~~~~~~ +// CHECK:STDERR: fail_main_use_other_extern.carbon:[[@LINE-7]]:1: In import. +// CHECK:STDERR: import Other library "fn"; +// CHECK:STDERR: ^~~~~~ +// CHECK:STDERR: other_fn.carbon:4:1: Name is previously declared here. +// CHECK:STDERR: fn F() {} +// CHECK:STDERR: ^~~~~~~~ import Other library "fn_extern"; fn Run() { - // CHECK:STDERR: fail_main_use_other_extern.carbon:[[@LINE+10]]:3: In name lookup for `F`. + // CHECK:STDERR: fail_main_use_other_extern.carbon:[[@LINE+4]]:3: In name lookup for `F`. // CHECK:STDERR: Other.F(); // CHECK:STDERR: ^~~~~~~ // 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(); } @@ -94,6 +94,18 @@ import Other library "fn_conflict"; library "use_other_ambiguous" api; import Other library "fn"; +// CHECK:STDERR: fail_main_use_other_ambiguous.carbon:[[@LINE+12]]:1: In import. +// CHECK:STDERR: import Other library "fn_conflict"; +// 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: fail_main_use_other_ambiguous.carbon:[[@LINE-7]]:1: In import. +// CHECK:STDERR: import Other library "fn"; +// CHECK:STDERR: ^~~~~~ +// CHECK:STDERR: other_fn.carbon:4:1: Name is previously declared here. +// CHECK:STDERR: fn F() {} +// CHECK:STDERR: ^~~~~~~~ import Other library "fn_conflict"; fn Run() { @@ -109,18 +121,24 @@ fn Run() { library "namespace_conflict" api; import library "other_ns"; -// CHECK:STDERR: fail_main_namespace_conflict.carbon:[[@LINE+7]]:1: ERROR: Duplicate name being declared in the same scope. +// CHECK:STDERR: fail_main_namespace_conflict.carbon:[[@LINE+10]]:1: ERROR: Duplicate name being declared in the same scope. // CHECK:STDERR: import Other library "fn"; // CHECK:STDERR: ^~~~~~ +// CHECK:STDERR: fail_main_namespace_conflict.carbon:[[@LINE-4]]:1: In import. +// CHECK:STDERR: import library "other_ns"; +// CHECK:STDERR: ^~~~~~ // CHECK:STDERR: main_other_ns.carbon:4:1: Name is previously declared here. // CHECK:STDERR: namespace Other; // CHECK:STDERR: ^~~~~~~~~~~~~~~~ // CHECK:STDERR: import Other library "fn"; -// CHECK:STDERR: fail_main_namespace_conflict.carbon:[[@LINE+7]]:1: ERROR: Duplicate name being declared in the same scope. +// CHECK:STDERR: fail_main_namespace_conflict.carbon:[[@LINE+10]]:1: ERROR: Duplicate name being declared in the same scope. // CHECK:STDERR: fn Other.F() {} // CHECK:STDERR: ^~~~~~~~~~~~~~ +// CHECK:STDERR: fail_main_namespace_conflict.carbon:[[@LINE-5]]:1: In import. +// CHECK:STDERR: import Other library "fn"; +// CHECK:STDERR: ^~~~~~ // CHECK:STDERR: other_fn.carbon:4:1: Name is previously declared here. // CHECK:STDERR: fn F() {} // CHECK:STDERR: ^~~~~~~~ @@ -277,7 +295,7 @@ fn Other.G() {} // CHECK:STDOUT: !entry: // CHECK:STDOUT: %Other.ref: = name_ref Other, file.%Other [template = file.%Other] // CHECK:STDOUT: %F.ref: = name_ref F, file.%import_ref.1 [template = imports.%F.1] -// CHECK:STDOUT: %.loc18: init () = call %F.ref() +// CHECK:STDOUT: %.loc24: init () = call %F.ref() // CHECK:STDOUT: return // CHECK:STDOUT: } // CHECK:STDOUT: @@ -315,7 +333,7 @@ fn Other.G() {} // CHECK:STDOUT: !entry: // CHECK:STDOUT: %Other.ref: = name_ref Other, file.%Other [template = file.%Other] // CHECK:STDOUT: %F.ref: = name_ref F, file.%import_ref.1 [template = imports.%F.1] -// CHECK:STDOUT: %.loc12: init () = call %F.ref() +// CHECK:STDOUT: %.loc24: init () = call %F.ref() // CHECK:STDOUT: return // CHECK:STDOUT: } // CHECK:STDOUT: @@ -332,7 +350,7 @@ fn Other.G() {} // CHECK:STDOUT: %import_ref.1: = import_ref ir1, inst+1, used // CHECK:STDOUT: %Other: = namespace %import_ref.1, [template] {} // CHECK:STDOUT: %import_ref.2: = import_ref ir2, inst+1, used [template = imports.%F] -// CHECK:STDOUT: %.loc21: = fn_decl @.1 [template] {} +// CHECK:STDOUT: %.loc27: = fn_decl @.1 [template] {} // CHECK:STDOUT: } // CHECK:STDOUT: // CHECK:STDOUT: fn @F(); diff --git a/toolchain/check/testdata/packages/fail_conflict_no_namespaces.carbon b/toolchain/check/testdata/packages/fail_conflict_no_namespaces.carbon index feb2b8aec7b2..ad3927d9f082 100644 --- a/toolchain/check/testdata/packages/fail_conflict_no_namespaces.carbon +++ b/toolchain/check/testdata/packages/fail_conflict_no_namespaces.carbon @@ -14,12 +14,6 @@ fn Foo(); package Example library "var" api; -// CHECK:STDERR: var.carbon:[[@LINE+6]]:5: ERROR: Duplicate name being declared in the same scope. -// CHECK:STDERR: var Foo: i32; -// CHECK:STDERR: ^~~ -// CHECK:STDERR: fn.carbon:4:1: Name is previously declared here. -// CHECK:STDERR: fn Foo(); -// CHECK:STDERR: ^~~~~~~~~ var Foo: i32; // --- fail_conflict.carbon @@ -27,6 +21,18 @@ var Foo: i32; package Example library "conflict" api; import library "fn"; +// CHECK:STDERR: fail_conflict.carbon:[[@LINE+12]]:1: In import. +// CHECK:STDERR: import library "var"; +// CHECK:STDERR: ^~~~~~ +// CHECK:STDERR: var.carbon:4:5: ERROR: Duplicate name being declared in the same scope. +// CHECK:STDERR: var Foo: i32; +// CHECK:STDERR: ^~~ +// CHECK:STDERR: fail_conflict.carbon:[[@LINE-7]]:1: In import. +// CHECK:STDERR: import library "fn"; +// CHECK:STDERR: ^~~~~~ +// CHECK:STDERR: fn.carbon:4:1: Name is previously declared here. +// CHECK:STDERR: fn Foo(); +// CHECK:STDERR: ^~~~~~~~~ import library "var"; // CHECK:STDOUT: --- fn.carbon diff --git a/toolchain/diagnostics/diagnostic_converter.h b/toolchain/diagnostics/diagnostic_converter.h index ebc28e033e3f..e043ce56f9d3 100644 --- a/toolchain/diagnostics/diagnostic_converter.h +++ b/toolchain/diagnostics/diagnostic_converter.h @@ -15,9 +15,18 @@ namespace Carbon { template class DiagnosticConverter { public: + // Callback type used to report context messages from ConvertLocation. + // Note that the first parameter type is DiagnosticLocation rather than + // LocationT, because ConvertLocation must not recurse. + using ContextFnT = llvm::function_ref&)>; + virtual ~DiagnosticConverter() = default; - virtual auto ConvertLocation(LocationT loc) const -> DiagnosticLocation = 0; + // Converts a LocationT to a DiagnosticLocation. ConvertLocation may invoke + // context_fn to provide context messages. + virtual auto ConvertLocation(LocationT loc, ContextFnT context_fn) const + -> DiagnosticLocation = 0; // Converts arg types as needed. Not all uses require conversion, so the // default returns the argument unchanged. diff --git a/toolchain/diagnostics/diagnostic_emitter.h b/toolchain/diagnostics/diagnostic_emitter.h index c27ff91ad024..6ca20c3761b1 100644 --- a/toolchain/diagnostics/diagnostic_emitter.h +++ b/toolchain/diagnostics/diagnostic_emitter.h @@ -65,8 +65,7 @@ class DiagnosticEmitter { Internal::NoTypeDeduction... args) -> DiagnosticBuilder& { CARBON_CHECK(diagnostic_base.Level == DiagnosticLevel::Note) << static_cast(diagnostic_base.Level); - AddMessage(emitter_, location, diagnostic_base, - {emitter_->MakeAny(args)...}); + AddMessage(location, diagnostic_base, {emitter_->MakeAny(args)...}); return *this; } @@ -89,18 +88,39 @@ class DiagnosticEmitter { const Internal::DiagnosticBase& diagnostic_base, llvm::SmallVector args) : emitter_(emitter), diagnostic_({.level = diagnostic_base.Level}) { - AddMessage(emitter, location, diagnostic_base, std::move(args)); + AddMessage(location, diagnostic_base, std::move(args)); CARBON_CHECK(diagnostic_base.Level != DiagnosticLevel::Note); } + // Adds a message to the diagnostic, handling conversion of the location and + // arguments. template - auto AddMessage(DiagnosticEmitter* emitter, LocationT location, + auto AddMessage(LocationT location, const Internal::DiagnosticBase& diagnostic_base, llvm::SmallVector args) -> void { + AddMessageWithDiagnosticLocation( + emitter_->converter_->ConvertLocation( + location, + [&](DiagnosticLocation location, + const Internal::DiagnosticBase<>& diagnostic_base) { + AddMessageWithDiagnosticLocation(location, diagnostic_base, + args); + }), + diagnostic_base, args); + } + + // Adds a message to the diagnostic, handling conversion of the arguments. A + // DiagnosticLocation must be provided instead of a LocationT in order to + // avoid potential recursion. + template + auto AddMessageWithDiagnosticLocation( + DiagnosticLocation location, + const Internal::DiagnosticBase& diagnostic_base, + llvm::SmallVector args) { diagnostic_.messages.emplace_back(DiagnosticMessage{ .kind = diagnostic_base.Kind, .level = diagnostic_base.Level, - .location = emitter->converter_->ConvertLocation(location), + .location = location, .format = diagnostic_base.Format, .format_args = std::move(args), .format_fn = [](const DiagnosticMessage& message) -> std::string { diff --git a/toolchain/diagnostics/diagnostic_emitter_test.cpp b/toolchain/diagnostics/diagnostic_emitter_test.cpp index c689c62e157e..b68fdb0970fe 100644 --- a/toolchain/diagnostics/diagnostic_emitter_test.cpp +++ b/toolchain/diagnostics/diagnostic_emitter_test.cpp @@ -18,7 +18,8 @@ using ::Carbon::Testing::IsSingleDiagnostic; using testing::ElementsAre; struct FakeDiagnosticConverter : DiagnosticConverter { - auto ConvertLocation(int n) const -> DiagnosticLocation override { + auto ConvertLocation(int n, ContextFnT /*context_fn*/) const + -> DiagnosticLocation override { return {.line_number = 1, .column_number = n}; } }; diff --git a/toolchain/diagnostics/diagnostic_kind.def b/toolchain/diagnostics/diagnostic_kind.def index 00823cb05966..728a77d7f8d4 100644 --- a/toolchain/diagnostics/diagnostic_kind.def +++ b/toolchain/diagnostics/diagnostic_kind.def @@ -127,6 +127,9 @@ CARBON_DIAGNOSTIC_KIND(ExpectedAliasInitializer) CARBON_DIAGNOSTIC_KIND(SemanticsTodo) +// Location context. +CARBON_DIAGNOSTIC_KIND(InImport) + // Package/import checking diagnostics. CARBON_DIAGNOSTIC_KIND(IncorrectExtension) CARBON_DIAGNOSTIC_KIND(IncorrectExtensionImplNote) diff --git a/toolchain/diagnostics/null_diagnostics.h b/toolchain/diagnostics/null_diagnostics.h index c36dcd7cf3bc..5e9c6f8e3f45 100644 --- a/toolchain/diagnostics/null_diagnostics.h +++ b/toolchain/diagnostics/null_diagnostics.h @@ -11,8 +11,10 @@ namespace Carbon { template inline auto NullDiagnosticConverter() -> DiagnosticConverter& { - struct Converter : DiagnosticConverter { - auto ConvertLocation(LocationT /*loc*/) const + struct Converter : public DiagnosticConverter { + auto ConvertLocation( + LocationT /*loc*/, + DiagnosticConverter::ContextFnT /*context_fn*/) const -> DiagnosticLocation override { return {}; } diff --git a/toolchain/diagnostics/sorting_diagnostic_consumer_test.cpp b/toolchain/diagnostics/sorting_diagnostic_consumer_test.cpp index ef237e3d4ba0..ba3337145c04 100644 --- a/toolchain/diagnostics/sorting_diagnostic_consumer_test.cpp +++ b/toolchain/diagnostics/sorting_diagnostic_consumer_test.cpp @@ -20,7 +20,7 @@ using ::testing::InSequence; CARBON_DIAGNOSTIC(TestDiagnostic, Error, "{0}", llvm::StringLiteral); struct FakeDiagnosticConverter : DiagnosticConverter { - auto ConvertLocation(DiagnosticLocation loc) const + auto ConvertLocation(DiagnosticLocation loc, ContextFnT /*context_fn*/) const -> DiagnosticLocation override { return loc; } diff --git a/toolchain/lex/test_helpers.h b/toolchain/lex/test_helpers.h index 7d5d6418e3e2..2510a90ad7bd 100644 --- a/toolchain/lex/test_helpers.h +++ b/toolchain/lex/test_helpers.h @@ -24,7 +24,8 @@ class SingleTokenDiagnosticConverter : public DiagnosticConverter { explicit SingleTokenDiagnosticConverter(llvm::StringRef token) : token_(token) {} - auto ConvertLocation(const char* pos) const -> DiagnosticLocation override { + auto ConvertLocation(const char* pos, ContextFnT /*context_fn*/) const + -> DiagnosticLocation override { CARBON_CHECK(StringRefContainsPointer(token_, pos)) << "invalid diagnostic location"; llvm::StringRef prefix = token_.take_front(pos - token_.begin()); diff --git a/toolchain/lex/tokenized_buffer.cpp b/toolchain/lex/tokenized_buffer.cpp index 2af12fb3478d..65984170d463 100644 --- a/toolchain/lex/tokenized_buffer.cpp +++ b/toolchain/lex/tokenized_buffer.cpp @@ -346,7 +346,7 @@ auto TokenIterator::Print(llvm::raw_ostream& output) const -> void { } auto TokenizedBuffer::SourceBufferDiagnosticConverter::ConvertLocation( - const char* loc) const -> DiagnosticLocation { + const char* loc, ContextFnT /*context_fn*/) const -> DiagnosticLocation { CARBON_CHECK(StringRefContainsPointer(buffer_->source_->text(), loc)) << "location not within buffer"; int64_t offset = loc - buffer_->source_->text().begin(); @@ -390,7 +390,8 @@ auto TokenizedBuffer::SourceBufferDiagnosticConverter::ConvertLocation( .column_number = column_number + 1}; } -auto TokenDiagnosticConverter::ConvertLocation(TokenIndex token) const +auto TokenDiagnosticConverter::ConvertLocation(TokenIndex token, + ContextFnT context_fn) const -> DiagnosticLocation { // Map the token location into a position within the source buffer. const auto& token_info = buffer_->GetTokenInfo(token); @@ -403,7 +404,7 @@ auto TokenDiagnosticConverter::ConvertLocation(TokenIndex token) const // is a recovery token that doesn't correspond to the original source? DiagnosticLocation loc = TokenizedBuffer::SourceBufferDiagnosticConverter(buffer_).ConvertLocation( - token_start); + token_start, context_fn); loc.length = buffer_->GetTokenText(token).size(); return loc; } diff --git a/toolchain/lex/tokenized_buffer.h b/toolchain/lex/tokenized_buffer.h index b5bafde87cd0..af30bc8d39f5 100644 --- a/toolchain/lex/tokenized_buffer.h +++ b/toolchain/lex/tokenized_buffer.h @@ -119,7 +119,8 @@ class TokenDiagnosticConverter : public DiagnosticConverter { : buffer_(buffer) {} // Map the given token into a diagnostic location. - auto ConvertLocation(TokenIndex token) const -> DiagnosticLocation override; + auto ConvertLocation(TokenIndex token, ContextFnT context_fn) const + -> DiagnosticLocation override; private: const TokenizedBuffer* buffer_; @@ -255,7 +256,8 @@ class TokenizedBuffer : public Printable { // Map the given position within the source buffer into a diagnostic // location. - auto ConvertLocation(const char* loc) const -> DiagnosticLocation override; + auto ConvertLocation(const char* loc, ContextFnT context_fn) const + -> DiagnosticLocation override; private: const TokenizedBuffer* buffer_; diff --git a/toolchain/parse/tree_node_diagnostic_converter.h b/toolchain/parse/tree_node_diagnostic_converter.h index a6b13019981e..546f3a4a487a 100644 --- a/toolchain/parse/tree_node_diagnostic_converter.h +++ b/toolchain/parse/tree_node_diagnostic_converter.h @@ -44,7 +44,7 @@ class NodeLocationConverter : public DiagnosticConverter { parse_tree_(parse_tree) {} // Map the given token into a diagnostic location. - auto ConvertLocation(NodeLocation node_location) const + auto ConvertLocation(NodeLocation node_location, ContextFnT context_fn) const -> DiagnosticLocation override { // Support the invalid token as a way to emit only the filename, when there // is no line association. @@ -54,7 +54,7 @@ class NodeLocationConverter : public DiagnosticConverter { if (node_location.token_only()) { return token_converter_.ConvertLocation( - parse_tree_->node_token(node_location.node_id())); + parse_tree_->node_token(node_location.node_id()), context_fn); } // Construct a location that encompasses all tokens that descend from this @@ -74,11 +74,12 @@ class NodeLocationConverter : public DiagnosticConverter { } } DiagnosticLocation start_loc = - token_converter_.ConvertLocation(start_token); + token_converter_.ConvertLocation(start_token, context_fn); if (start_token == end_token) { return start_loc; } - DiagnosticLocation end_loc = token_converter_.ConvertLocation(end_token); + DiagnosticLocation end_loc = + token_converter_.ConvertLocation(end_token, context_fn); // For multiline locations we simply return the rest of the line for now // since true multiline locations are not yet supported. if (start_loc.line_number != end_loc.line_number) { diff --git a/toolchain/sem_ir/file.cpp b/toolchain/sem_ir/file.cpp index 1b1ac0d78b37..549337aec3f5 100644 --- a/toolchain/sem_ir/file.cpp +++ b/toolchain/sem_ir/file.cpp @@ -9,6 +9,7 @@ #include "llvm/ADT/SmallVector.h" #include "toolchain/base/value_store.h" #include "toolchain/base/yaml.h" +#include "toolchain/parse/node_ids.h" #include "toolchain/sem_ir/builtin_kind.h" #include "toolchain/sem_ir/ids.h" #include "toolchain/sem_ir/inst.h" @@ -69,7 +70,8 @@ File::File(SharedValueStores& value_stores, std::string filename, inst_blocks_(allocator_), constants_(*this, allocator_) { CARBON_CHECK(builtins != nullptr); - auto builtins_id = import_irs_.Add(builtins); + auto builtins_id = + import_irs_.Add({.node_id = Parse::NodeId::Invalid, .sem_ir = builtins}); CARBON_CHECK(builtins_id == ImportIRId::Builtins) << "Builtins must be the first IR"; @@ -105,8 +107,8 @@ File::File(SharedValueStores& value_stores, std::string filename, for (auto [i, inst] : llvm::enumerate(builtins->insts_.array_ref())) { // We can reuse the type_id from the builtin IR's inst because they're // special-cased values. - insts_.AddInNoBlock({ImportRefUsed{ - inst.type_id(), ImportIRId::Builtins, SemIR::InstId(i)}}); + insts_.AddInNoBlock(ImportRefUsed{ + inst.type_id(), ImportIRId::Builtins, SemIR::InstId(i)}); } }) {} @@ -374,8 +376,9 @@ static auto StringifyTypeExprImpl(const SemIR::File& outer_sem_ir, } case ImportRefUsed::Kind: { auto import_ref = inst.As(); - steps.push_back({.sem_ir = *sem_ir.import_irs().Get(import_ref.ir_id), - .inst_id = import_ref.inst_id}); + steps.push_back( + {.sem_ir = *sem_ir.import_irs().Get(import_ref.ir_id).sem_ir, + .inst_id = import_ref.inst_id}); break; } case InterfaceType::Kind: { @@ -556,7 +559,7 @@ auto GetExprCategory(const File& file, InstId inst_id) -> ExprCategory { case ImportRefUsed::Kind: { auto import_ref = inst.As(); - ir = ir->import_irs().Get(import_ref.ir_id); + ir = ir->import_irs().Get(import_ref.ir_id).sem_ir; inst_id = import_ref.inst_id; continue; } diff --git a/toolchain/sem_ir/file.h b/toolchain/sem_ir/file.h index 21da089b019e..038d2d5abd2f 100644 --- a/toolchain/sem_ir/file.h +++ b/toolchain/sem_ir/file.h @@ -38,6 +38,17 @@ struct BindNameInfo : public Printable { NameScopeId enclosing_scope_id; }; +class File; + +struct ImportIR : public Printable { + auto Print(llvm::raw_ostream& out) const -> void { out << node_id; } + + // The node ID for the import. + Parse::ImportDirectiveId node_id; + // The imported IR. + const File* sem_ir; +}; + // Provides semantic analysis on a Parse::Tree. class File : public Printable { public: diff --git a/toolchain/sem_ir/ids.h b/toolchain/sem_ir/ids.h index 1079e5e8157e..9bb6577165bc 100644 --- a/toolchain/sem_ir/ids.h +++ b/toolchain/sem_ir/ids.h @@ -20,6 +20,7 @@ class Inst; struct BindNameInfo; struct Class; struct Function; +struct ImportIR; struct Interface; struct Impl; struct NameScope; @@ -248,7 +249,7 @@ constexpr ImplId ImplId::Invalid = ImplId(InvalidIndex); // The ID of an imported IR. struct ImportIRId : public IdBase, public Printable { - using ValueType = const File*; + using ValueType = ImportIR; static const ImportIRId Builtins; using IdBase::IdBase; diff --git a/toolchain/source/source_buffer.cpp b/toolchain/source/source_buffer.cpp index c2887663c697..09248e2be823 100644 --- a/toolchain/source/source_buffer.cpp +++ b/toolchain/source/source_buffer.cpp @@ -12,7 +12,8 @@ namespace Carbon { namespace { struct FilenameConverter : DiagnosticConverter { - auto ConvertLocation(llvm::StringRef filename) const + auto ConvertLocation(llvm::StringRef filename, + ContextFnT /*context_fn*/) const -> DiagnosticLocation override { return {.filename = filename}; }