diff --git a/toolchain/check/check.cpp b/toolchain/check/check.cpp index c51db1fbce86..81f22bec1dba 100644 --- a/toolchain/check/check.cpp +++ b/toolchain/check/check.cpp @@ -114,12 +114,13 @@ struct UnitInfo { struct PackageImports { // Use the constructor so that the SmallVector is only constructed // as-needed. - explicit PackageImports(Parse::NodeId node) : node(node) {} + explicit PackageImports(Parse::ImportDirectiveId node_id) + : node_id(node_id) {} // The first `import` directive in the file, which declared the package's // identifier (even if the import failed). Used for associating diagnostics // not specific to a single import. - Parse::NodeId node; + Parse::ImportDirectiveId node_id; // Whether there's an import that failed to load. bool has_load_error = false; // The list of valid imports. @@ -181,10 +182,11 @@ static auto InitPackageScopeAndImports(Context& context, UnitInfo& unit_info) bool error_in_import = self_import->second.has_load_error; for (const auto& import : self_import->second.imports) { const auto& import_sem_ir = **import.unit_info->unit->sem_ir; - Import(context, namespace_type_id, import_sem_ir); - error_in_import = error_in_import || import_sem_ir.name_scopes() - .Get(SemIR::NameScopeId::Package) - .has_error; + ImportLibraryFromCurrentPackage(context, namespace_type_id, + import_sem_ir); + error_in_import |= import_sem_ir.name_scopes() + .Get(SemIR::NameScopeId::Package) + .has_error; } // If an import of the current package caused an error for the imported @@ -210,8 +212,9 @@ static auto InitPackageScopeAndImports(Context& context, UnitInfo& unit_info) for (auto import : package_imports.imports) { sem_irs.push_back(&**import.unit_info->unit->sem_ir); } - context.AddPackageImports(package_imports.node, package_id, sem_irs, - package_imports.has_load_error); + ImportLibrariesFromOtherPackage(context, namespace_type_id, + package_imports.node_id, package_id, + sem_irs, package_imports.has_load_error); } context.import_ir_constant_values().resize( @@ -338,12 +341,12 @@ static auto TrackImport( if (explicit_import_map) { // Diagnose redundant imports. if (auto [insert_it, success] = - explicit_import_map->insert({import_key, import.node}); + explicit_import_map->insert({import_key, import.node_id}); !success) { CARBON_DIAGNOSTIC(RepeatedImport, Error, "Library imported more than once."); CARBON_DIAGNOSTIC(FirstImported, Note, "First import here."); - unit_info.emitter.Build(import.node, RepeatedImport) + unit_info.emitter.Build(import.node_id, RepeatedImport) .Note(insert_it->second, FirstImported) .Emit(); return; @@ -377,7 +380,7 @@ static auto TrackImport( CARBON_DIAGNOSTIC(ImportSelf, Error, "File cannot import itself."); bool is_impl = !packaging || packaging->api_or_impl == Parse::Tree::ApiOrImpl::Impl; - unit_info.emitter.Emit(import.node, + unit_info.emitter.Emit(import.node_id, is_impl ? ExplicitImportApi : ImportSelf); return; } @@ -388,7 +391,7 @@ static auto TrackImport( is_import_default_library) { CARBON_DIAGNOSTIC(ImportMainDefaultLibrary, Error, "Cannot import `Main//default`."); - unit_info.emitter.Emit(import.node, ImportMainDefaultLibrary); + unit_info.emitter.Emit(import.node_id, ImportMainDefaultLibrary); return; } @@ -400,7 +403,7 @@ static auto TrackImport( CARBON_DIAGNOSTIC( ImportCurrentPackageByName, Error, "Imports from the current package must omit the package name."); - unit_info.emitter.Emit(import.node, ImportCurrentPackageByName); + unit_info.emitter.Emit(import.node_id, ImportCurrentPackageByName); return; } @@ -408,7 +411,7 @@ static auto TrackImport( if (is_explicit_main) { CARBON_DIAGNOSTIC(ImportMainPackage, Error, "Cannot import `Main` from other packages."); - unit_info.emitter.Emit(import.node, ImportMainPackage); + unit_info.emitter.Emit(import.node_id, ImportMainPackage); return; } } @@ -420,9 +423,9 @@ static auto TrackImport( } // Get the package imports. - auto package_imports_it = - unit_info.package_imports_map.try_emplace(import.package_id, import.node) - .first; + auto package_imports_it = unit_info.package_imports_map + .try_emplace(import.package_id, import.node_id) + .first; if (auto api = api_map.find(import_key); api != api_map.end()) { // Add references between the file and imported api. @@ -435,8 +438,9 @@ static auto TrackImport( CARBON_DIAGNOSTIC(LibraryApiNotFound, Error, "Corresponding API not found."); CARBON_DIAGNOSTIC(ImportNotFound, Error, "Imported API not found."); - unit_info.emitter.Emit( - import.node, explicit_import_map ? ImportNotFound : LibraryApiNotFound); + unit_info.emitter.Emit(import.node_id, explicit_import_map + ? ImportNotFound + : LibraryApiNotFound); } } @@ -463,9 +467,9 @@ static auto BuildApiMapAndDiagnosePackaging( "`Main//default` must omit `package` directive."); CARBON_DIAGNOSTIC(ExplicitMainLibrary, Error, "Use `library` directive in `Main` package libraries."); - unit_info.emitter.Emit(packaging->names.node, import_key.second.empty() - ? ExplicitMainPackage - : ExplicitMainLibrary); + unit_info.emitter.Emit(packaging->names.node_id, + import_key.second.empty() ? ExplicitMainPackage + : ExplicitMainLibrary); continue; } @@ -485,7 +489,7 @@ static auto BuildApiMapAndDiagnosePackaging( CARBON_DIAGNOSTIC(DuplicateLibraryApi, Error, "Library's API previously provided by `{0}`.", std::string); - unit_info.emitter.Emit(packaging->names.node, DuplicateLibraryApi, + unit_info.emitter.Emit(packaging->names.node_id, DuplicateLibraryApi, prev_filename.str()); } else { CARBON_DIAGNOSTIC(DuplicateMainApi, Error, @@ -512,7 +516,7 @@ static auto BuildApiMapAndDiagnosePackaging( "File extension of `{0}` required for `{1}`.", llvm::StringLiteral, Lex::TokenKind); auto diag = unit_info.emitter.Build( - packaging ? packaging->names.node : Parse::NodeId::Invalid, + packaging ? packaging->names.node_id : Parse::NodeId::Invalid, IncorrectExtension, want_ext, is_impl ? Lex::TokenKind::Impl : Lex::TokenKind::Api); if (is_api_with_impl_ext) { @@ -608,7 +612,7 @@ auto CheckParseTrees(const SemIR::File& builtin_ir, CARBON_DIAGNOSTIC(ImportCycleDetected, Error, "Import cannot be used due to a cycle. Cycle " "must be fixed to import."); - unit_info.emitter.Emit(import_it->names.node, + unit_info.emitter.Emit(import_it->names.node_id, ImportCycleDetected); // Make this look the same as an import which wasn't found. package_imports.has_load_error = true; diff --git a/toolchain/check/context.cpp b/toolchain/check/context.cpp index 7a9c85d1e051..0dabbb9cf3ed 100644 --- a/toolchain/check/context.cpp +++ b/toolchain/check/context.cpp @@ -133,14 +133,14 @@ auto Context::ReplaceInstBeforeConstantUse( constant_values().Set(inst_id, const_id); } -auto Context::DiagnoseDuplicateName(SemIR::InstId dup_def_id, - SemIR::InstId prev_def_id) -> void { +auto Context::DiagnoseDuplicateName(SemIRLocation dup_def, + SemIRLocation prev_def) -> void { CARBON_DIAGNOSTIC(NameDeclDuplicate, Error, "Duplicate name being declared in the same scope."); CARBON_DIAGNOSTIC(NameDeclPrevious, Note, "Name is previously declared here."); - emitter_->Build(dup_def_id, NameDeclDuplicate) - .Note(prev_def_id, NameDeclPrevious) + emitter_->Build(dup_def, NameDeclDuplicate) + .Note(prev_def, NameDeclPrevious) .Emit(); } @@ -182,43 +182,6 @@ auto Context::NoteUndefinedInterface(SemIR::InterfaceId interface_id, } } -auto Context::AddPackageImports(Parse::NodeId import_node, - IdentifierId package_id, - llvm::ArrayRef sem_irs, - bool has_load_error) -> void { - CARBON_CHECK(has_load_error || !sem_irs.empty()) - << "There should be either a load error or at least one IR."; - - auto name_id = SemIR::NameId::ForIdentifier(package_id); - - SemIR::ImportIRId first_id(import_irs().size()); - for (const auto* sem_ir : sem_irs) { - import_irs().Add(sem_ir); - } - if (has_load_error) { - import_irs().Add(nullptr); - } - SemIR::ImportIRId last_id(import_irs().size() - 1); - - auto type_id = GetBuiltinType(SemIR::BuiltinKind::NamespaceType); - auto inst_id = - AddInst({import_node, SemIR::Import{.type_id = type_id, - .first_import_ir_id = first_id, - .last_import_ir_id = last_id}}); - - // Add the import to lookup. Should always succeed because imports will be - // uniquely named. - AddNameToLookup(name_id, inst_id); - // Add a name for formatted output. This isn't used in name lookup in order - // to reduce indirection, but it's separate from the Import because it - // otherwise fits in an Inst. - auto bind_name_id = bind_names().Add( - {.name_id = name_id, .enclosing_scope_id = SemIR::NameScopeId::Package}); - AddInst({import_node, SemIR::BindName{.type_id = type_id, - .bind_name_id = bind_name_id, - .value_id = inst_id}}); -} - auto Context::AddNameToLookup(SemIR::NameId name_id, SemIR::InstId target_id) -> void { if (auto existing = scope_stack().LookupOrAddName(name_id, target_id); @@ -837,7 +800,6 @@ class TypeCompleter { case SemIR::FieldDecl::Kind: case SemIR::FunctionDecl::Kind: case SemIR::ImplDecl::Kind: - case SemIR::Import::Kind: case SemIR::ImportRefUnused::Kind: case SemIR::InitializeFrom::Kind: case SemIR::InterfaceDecl::Kind: diff --git a/toolchain/check/context.h b/toolchain/check/context.h index 8d103a120ea5..79525cbfbec3 100644 --- a/toolchain/check/context.h +++ b/toolchain/check/context.h @@ -106,12 +106,6 @@ class Context { sem_ir().insts().SetNodeId(inst_id, node_id); } - // Adds a package's imports to name lookup, with all libraries together. - // sem_irs will all be non-null; has_load_error must be used for any errors. - auto AddPackageImports(Parse::NodeId import_node, IdentifierId package_id, - llvm::ArrayRef sem_irs, - bool has_load_error) -> void; - // Adds a name to name lookup. Prints a diagnostic for name conflicts. auto AddNameToLookup(SemIR::NameId name_id, SemIR::InstId target_id) -> void; @@ -138,8 +132,8 @@ class Context { -> SemIR::InstId; // Prints a diagnostic for a duplicate name. - auto DiagnoseDuplicateName(SemIR::InstId dup_def_id, - SemIR::InstId prev_def_id) -> void; + auto DiagnoseDuplicateName(SemIRLocation dup_def, SemIRLocation prev_def) + -> void; // Prints a diagnostic for a missing name. auto DiagnoseNameNotFound(Parse::NodeId node_id, SemIR::NameId name_id) diff --git a/toolchain/check/decl_name_stack.cpp b/toolchain/check/decl_name_stack.cpp index e37ec8c5f701..9c80fe02b527 100644 --- a/toolchain/check/decl_name_stack.cpp +++ b/toolchain/check/decl_name_stack.cpp @@ -214,6 +214,19 @@ auto DeclNameStack::UpdateScopeIfNeeded(NameContext& name_context, auto scope_id = resolved_inst.As().name_scope_id; name_context.state = NameContext::State::Resolved; name_context.target_scope_id = scope_id; + auto& scope = context_->name_scopes().Get(scope_id); + if (scope.is_closed_import) { + CARBON_DIAGNOSTIC(QualifiedDeclOutsidePackage, Error, + "Imported packages cannot be used for declarations."); + CARBON_DIAGNOSTIC(QualifiedDeclOutsidePackageSource, Note, + "Package imported here."); + context_->emitter() + .Build(name_context.node_id, QualifiedDeclOutsidePackage) + .Note(scope.inst_id, QualifiedDeclOutsidePackageSource) + .Emit(); + // Only error once per package. + scope.is_closed_import = false; + } if (!is_unqualified) { PushNameQualifierScope(*context_, name_context.resolved_inst_id, scope_id, diff --git a/toolchain/check/eval.cpp b/toolchain/check/eval.cpp index 533441f82491..7b1e9366303a 100644 --- a/toolchain/check/eval.cpp +++ b/toolchain/check/eval.cpp @@ -506,7 +506,6 @@ auto TryEvalInst(Context& context, SemIR::InstId inst_id, SemIR::Inst inst) case SemIR::BranchIf::Kind: case SemIR::BranchWithArg::Kind: case SemIR::ImplDecl::Kind: - case SemIR::Import::Kind: case SemIR::Param::Kind: case SemIR::ReturnExpr::Kind: case SemIR::Return::Kind: diff --git a/toolchain/check/import.cpp b/toolchain/check/import.cpp index 1f7806269d5f..d0c44f77a6f7 100644 --- a/toolchain/check/import.cpp +++ b/toolchain/check/import.cpp @@ -70,6 +70,50 @@ static auto CopyNameFromImportIR(Context& context, return import_name_id; } +// Adds a namespace to the IR. The bool on return is true if there was a name +// conflict. diagnose_duplicate_namespace is used when handling a cross-package +// import, where an existing namespace is in the current package and the new +// namespace is a different package. +static auto AddNamespace( + Context& context, SemIR::TypeId namespace_type_id, + Parse::ImportDirectiveId node_id, SemIR::NameId name_id, + SemIR::NameScopeId enclosing_scope_id, bool diagnose_duplicate_namespace, + std::optional> make_import_id) + -> std::pair { + auto& enclosing_scope = context.name_scopes().Get(enclosing_scope_id); + auto [it, success] = + enclosing_scope.names.insert({name_id, SemIR::InstId::Invalid}); + if (!success) { + if (auto namespace_inst = + context.insts().TryGetAs(it->second)) { + if (diagnose_duplicate_namespace) { + context.DiagnoseDuplicateName(node_id, it->second); + } + return {namespace_inst->name_scope_id, true}; + } + } + + auto import_id = + make_import_id ? (*make_import_id)() : SemIR::InstId::Invalid; + auto namespace_inst = SemIR::Namespace{ + namespace_type_id, SemIR::NameScopeId::Invalid, import_id}; + // Use the invalid node because there's no node to associate with. + auto namespace_id = context.AddPlaceholderInst({node_id, namespace_inst}); + namespace_inst.name_scope_id = + context.name_scopes().Add(namespace_id, name_id, enclosing_scope_id); + context.ReplaceInstBeforeConstantUse(namespace_id, {node_id, namespace_inst}); + + // Diagnose if there's a name conflict, but still produce the namespace to + // supersede the name conflict in order to avoid repeat diagnostics. + if (!success) { + context.DiagnoseDuplicateName(namespace_id, it->second); + } + + it->second = namespace_id; + return {namespace_inst.name_scope_id, false}; +} + +// Adds a copied namespace to the cache. static auto CacheCopiedNamespace( llvm::DenseMap& copied_namespaces, SemIR::NameScopeId import_scope_id, SemIR::NameScopeId to_scope_id) @@ -90,42 +134,19 @@ static auto CopySingleNameScopeFromImportIR( SemIR::NameScopeId import_scope_id, SemIR::NameScopeId enclosing_scope_id, SemIR::NameId name_id, SemIR::TypeId namespace_type_id) -> SemIR::NameScopeId { - auto& scope = context.name_scopes().Get(enclosing_scope_id); - auto [it, success] = scope.names.insert({name_id, SemIR::InstId::Invalid}); - if (!success) { - if (auto namespace_inst = - context.insts().TryGetAs(it->second)) { - // Namespaces are open, so we can append to the existing one even if it - // comes from a different file. - CacheCopiedNamespace(copied_namespaces, import_scope_id, - namespace_inst->name_scope_id); - return namespace_inst->name_scope_id; - } - } - // Produce the namespace for the entry. - auto ref_id = context.AddInst(SemIR::ImportRefUsed{ - .type_id = namespace_type_id, .ir_id = ir_id, .inst_id = import_inst_id}); - auto namespace_inst = - SemIR::Namespace{namespace_type_id, SemIR::NameScopeId::Invalid, ref_id}; - // Use the invalid node because there's no node to associate with. - auto namespace_id = - context.AddPlaceholderInst({Parse::NodeId::Invalid, namespace_inst}); - namespace_inst.name_scope_id = - context.name_scopes().Add(namespace_id, name_id, enclosing_scope_id); - context.ReplaceInstBeforeConstantUse( - namespace_id, {Parse::NodeId::Invalid, namespace_inst}); + auto make_import_id = [&]() { + return context.AddInst(SemIR::ImportRefUsed{.type_id = namespace_type_id, + .ir_id = ir_id, + .inst_id = import_inst_id}); + }; + auto [namespace_scope_id, _] = + AddNamespace(context, namespace_type_id, Parse::NodeId::Invalid, name_id, + enclosing_scope_id, /*diagnose_duplicate_namespace=*/false, + make_import_id); - // Diagnose if there's a name conflict, but still produce the namespace to - // supersede the name conflict in order to avoid repeat diagnostics. - if (!success) { - context.DiagnoseDuplicateName(namespace_id, it->second); - } - - it->second = namespace_id; - CacheCopiedNamespace(copied_namespaces, import_scope_id, - namespace_inst.name_scope_id); - return namespace_inst.name_scope_id; + CacheCopiedNamespace(copied_namespaces, import_scope_id, namespace_scope_id); + return namespace_scope_id; } // Copies enclosing name scopes from the import IR. Handles the parent @@ -180,8 +201,9 @@ static auto CopyEnclosingNameScopesFromImportIR( return scope_cursor; } -auto Import(Context& context, SemIR::TypeId namespace_type_id, - const SemIR::File& import_sem_ir) -> void { +auto ImportLibraryFromCurrentPackage(Context& context, + SemIR::TypeId namespace_type_id, + const SemIR::File& import_sem_ir) -> void { auto ir_id = context.import_irs().Add(&import_sem_ir); for (const auto import_inst_id : @@ -227,4 +249,30 @@ auto Import(Context& context, SemIR::TypeId namespace_type_id, } } +auto ImportLibrariesFromOtherPackage(Context& context, + SemIR::TypeId namespace_type_id, + Parse::ImportDirectiveId node_id, + IdentifierId package_id, + llvm::ArrayRef sem_irs, + bool has_load_error) -> void { + CARBON_CHECK(has_load_error || !sem_irs.empty()) + << "There should be either a load error or at least one IR."; + + auto name_id = SemIR::NameId::ForIdentifier(package_id); + + auto [namespace_scope_id, is_duplicate] = AddNamespace( + context, namespace_type_id, node_id, name_id, SemIR::NameScopeId::Package, + /*diagnose_duplicate_namespace=*/true, /*make_import_id=*/std::nullopt); + + auto& scope = context.name_scopes().Get(namespace_scope_id); + scope.is_closed_import = !is_duplicate; + for (const auto* sem_ir : sem_irs) { + scope.import_ir_scopes.push_back( + {context.import_irs().Add(sem_ir), SemIR::NameScopeId::Package}); + } + if (has_load_error) { + scope.has_error = has_load_error; + } +} + } // namespace Carbon::Check diff --git a/toolchain/check/import.h b/toolchain/check/import.h index 0dad0bcfa0e2..5f4849d92650 100644 --- a/toolchain/check/import.h +++ b/toolchain/check/import.h @@ -10,9 +10,26 @@ namespace Carbon::Check { -// Add imports to the root block. -auto Import(Context& context, SemIR::TypeId namespace_type_id, - const SemIR::File& import_sem_ir) -> void; +// Add imports from a single library in the current package. This pulls in all +// names; conflicts for things such as `package.a.b.c` will be flagged even +// though they are several layers deep. +auto ImportLibraryFromCurrentPackage(Context& context, + SemIR::TypeId namespace_type_id, + const SemIR::File& import_sem_ir) -> void; + +// Adds another package's imports to name lookup, with all libraries together. +// This only adds the package name to lookup, so that `package.ImportedPackage` +// 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. +auto ImportLibrariesFromOtherPackage(Context& context, + SemIR::TypeId namespace_type_id, + Parse::ImportDirectiveId node_id, + IdentifierId package_id, + llvm::ArrayRef sem_irs, + bool has_load_error) -> void; } // namespace Carbon::Check diff --git a/toolchain/check/testdata/packages/cross_package_import.carbon b/toolchain/check/testdata/packages/cross_package_import.carbon new file mode 100644 index 000000000000..28436ba30ed6 --- /dev/null +++ b/toolchain/check/testdata/packages/cross_package_import.carbon @@ -0,0 +1,276 @@ +// 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_fn.carbon + +package Other library "fn" api; + +fn F() {} + +// --- other_fn_conflict.carbon + +package Other library "fn_conflict" api; + +fn F() {} + +// --- other_fn2.carbon + +package Other library "fn2" api; + +fn F2() {} + +// --- main_other_ns.carbon + +library "other_ns" api; + +namespace Other; + +// ============================================================================ +// Test files +// ============================================================================ + +// --- fail_main_use_other.carbon +// TODO: Once name lookup works, nothing should fail here. + +library "use_other" api; + +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. + // CHECK:STDERR: Other.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 + +library "unused_other_ambiguous" api; + +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; + +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: Other.F(); + // CHECK:STDERR: ^~~~~~~ + Other.F(); +} + +// --- fail_main_namespace_conflict.carbon + +library "namespace_conflict" api; + +import library "other_ns"; +// CHECK:STDERR: fail_main_namespace_conflict.carbon:[[@LINE+6]]:1: ERROR: Duplicate name being declared in the same scope. +// CHECK:STDERR: import Other library "fn"; +// CHECK:STDERR: ^~~~~~ +// CHECK:STDERR: main_other_ns.carbon:4:1: Name is previously declared here. +// CHECK:STDERR: namespace Other; +// CHECK:STDERR: ^~~~~~~~~~~~~~~~ +import Other library "fn"; + +fn Other.F() {} + +// --- fail_main_reopen_other.carbon + +library "reopen_other" api; + +import Other library "fn"; + +// CHECK:STDERR: fail_main_reopen_other.carbon:[[@LINE+6]]:11: ERROR: Imported packages cannot be used for declarations. +// CHECK:STDERR: namespace Other; +// CHECK:STDERR: ^~~~~ +// CHECK:STDERR: fail_main_reopen_other.carbon:[[@LINE-5]]:1: Package imported here. +// CHECK:STDERR: import Other library "fn"; +// CHECK:STDERR: ^~~~~~ +namespace Other; + +// This is not diagnosed after the diagnostic on `namespace Other;`. +fn Other.G() {} + +// --- fail_main_add_to_other.carbon + +library "add_to_other" api; + +import Other library "fn"; + +// CHECK:STDERR: fail_main_add_to_other.carbon:[[@LINE+6]]:4: ERROR: Imported packages cannot be used for declarations. +// CHECK:STDERR: fn Other.G() {} +// CHECK:STDERR: ^~~~~ +// CHECK:STDERR: fail_main_add_to_other.carbon:[[@LINE-5]]:1: Package imported here. +// CHECK:STDERR: import Other library "fn"; +// CHECK:STDERR: ^~~~~~ +fn Other.G() {} + +// CHECK:STDOUT: --- other_fn.carbon +// CHECK:STDOUT: +// CHECK:STDOUT: file { +// CHECK:STDOUT: package: = namespace [template] { +// CHECK:STDOUT: .F = %F +// CHECK:STDOUT: } +// CHECK:STDOUT: %F: = fn_decl @F [template] {} +// CHECK:STDOUT: } +// CHECK:STDOUT: +// CHECK:STDOUT: fn @F() { +// CHECK:STDOUT: !entry: +// CHECK:STDOUT: return +// CHECK:STDOUT: } +// CHECK:STDOUT: +// CHECK:STDOUT: --- other_fn_conflict.carbon +// CHECK:STDOUT: +// CHECK:STDOUT: file { +// CHECK:STDOUT: package: = namespace [template] { +// CHECK:STDOUT: .F = %F +// CHECK:STDOUT: } +// CHECK:STDOUT: %F: = fn_decl @F [template] {} +// CHECK:STDOUT: } +// CHECK:STDOUT: +// CHECK:STDOUT: fn @F() { +// CHECK:STDOUT: !entry: +// CHECK:STDOUT: return +// CHECK:STDOUT: } +// CHECK:STDOUT: +// CHECK:STDOUT: --- other_fn2.carbon +// CHECK:STDOUT: +// CHECK:STDOUT: file { +// CHECK:STDOUT: package: = namespace [template] { +// CHECK:STDOUT: .F2 = %F2 +// CHECK:STDOUT: } +// CHECK:STDOUT: %F2: = fn_decl @F2 [template] {} +// CHECK:STDOUT: } +// CHECK:STDOUT: +// CHECK:STDOUT: fn @F2() { +// CHECK:STDOUT: !entry: +// CHECK:STDOUT: return +// CHECK:STDOUT: } +// CHECK:STDOUT: +// CHECK:STDOUT: --- main_other_ns.carbon +// CHECK:STDOUT: +// CHECK:STDOUT: file { +// CHECK:STDOUT: package: = namespace [template] { +// CHECK:STDOUT: .Other = %Other +// CHECK:STDOUT: } +// CHECK:STDOUT: %Other: = namespace [template] {} +// CHECK:STDOUT: } +// CHECK:STDOUT: +// CHECK:STDOUT: --- fail_main_use_other.carbon +// CHECK:STDOUT: +// CHECK:STDOUT: file { +// CHECK:STDOUT: package: = namespace [template] { +// CHECK:STDOUT: .Other = %Other +// CHECK:STDOUT: .Run = %Run +// CHECK:STDOUT: } +// CHECK:STDOUT: %Other: = namespace [template] {} +// CHECK:STDOUT: %Run: = fn_decl @Run [template] {} +// CHECK:STDOUT: } +// CHECK:STDOUT: +// CHECK:STDOUT: fn @Run() { +// CHECK:STDOUT: !entry: +// CHECK:STDOUT: %Other.ref.loc12: = name_ref Other, file.%Other [template = file.%Other] +// CHECK:STDOUT: %F.ref: = name_ref F, [template = ] +// CHECK:STDOUT: %Other.ref.loc16: = name_ref Other, file.%Other [template = file.%Other] +// CHECK:STDOUT: %F2.ref: = name_ref F2, [template = ] +// CHECK:STDOUT: return +// CHECK:STDOUT: } +// CHECK:STDOUT: +// CHECK:STDOUT: --- main_unused_other_ambiguous.carbon +// CHECK:STDOUT: +// CHECK:STDOUT: file { +// CHECK:STDOUT: package: = namespace [template] { +// CHECK:STDOUT: .Other = %Other +// CHECK:STDOUT: } +// CHECK:STDOUT: %Other: = namespace [template] {} +// CHECK:STDOUT: } +// CHECK:STDOUT: +// CHECK:STDOUT: --- fail_main_use_other_ambiguous.carbon +// CHECK:STDOUT: +// CHECK:STDOUT: file { +// CHECK:STDOUT: package: = namespace [template] { +// CHECK:STDOUT: .Other = %Other +// CHECK:STDOUT: .Run = %Run +// CHECK:STDOUT: } +// CHECK:STDOUT: %Other: = namespace [template] {} +// CHECK:STDOUT: %Run: = fn_decl @Run [template] {} +// CHECK:STDOUT: } +// CHECK:STDOUT: +// CHECK:STDOUT: fn @Run() { +// CHECK:STDOUT: !entry: +// CHECK:STDOUT: %Other.ref: = name_ref Other, file.%Other [template = file.%Other] +// CHECK:STDOUT: %F.ref: = name_ref F, [template = ] +// CHECK:STDOUT: return +// CHECK:STDOUT: } +// CHECK:STDOUT: +// CHECK:STDOUT: --- fail_main_namespace_conflict.carbon +// CHECK:STDOUT: +// CHECK:STDOUT: file { +// CHECK:STDOUT: package: = namespace [template] { +// CHECK:STDOUT: .Other = %Other +// CHECK:STDOUT: } +// CHECK:STDOUT: %import_ref: = import_ref ir1, inst+1, used +// CHECK:STDOUT: %Other: = namespace %import_ref, [template] { +// CHECK:STDOUT: .F = %F +// CHECK:STDOUT: } +// CHECK:STDOUT: %F: = fn_decl @F [template] {} +// CHECK:STDOUT: } +// CHECK:STDOUT: +// CHECK:STDOUT: fn @F() { +// CHECK:STDOUT: !entry: +// CHECK:STDOUT: return +// CHECK:STDOUT: } +// CHECK:STDOUT: +// CHECK:STDOUT: --- fail_main_reopen_other.carbon +// CHECK:STDOUT: +// CHECK:STDOUT: file { +// CHECK:STDOUT: package: = namespace [template] { +// CHECK:STDOUT: .Other = %Other +// CHECK:STDOUT: } +// CHECK:STDOUT: %Other: = namespace [template] { +// CHECK:STDOUT: .G = %G +// CHECK:STDOUT: } +// CHECK:STDOUT: %.loc12: = namespace [template] {} +// CHECK:STDOUT: %G: = fn_decl @G [template] {} +// CHECK:STDOUT: } +// CHECK:STDOUT: +// CHECK:STDOUT: fn @G() { +// CHECK:STDOUT: !entry: +// CHECK:STDOUT: return +// CHECK:STDOUT: } +// CHECK:STDOUT: +// CHECK:STDOUT: --- fail_main_add_to_other.carbon +// CHECK:STDOUT: +// CHECK:STDOUT: file { +// CHECK:STDOUT: package: = namespace [template] { +// CHECK:STDOUT: .Other = %Other +// CHECK:STDOUT: } +// CHECK:STDOUT: %Other: = namespace [template] { +// CHECK:STDOUT: .G = %G +// CHECK:STDOUT: } +// CHECK:STDOUT: %G: = fn_decl @G [template] {} +// CHECK:STDOUT: } +// CHECK:STDOUT: +// CHECK:STDOUT: fn @G() { +// CHECK:STDOUT: !entry: +// CHECK:STDOUT: return +// CHECK:STDOUT: } +// CHECK:STDOUT: diff --git a/toolchain/check/testdata/packages/explicit_imports.carbon b/toolchain/check/testdata/packages/explicit_imports.carbon index b9d9efcaea8f..4a30147813bb 100644 --- a/toolchain/check/testdata/packages/explicit_imports.carbon +++ b/toolchain/check/testdata/packages/explicit_imports.carbon @@ -55,9 +55,10 @@ import library "lib"; // CHECK:STDOUT: --- different_package.carbon // CHECK:STDOUT: // CHECK:STDOUT: file { -// CHECK:STDOUT: package: = namespace [template] {} -// CHECK:STDOUT: %import: = import ir1, ir2 -// CHECK:STDOUT: %Api: = bind_name Api, %import +// CHECK:STDOUT: package: = namespace [template] { +// CHECK:STDOUT: .Api = %Api +// CHECK:STDOUT: } +// CHECK:STDOUT: %Api: = namespace [template] {} // CHECK:STDOUT: } // CHECK:STDOUT: // CHECK:STDOUT: --- main_lib_api.carbon diff --git a/toolchain/check/testdata/packages/fail_cycle.carbon b/toolchain/check/testdata/packages/fail_cycle.carbon index 6d9e30040874..01f366eb2127 100644 --- a/toolchain/check/testdata/packages/fail_cycle.carbon +++ b/toolchain/check/testdata/packages/fail_cycle.carbon @@ -50,25 +50,34 @@ import B; // CHECK:STDOUT: --- fail_a.carbon // CHECK:STDOUT: // CHECK:STDOUT: file { -// CHECK:STDOUT: package: = namespace [template] {} -// CHECK:STDOUT: %import: = import ir1, ir1 -// CHECK:STDOUT: %B: = bind_name B, %import +// CHECK:STDOUT: package: = namespace [template] { +// CHECK:STDOUT: .B = %B +// CHECK:STDOUT: } +// CHECK:STDOUT: %B: = namespace [template] { +// CHECK:STDOUT: has_error +// CHECK:STDOUT: } // CHECK:STDOUT: } // CHECK:STDOUT: // CHECK:STDOUT: --- fail_b.carbon // CHECK:STDOUT: // CHECK:STDOUT: file { -// CHECK:STDOUT: package: = namespace [template] {} -// CHECK:STDOUT: %import: = import ir1, ir1 -// CHECK:STDOUT: %C: = bind_name C, %import +// CHECK:STDOUT: package: = namespace [template] { +// CHECK:STDOUT: .C = %C +// CHECK:STDOUT: } +// CHECK:STDOUT: %C: = namespace [template] { +// CHECK:STDOUT: has_error +// CHECK:STDOUT: } // CHECK:STDOUT: } // CHECK:STDOUT: // CHECK:STDOUT: --- fail_c.carbon // CHECK:STDOUT: // CHECK:STDOUT: file { -// CHECK:STDOUT: package: = namespace [template] {} -// CHECK:STDOUT: %import: = import ir1, ir1 -// CHECK:STDOUT: %A: = bind_name A, %import +// CHECK:STDOUT: package: = namespace [template] { +// CHECK:STDOUT: .A = %A +// CHECK:STDOUT: } +// CHECK:STDOUT: %A: = namespace [template] { +// CHECK:STDOUT: has_error +// CHECK:STDOUT: } // CHECK:STDOUT: } // CHECK:STDOUT: // CHECK:STDOUT: --- fail_c.impl.carbon @@ -82,8 +91,11 @@ import B; // CHECK:STDOUT: --- fail_cycle_child.carbon // CHECK:STDOUT: // CHECK:STDOUT: file { -// CHECK:STDOUT: package: = namespace [template] {} -// CHECK:STDOUT: %import: = import ir1, ir1 -// CHECK:STDOUT: %B: = bind_name B, %import +// CHECK:STDOUT: package: = namespace [template] { +// CHECK:STDOUT: .B = %B +// CHECK:STDOUT: } +// CHECK:STDOUT: %B: = namespace [template] { +// CHECK:STDOUT: has_error +// CHECK:STDOUT: } // CHECK:STDOUT: } // CHECK:STDOUT: diff --git a/toolchain/check/testdata/packages/fail_import_invalid.carbon b/toolchain/check/testdata/packages/fail_import_invalid.carbon index 46838511fc0c..c304fb86327a 100644 --- a/toolchain/check/testdata/packages/fail_import_invalid.carbon +++ b/toolchain/check/testdata/packages/fail_import_invalid.carbon @@ -133,8 +133,11 @@ import ImportNotFound; // CHECK:STDOUT: --- fail_not_found.carbon // CHECK:STDOUT: // CHECK:STDOUT: file { -// CHECK:STDOUT: package: = namespace [template] {} -// CHECK:STDOUT: %import: = import ir1, ir1 -// CHECK:STDOUT: %ImportNotFound: = bind_name ImportNotFound, %import +// CHECK:STDOUT: package: = namespace [template] { +// CHECK:STDOUT: .ImportNotFound = %ImportNotFound +// CHECK:STDOUT: } +// CHECK:STDOUT: %ImportNotFound: = namespace [template] { +// CHECK:STDOUT: has_error +// CHECK:STDOUT: } // CHECK:STDOUT: } // CHECK:STDOUT: diff --git a/toolchain/check/testdata/packages/fail_import_repeat.carbon b/toolchain/check/testdata/packages/fail_import_repeat.carbon index 07d9d32fb65b..1099ff540413 100644 --- a/toolchain/check/testdata/packages/fail_import_repeat.carbon +++ b/toolchain/check/testdata/packages/fail_import_repeat.carbon @@ -79,9 +79,10 @@ import library default; // CHECK:STDOUT: --- fail_import.carbon // CHECK:STDOUT: // CHECK:STDOUT: file { -// CHECK:STDOUT: package: = namespace [template] {} -// CHECK:STDOUT: %import: = import ir2, ir3 -// CHECK:STDOUT: %Api: = bind_name Api, %import +// CHECK:STDOUT: package: = namespace [template] { +// CHECK:STDOUT: .Api = %Api +// CHECK:STDOUT: } +// CHECK:STDOUT: %Api: = namespace [template] {} // CHECK:STDOUT: } // CHECK:STDOUT: // CHECK:STDOUT: --- fail_default_import.carbon diff --git a/toolchain/diagnostics/diagnostic_kind.def b/toolchain/diagnostics/diagnostic_kind.def index 2d69e340d352..876c63fe013f 100644 --- a/toolchain/diagnostics/diagnostic_kind.def +++ b/toolchain/diagnostics/diagnostic_kind.def @@ -210,6 +210,13 @@ CARBON_DIAGNOSTIC_KIND(MissingImplInMemberAccess) CARBON_DIAGNOSTIC_KIND(ExpectedInitializerAfterLet) CARBON_DIAGNOSTIC_KIND(ExpectedSymbolicBindingInAssociatedConstant) +// Qualified declaration name checking. +CARBON_DIAGNOSTIC_KIND(QualifiedDeclOutsidePackage) +CARBON_DIAGNOSTIC_KIND(QualifiedDeclOutsidePackageSource) +CARBON_DIAGNOSTIC_KIND(QualifiedDeclOutsideScopeEntity) +CARBON_DIAGNOSTIC_KIND(QualifiedDeclInIncompleteClassScope) +CARBON_DIAGNOSTIC_KIND(QualifiedDeclInUndefinedInterfaceScope) + CARBON_DIAGNOSTIC_KIND(AddrOfEphemeralRef) CARBON_DIAGNOSTIC_KIND(AddrOfNonRef) CARBON_DIAGNOSTIC_KIND(AddrOnNonSelfParam) @@ -262,9 +269,6 @@ CARBON_DIAGNOSTIC_KIND(ReturnStatementMissingExpr) CARBON_DIAGNOSTIC_KIND(ImplicitAsConversionFailure) CARBON_DIAGNOSTIC_KIND(ExplicitAsConversionFailure) CARBON_DIAGNOSTIC_KIND(TypeExprEvaluationFailure) -CARBON_DIAGNOSTIC_KIND(QualifiedDeclOutsideScopeEntity) -CARBON_DIAGNOSTIC_KIND(QualifiedDeclInIncompleteClassScope) -CARBON_DIAGNOSTIC_KIND(QualifiedDeclInUndefinedInterfaceScope) CARBON_DIAGNOSTIC_KIND(QualifiedNameInNonScope) CARBON_DIAGNOSTIC_KIND(QualifiedNameNonScopeEntity) CARBON_DIAGNOSTIC_KIND(QualifiedExprInIncompleteClassScope) diff --git a/toolchain/lower/handle.cpp b/toolchain/lower/handle.cpp index acda3092c293..958160de137e 100644 --- a/toolchain/lower/handle.cpp +++ b/toolchain/lower/handle.cpp @@ -217,11 +217,6 @@ auto HandleImplDecl(FunctionContext& /*context*/, SemIR::InstId /*inst_id*/, FatalErrorIfEncountered(inst); } -auto HandleImport(FunctionContext& /*context*/, SemIR::InstId /*inst_id*/, - SemIR::Import inst) -> void { - FatalErrorIfEncountered(inst); -} - auto HandleImportRefUnused(FunctionContext& /*context*/, SemIR::InstId /*inst_id*/, SemIR::ImportRefUnused inst) -> void { diff --git a/toolchain/parse/handle_import_and_package.cpp b/toolchain/parse/handle_import_and_package.cpp index 8e00befe7bb7..f0f4f8481414 100644 --- a/toolchain/parse/handle_import_and_package.cpp +++ b/toolchain/parse/handle_import_and_package.cpp @@ -5,6 +5,7 @@ #include "toolchain/base/value_store.h" #include "toolchain/lex/tokenized_buffer.h" #include "toolchain/parse/context.h" +#include "toolchain/parse/node_ids.h" #include "toolchain/parse/node_kind.h" namespace Carbon::Parse { @@ -75,7 +76,8 @@ static auto HandleDirectiveContent(Context& context, NodeKind directive, llvm::function_ref on_parse_error) -> void { - Tree::PackagingNames names{.node = NodeId(state.subtree_start)}; + Tree::PackagingNames names{ + .node_id = ImportDirectiveId(NodeId(state.subtree_start))}; if (directive != NodeKind::LibraryDirective) { if (auto package_name_token = context.ConsumeIf(Lex::TokenKind::Identifier)) { diff --git a/toolchain/parse/node_ids.h b/toolchain/parse/node_ids.h index 7edaf90d8459..a2c70b6104a9 100644 --- a/toolchain/parse/node_ids.h +++ b/toolchain/parse/node_ids.h @@ -91,6 +91,7 @@ using AnyFunctionDeclId = using AnyImplDeclId = NodeIdOneOf; using AnyInterfaceDeclId = NodeIdOneOf; +using AnyNamespaceId = NodeIdOneOf; // NodeId with kind that is anything but T::Kind. template diff --git a/toolchain/parse/tree.h b/toolchain/parse/tree.h index 1b5fbd632ad9..5e5cf1c05285 100644 --- a/toolchain/parse/tree.h +++ b/toolchain/parse/tree.h @@ -57,7 +57,7 @@ class Tree : public Printable { // Names in packaging, whether the file's packaging or an import. Links back // to the node for diagnostics. struct PackagingNames { - NodeId node; + ImportDirectiveId node_id; IdentifierId package_id = IdentifierId::Invalid; StringLiteralValueId library_id = StringLiteralValueId::Invalid; }; diff --git a/toolchain/sem_ir/file.cpp b/toolchain/sem_ir/file.cpp index 9f1199c8b86c..1b1ac0d78b37 100644 --- a/toolchain/sem_ir/file.cpp +++ b/toolchain/sem_ir/file.cpp @@ -234,7 +234,6 @@ static auto GetTypePrecedence(InstKind kind) -> int { case FieldDecl::Kind: case FunctionDecl::Kind: case ImplDecl::Kind: - case Import::Kind: case ImportRefUnused::Kind: case InitializeFrom::Kind: case InterfaceDecl::Kind: @@ -483,7 +482,6 @@ static auto StringifyTypeExprImpl(const SemIR::File& outer_sem_ir, case FieldDecl::Kind: case FunctionDecl::Kind: case ImplDecl::Kind: - case Import::Kind: case ImportRefUnused::Kind: case InitializeFrom::Kind: case InterfaceDecl::Kind: @@ -549,7 +547,6 @@ auto GetExprCategory(const File& file, InstId inst_id) -> ExprCategory { case FieldDecl::Kind: case FunctionDecl::Kind: case ImplDecl::Kind: - case Import::Kind: case ImportRefUnused::Kind: case Namespace::Kind: case Return::Kind: diff --git a/toolchain/sem_ir/formatter.cpp b/toolchain/sem_ir/formatter.cpp index 659e3917b0c0..52d9044bdb3c 100644 --- a/toolchain/sem_ir/formatter.cpp +++ b/toolchain/sem_ir/formatter.cpp @@ -503,10 +503,6 @@ class InstNamer { CollectNamesInBlock(scope_id, inst.As().decl_block_id); break; } - case Import::Kind: { - add_inst_name("import"); - continue; - } case ImportRefUnused::Kind: case ImportRefUsed::Kind: { add_inst_name("import_ref"); diff --git a/toolchain/sem_ir/inst_kind.def b/toolchain/sem_ir/inst_kind.def index 2df9477926a4..83e6ea995bf0 100644 --- a/toolchain/sem_ir/inst_kind.def +++ b/toolchain/sem_ir/inst_kind.def @@ -50,7 +50,6 @@ CARBON_SEM_IR_INST_KIND(FacetTypeAccess) CARBON_SEM_IR_INST_KIND(FieldDecl) CARBON_SEM_IR_INST_KIND(FunctionDecl) CARBON_SEM_IR_INST_KIND(ImplDecl) -CARBON_SEM_IR_INST_KIND(Import) CARBON_SEM_IR_INST_KIND(ImportRefUnused) CARBON_SEM_IR_INST_KIND(ImportRefUsed) CARBON_SEM_IR_INST_KIND(InitializeFrom) diff --git a/toolchain/sem_ir/name_scope.h b/toolchain/sem_ir/name_scope.h index cfc16aaa9859..0df120ff080e 100644 --- a/toolchain/sem_ir/name_scope.h +++ b/toolchain/sem_ir/name_scope.h @@ -74,6 +74,14 @@ struct NameScope : Printable { // to be missing names as a result of the error, and no further errors are // produced for lookup failures in this scope. bool has_error = false; + + // True if this is a closed namespace created by importing a package. + bool is_closed_import = false; + + // Imported IR scopes that compose this namespace. This will be empty for + // scopes that correspond to the current package. + llvm::SmallVector, 0> + import_ir_scopes; }; // Provides a ValueStore wrapper for an API specific to name scopes. diff --git a/toolchain/sem_ir/typed_insts.h b/toolchain/sem_ir/typed_insts.h index bb7940bcbbd6..852fbdc3a724 100644 --- a/toolchain/sem_ir/typed_insts.h +++ b/toolchain/sem_ir/typed_insts.h @@ -457,19 +457,6 @@ struct ImplDecl { InstBlockId decl_block_id; }; -// An import corresponds to some number of IRs. The range of imported IRs is -// inclusive of last_import_ir_id, and will always be non-empty. If -// there was an import error, first_import_ir_id will reference a -// nullptr IR; there should only ever be one nullptr in the range. -struct Import { - // TODO: Should always be an ImportDirectiveId? - static constexpr auto Kind = InstKind::Import.Define("import"); - - TypeId type_id; - ImportIRId first_import_ir_id; - ImportIRId last_import_ir_id; -}; - // Common representation for all kinds of `ImportRef*` node. struct AnyImportRef { static constexpr InstKind Kinds[] = {InstKind::ImportRefUnused, @@ -482,7 +469,6 @@ struct AnyImportRef { // An imported entity that hasn't yet been referenced. If referenced, it should // turn into an ImportRefUsed. -// TODO: Add ImportRefUsed. struct ImportRefUnused { // No parse node: any parse node logic must use the referenced IR. static constexpr auto Kind = @@ -582,7 +568,7 @@ struct NameRef { struct Namespace { static constexpr auto Kind = - InstKind::Namespace.Define("namespace"); + InstKind::Namespace.Define("namespace"); TypeId type_id; NameScopeId name_scope_id;