From f4a741903fe410c2cc07930394ee2a0a78fcd3ba Mon Sep 17 00:00:00 2001 From: Jon Ross-Perkins Date: Thu, 25 Jan 2024 10:19:22 -0800 Subject: [PATCH] Add import support for remaining decl types. (#3651) I'd excluded these initially just because I was thinking towards copies, but under the current model I'm trying to catch all the decl types just for consistency. Note references will still be a TODO error (LazyImportRef is already tested for this, it just didn't feel necessary to add individual tests while I try to sort out behavior). Fixes an oversight where declarations in an entity's scope were being added to the list of exports. Note I'm trimming some Import API arguments as now-unused. --- toolchain/check/check.cpp | 4 +- toolchain/check/context.h | 5 +- toolchain/check/decl_name_stack.cpp | 8 ++- toolchain/check/import.cpp | 25 +++++--- toolchain/check/import.h | 3 +- toolchain/check/scope_index.h | 4 ++ toolchain/check/testdata/class/import.carbon | 64 +++++++++++++++++++ .../function/declaration/import.carbon | 36 +++++++++++ .../function/definition/import.carbon | 39 +++++++++++ .../check/testdata/interface/import.carbon | 58 +++++++++++++++++ 10 files changed, 230 insertions(+), 16 deletions(-) create mode 100644 toolchain/check/testdata/class/import.carbon create mode 100644 toolchain/check/testdata/function/declaration/import.carbon create mode 100644 toolchain/check/testdata/function/definition/import.carbon create mode 100644 toolchain/check/testdata/interface/import.carbon diff --git a/toolchain/check/check.cpp b/toolchain/check/check.cpp index 38c26989abd2..ddc9e23d3934 100644 --- a/toolchain/check/check.cpp +++ b/toolchain/check/check.cpp @@ -165,8 +165,7 @@ 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, self_import->second.node, - import_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; @@ -183,6 +182,7 @@ static auto InitPackageScopeAndImports(Context& context, UnitInfo& unit_info) // Push the scope; there are no names to add. context.PushScope(package_inst_id, SemIR::NameScopeId::Package); } + CARBON_CHECK(context.current_scope_index() == ScopeIndex::Package); for (auto& [package_id, package_imports] : unit_info.package_imports_map) { if (!package_id.is_valid()) { diff --git a/toolchain/check/context.h b/toolchain/check/context.h index 0ff65d7926d4..c87d10dc8598 100644 --- a/toolchain/check/context.h +++ b/toolchain/check/context.h @@ -547,8 +547,9 @@ class Context { llvm::SmallVector> non_lexical_scope_stack_; - // The index of the next scope that will be pushed onto scope_stack_. - ScopeIndex next_scope_index_ = ScopeIndex(0); + // The index of the next scope that will be pushed onto scope_stack_. The + // first is always the package scope. + ScopeIndex next_scope_index_ = ScopeIndex::Package; // The stack used for qualified declaration name construction. DeclNameStack decl_name_stack_; diff --git a/toolchain/check/decl_name_stack.cpp b/toolchain/check/decl_name_stack.cpp index d9b64d8cc011..bf6296d5aeb9 100644 --- a/toolchain/check/decl_name_stack.cpp +++ b/toolchain/check/decl_name_stack.cpp @@ -83,7 +83,13 @@ auto DeclNameStack::LookupOrAddName(NameContext name_context, QualifiedDeclOutsideScopeEntity); } } - context_->AddExport(target_id); + + // Exports are only tracked when the declaration is at the file-level + // scope. Otherwise, it's in some other entity, such as a class. + if (name_context.enclosing_scope == ScopeIndex::Package) { + context_->AddExport(target_id); + } + auto [_, success] = name_scope.names.insert( {name_context.unresolved_name_id, target_id}); CARBON_CHECK(success) diff --git a/toolchain/check/import.cpp b/toolchain/check/import.cpp index 083082b8b54a..9b13d4477633 100644 --- a/toolchain/check/import.cpp +++ b/toolchain/check/import.cpp @@ -17,8 +17,7 @@ namespace Carbon::Check { // Returns name information for the entity, corresponding to IDs in the import // IR rather than the current IR. May return Invalid for a TODO. -static auto GetImportName(Parse::NodeId parse_node, Context& context, - const SemIR::File& import_sem_ir, +static auto GetImportName(const SemIR::File& import_sem_ir, SemIR::Inst import_inst) -> std::pair { switch (import_inst.kind()) { @@ -28,12 +27,24 @@ static auto GetImportName(Parse::NodeId parse_node, Context& context, return {bind_name.name_id, bind_name.enclosing_scope_id}; } + case SemIR::InstKind::ClassDecl: { + const auto& class_info = import_sem_ir.classes().Get( + import_inst.As().class_id); + return {class_info.name_id, class_info.enclosing_scope_id}; + } + case SemIR::InstKind::FunctionDecl: { const auto& function = import_sem_ir.functions().Get( import_inst.As().function_id); return {function.name_id, function.enclosing_scope_id}; } + case SemIR::InstKind::InterfaceDecl: { + const auto& interface = import_sem_ir.interfaces().Get( + import_inst.As().interface_id); + return {interface.name_id, interface.enclosing_scope_id}; + } + case SemIR::InstKind::Namespace: { const auto& scope = import_sem_ir.name_scopes().Get( import_inst.As().name_scope_id); @@ -41,10 +52,7 @@ static auto GetImportName(Parse::NodeId parse_node, Context& context, } default: - context.TODO(parse_node, (llvm::Twine("Support GetImportName of ") + - import_inst.kind().name()) - .str()); - return {SemIR::NameId::Invalid, SemIR::NameScopeId::Invalid}; + CARBON_FATAL() << "Unsupported export kind: " << import_inst; } } @@ -172,15 +180,14 @@ static auto CopyEnclosingNameScopesFromImportIR( } auto Import(Context& context, SemIR::TypeId namespace_type_id, - Parse::NodeId import_node, const SemIR::File& import_sem_ir) - -> void { + const SemIR::File& import_sem_ir) -> void { auto ir_id = context.cross_ref_irs().Add(&import_sem_ir); for (const auto import_inst_id : import_sem_ir.inst_blocks().Get(SemIR::InstBlockId::Exports)) { auto import_inst = import_sem_ir.insts().Get(import_inst_id); auto [import_name_id, import_enclosing_scope_id] = - GetImportName(import_node, context, import_sem_ir, import_inst); + GetImportName(import_sem_ir, import_inst); // TODO: This should only be invalid when GetImportName for an inst // isn't yet implemented. Long-term this should be removed. if (!import_name_id.is_valid()) { diff --git a/toolchain/check/import.h b/toolchain/check/import.h index 499b84c0067a..0dad0bcfa0e2 100644 --- a/toolchain/check/import.h +++ b/toolchain/check/import.h @@ -12,8 +12,7 @@ namespace Carbon::Check { // Add imports to the root block. auto Import(Context& context, SemIR::TypeId namespace_type_id, - Parse::NodeId import_node, const SemIR::File& import_sem_ir) - -> void; + const SemIR::File& import_sem_ir) -> void; } // namespace Carbon::Check diff --git a/toolchain/check/scope_index.h b/toolchain/check/scope_index.h index bc58bfcd0de7..38cc649fc72b 100644 --- a/toolchain/check/scope_index.h +++ b/toolchain/check/scope_index.h @@ -18,9 +18,13 @@ namespace Carbon::Check { // `ScopeIndex` values are comparable. Lower `ScopeIndex` values correspond to // scopes entered earlier in the file. struct ScopeIndex : public IndexBase, public Printable { + static const ScopeIndex Package; + using IndexBase::IndexBase; }; +constexpr ScopeIndex ScopeIndex::Package = ScopeIndex(0); + } // namespace Carbon::Check #endif // CARBON_TOOLCHAIN_CHECK_SCOPE_INDEX_H_ diff --git a/toolchain/check/testdata/class/import.carbon b/toolchain/check/testdata/class/import.carbon new file mode 100644 index 000000000000..eb53337fe447 --- /dev/null +++ b/toolchain/check/testdata/class/import.carbon @@ -0,0 +1,64 @@ +// 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 + +// --- a.carbon + +library "a" api; + +class Empty { +} + +class ForwardDeclared; + +class ForwardDeclared { + fn F(); +} + +// --- b.carbon + +library "b" api; + +import library "a"; + +// TODO: When ready, consider tests of basic import functionality. + +// CHECK:STDOUT: --- a.carbon +// CHECK:STDOUT: +// CHECK:STDOUT: constants { +// CHECK:STDOUT: %Empty: type = class_type @Empty [template] +// CHECK:STDOUT: %.1: type = struct_type {} [template] +// CHECK:STDOUT: %ForwardDeclared: type = class_type @ForwardDeclared [template] +// CHECK:STDOUT: } +// CHECK:STDOUT: +// CHECK:STDOUT: file { +// CHECK:STDOUT: package: = namespace {.Empty = %Empty.decl, .ForwardDeclared = %ForwardDeclared.decl.loc7} [template] +// CHECK:STDOUT: %Empty.decl = class_decl @Empty, () +// CHECK:STDOUT: %ForwardDeclared.decl.loc7 = class_decl @ForwardDeclared, () +// CHECK:STDOUT: %ForwardDeclared.decl.loc9 = class_decl @ForwardDeclared, () +// CHECK:STDOUT: } +// CHECK:STDOUT: +// CHECK:STDOUT: class @Empty { +// CHECK:STDOUT: +// CHECK:STDOUT: !members: +// CHECK:STDOUT: } +// CHECK:STDOUT: +// CHECK:STDOUT: class @ForwardDeclared { +// CHECK:STDOUT: %F: = fn_decl @F [template] +// CHECK:STDOUT: +// CHECK:STDOUT: !members: +// CHECK:STDOUT: .F = %F +// CHECK:STDOUT: } +// CHECK:STDOUT: +// CHECK:STDOUT: fn @F(); +// CHECK:STDOUT: +// CHECK:STDOUT: --- b.carbon +// CHECK:STDOUT: +// CHECK:STDOUT: file { +// CHECK:STDOUT: package: = namespace {.Empty = %lazy_import_ref.1, .ForwardDeclared = %lazy_import_ref.2} [template] +// CHECK:STDOUT: %lazy_import_ref.1 = lazy_import_ref ir1, inst+1 +// CHECK:STDOUT: %lazy_import_ref.2 = lazy_import_ref ir1, inst+4 +// CHECK:STDOUT: } +// CHECK:STDOUT: diff --git a/toolchain/check/testdata/function/declaration/import.carbon b/toolchain/check/testdata/function/declaration/import.carbon new file mode 100644 index 000000000000..8b8df9c9e22c --- /dev/null +++ b/toolchain/check/testdata/function/declaration/import.carbon @@ -0,0 +1,36 @@ +// 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 + +// --- a.carbon + +library "a" api; + +fn F(); + +// --- b.carbon + +library "b" api; + +import library "a"; + +// TODO: When ready, consider tests of basic import functionality. + +// CHECK:STDOUT: --- a.carbon +// CHECK:STDOUT: +// CHECK:STDOUT: file { +// CHECK:STDOUT: package: = namespace {.F = %F} [template] +// CHECK:STDOUT: %F: = fn_decl @F [template] +// CHECK:STDOUT: } +// CHECK:STDOUT: +// CHECK:STDOUT: fn @F(); +// CHECK:STDOUT: +// CHECK:STDOUT: --- b.carbon +// CHECK:STDOUT: +// CHECK:STDOUT: file { +// CHECK:STDOUT: package: = namespace {.F = %lazy_import_ref} [template] +// CHECK:STDOUT: %lazy_import_ref = lazy_import_ref ir1, inst+1 +// CHECK:STDOUT: } +// CHECK:STDOUT: diff --git a/toolchain/check/testdata/function/definition/import.carbon b/toolchain/check/testdata/function/definition/import.carbon new file mode 100644 index 000000000000..19abe6c408ac --- /dev/null +++ b/toolchain/check/testdata/function/definition/import.carbon @@ -0,0 +1,39 @@ +// 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 + +// --- a.carbon + +library "a" api; + +fn F() {} + +// --- b.carbon + +library "b" api; + +import library "a"; + +// TODO: When ready, consider tests of basic import functionality. + +// CHECK:STDOUT: --- a.carbon +// CHECK:STDOUT: +// CHECK:STDOUT: file { +// CHECK:STDOUT: package: = namespace {.F = %F} [template] +// 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: --- b.carbon +// CHECK:STDOUT: +// CHECK:STDOUT: file { +// CHECK:STDOUT: package: = namespace {.F = %lazy_import_ref} [template] +// CHECK:STDOUT: %lazy_import_ref = lazy_import_ref ir1, inst+1 +// CHECK:STDOUT: } +// CHECK:STDOUT: diff --git a/toolchain/check/testdata/interface/import.carbon b/toolchain/check/testdata/interface/import.carbon new file mode 100644 index 000000000000..e5ba9abfc8a7 --- /dev/null +++ b/toolchain/check/testdata/interface/import.carbon @@ -0,0 +1,58 @@ +// 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 + +// --- a.carbon + +library "a" api; + +interface Empty { +} + +interface ForwardDeclared; + +interface ForwardDeclared { + fn F(); +} + +// --- b.carbon + +library "b" api; + +import library "a"; + +// TODO: When ready, consider tests of basic import functionality. + +// CHECK:STDOUT: --- a.carbon +// CHECK:STDOUT: +// CHECK:STDOUT: file { +// CHECK:STDOUT: package: = namespace {.Empty = %Empty.decl, .ForwardDeclared = %ForwardDeclared.decl.loc7} [template] +// CHECK:STDOUT: %Empty.decl = interface_decl @Empty, () +// CHECK:STDOUT: %ForwardDeclared.decl.loc7 = interface_decl @ForwardDeclared, () +// CHECK:STDOUT: %ForwardDeclared.decl.loc9 = interface_decl @ForwardDeclared, () +// CHECK:STDOUT: } +// CHECK:STDOUT: +// CHECK:STDOUT: interface @Empty { +// CHECK:STDOUT: +// CHECK:STDOUT: !members: +// CHECK:STDOUT: } +// CHECK:STDOUT: +// CHECK:STDOUT: interface @ForwardDeclared { +// CHECK:STDOUT: %F: = fn_decl @F [template] +// CHECK:STDOUT: +// CHECK:STDOUT: !members: +// CHECK:STDOUT: .F = %F +// CHECK:STDOUT: } +// CHECK:STDOUT: +// CHECK:STDOUT: fn @F(); +// CHECK:STDOUT: +// CHECK:STDOUT: --- b.carbon +// CHECK:STDOUT: +// CHECK:STDOUT: file { +// CHECK:STDOUT: package: = namespace {.Empty = %lazy_import_ref.1, .ForwardDeclared = %lazy_import_ref.2} [template] +// CHECK:STDOUT: %lazy_import_ref.1 = lazy_import_ref ir1, inst+1 +// CHECK:STDOUT: %lazy_import_ref.2 = lazy_import_ref ir1, inst+2 +// CHECK:STDOUT: } +// CHECK:STDOUT: