From abe8ce6653cbceec3ef12967533a5edaba860693 Mon Sep 17 00:00:00 2001 From: Jon Ross-Perkins Date: Mon, 26 Feb 2024 14:55:22 -0800 Subject: [PATCH] Add support for importing interfaces. (#3726) Interface support is pretty skeletal so this may need additions later, but I think it's still worthwhile to fill in the necessary bits now. With this change, the expectation is then that everything we have right now which _can_ be imported, is supported for import (at least for the "current package, no overlap" case). --- toolchain/check/handle_class.cpp | 2 +- toolchain/check/handle_impl.cpp | 2 +- toolchain/check/handle_interface.cpp | 2 +- toolchain/check/import_ref.cpp | 108 +++++++++++++----- .../check/testdata/interface/import.carbon | 69 ++++++++++- 5 files changed, 148 insertions(+), 35 deletions(-) diff --git a/toolchain/check/handle_class.cpp b/toolchain/check/handle_class.cpp index fb28fe634a88..b54880fd1aae 100644 --- a/toolchain/check/handle_class.cpp +++ b/toolchain/check/handle_class.cpp @@ -132,7 +132,7 @@ auto HandleClassDefinitionStart(Context& context, auto& class_info = context.classes().Get(class_id); // Track that this declaration is the definition. - if (class_info.definition_id.is_valid()) { + if (class_info.is_defined()) { CARBON_DIAGNOSTIC(ClassRedefinition, Error, "Redefinition of class {0}.", SemIR::NameId); CARBON_DIAGNOSTIC(ClassPreviousDefinition, Note, diff --git a/toolchain/check/handle_impl.cpp b/toolchain/check/handle_impl.cpp index 4d6b86b26420..560f8fda6a38 100644 --- a/toolchain/check/handle_impl.cpp +++ b/toolchain/check/handle_impl.cpp @@ -230,7 +230,7 @@ auto HandleImplDefinitionStart(Context& context, auto [impl_id, impl_decl_id] = BuildImplDecl(context, parse_node); auto& impl_info = context.impls().Get(impl_id); - if (impl_info.definition_id.is_valid()) { + if (impl_info.is_defined()) { CARBON_DIAGNOSTIC(ImplRedefinition, Error, "Redefinition of `impl {0} as {1}`.", SemIR::TypeId, SemIR::TypeId); diff --git a/toolchain/check/handle_interface.cpp b/toolchain/check/handle_interface.cpp index 26b381645a2e..8e643c9d4a7f 100644 --- a/toolchain/check/handle_interface.cpp +++ b/toolchain/check/handle_interface.cpp @@ -107,7 +107,7 @@ auto HandleInterfaceDefinitionStart( auto& interface_info = context.interfaces().Get(interface_id); // Track that this declaration is the definition. - if (interface_info.definition_id.is_valid()) { + if (interface_info.is_defined()) { CARBON_DIAGNOSTIC(InterfaceRedefinition, Error, "Redefinition of interface {0}.", SemIR::NameId); CARBON_DIAGNOSTIC(InterfacePreviousDefinition, Note, diff --git a/toolchain/check/import_ref.cpp b/toolchain/check/import_ref.cpp index f15d9479b135..75c676652848 100644 --- a/toolchain/check/import_ref.cpp +++ b/toolchain/check/import_ref.cpp @@ -236,6 +236,24 @@ class ImportRefResolver { return import_name_id; } + // Adds ImportRefUnused entries for members of the imported scope, for name + // lookup. Returns the block used for the refs, primarily for textual IR + // formatting. + auto AddNameScopeImportRefs(const SemIR::NameScope& import_scope, + SemIR::NameScope& new_scope) + -> SemIR::InstBlockId { + // Push a block so that we can add scoped instructions to it. + context_.inst_block_stack().Push(); + for (auto [entry_name_id, entry_inst_id] : import_scope.names) { + auto ref_id = context_.AddPlaceholderInst( + SemIR::ImportRefUnused{import_ir_id_, entry_inst_id}); + CARBON_CHECK( + new_scope.names.insert({GetLocalNameId(entry_name_id), ref_id}) + .second); + } + return context_.inst_block_stack().Pop(); + } + // Tries to resolve the InstId, returning a constant when ready, or Invalid if // more has been added to the stack. A similar API is followed for all // following TryResolveTypedInst helper functions. @@ -280,6 +298,12 @@ class ImportRefResolver { case SemIR::InstKind::FunctionDecl: return TryResolveTypedInst(inst.As()); + case SemIR::InstKind::InterfaceDecl: + return TryResolveTypedInst(inst.As()); + + case SemIR::InstKind::InterfaceType: + return TryResolveTypedInst(inst.As()); + case SemIR::InstKind::PointerType: return TryResolveTypedInst(inst.As()); @@ -298,10 +322,6 @@ class ImportRefResolver { // `inst`. return TryEvalInst(context_, inst_id, inst); - case SemIR::InstKind::InterfaceDecl: - // TODO: Not implemented. - return SemIR::ConstantId::Error; - default: context_.TODO( Parse::NodeId::Invalid, @@ -358,7 +378,7 @@ class ImportRefResolver { .inheritance_kind = import_class.inheritance_kind, }); - // Write the function ID into the ClassDecl. + // Write the class ID into the ClassDecl. context_.ReplaceInstBeforeConstantUse(class_decl_id, {Parse::NodeId::Invalid, class_decl}); auto self_const_id = context_.constant_values().Get(class_decl_id); @@ -389,19 +409,9 @@ class ImportRefResolver { context_.name_scopes().Add(new_class.decl_id, SemIR::NameId::Invalid, new_class.enclosing_scope_id); auto& new_scope = context_.name_scopes().Get(new_class.scope_id); - const auto& old_scope = import_ir_.name_scopes().Get(import_class.scope_id); - // Push a block so that we can add scoped instructions to it, primarily for - // textual IR formatting. - context_.inst_block_stack().Push(); - for (auto [entry_name_id, entry_inst_id] : old_scope.names) { - CARBON_CHECK( - new_scope.names - .insert({GetLocalNameId(entry_name_id), - context_.AddPlaceholderInst(SemIR::ImportRefUnused{ - import_ir_id_, entry_inst_id})}) - .second); - } - new_class.body_block_id = context_.inst_block_stack().Pop(); + const auto& import_scope = + import_ir_.name_scopes().Get(import_class.scope_id); + new_class.body_block_id = AddNameScopeImportRefs(import_scope, new_scope); if (import_class.base_id.is_valid()) { new_class.base_id = base_const_id.inst_id(); @@ -415,7 +425,7 @@ class ImportRefResolver { new_scope.extended_scopes.push_back(base_class.scope_id); } CARBON_CHECK(new_scope.extended_scopes.size() == - old_scope.extended_scopes.size()); + import_scope.extended_scopes.size()); } auto TryResolveTypedInst(SemIR::ClassDecl inst, SemIR::InstId inst_id, @@ -428,7 +438,7 @@ class ImportRefResolver { if (!made_incomplete_type) { class_const_id = MakeIncompleteClass(inst_id, import_class); // If there's only a forward declaration, we're done. - if (!import_class.object_repr_id.is_valid()) { + if (!import_class.is_defined()) { return class_const_id; } // This may not be needed because all constants might be ready, but we do @@ -437,7 +447,7 @@ class ImportRefResolver { work_stack_.back().made_incomplete_type = true; } - CARBON_CHECK(import_class.object_repr_id.is_valid()) + CARBON_CHECK(import_class.is_defined()) << "Only reachable when there's a definition."; // Load constants for the definition. @@ -555,6 +565,56 @@ class ImportRefResolver { return context_.constant_values().Get(function_decl_id); } + auto TryResolveTypedInst(SemIR::InterfaceDecl inst) -> SemIR::ConstantId { + const auto& import_interface = + import_ir_.interfaces().Get(inst.interface_id); + + auto interface_decl = SemIR::InterfaceDecl{SemIR::TypeId::Invalid, + SemIR::InterfaceId::Invalid, + SemIR::InstBlockId::Empty}; + auto interface_decl_id = + context_.AddPlaceholderInst({Parse::NodeId::Invalid, interface_decl}); + + // Start with an incomplete interface. + SemIR::Interface new_interface = { + .name_id = GetLocalNameId(import_interface.name_id), + .enclosing_scope_id = NoEnclosingScopeForImports, + .decl_id = interface_decl_id, + }; + + // If the interface is defined, we can complete it immediately. No constants + // are required. Do this before adding it to interfaces. + if (import_interface.is_defined()) { + new_interface.scope_id = context_.name_scopes().Add( + new_interface.decl_id, SemIR::NameId::Invalid, + new_interface.enclosing_scope_id); + auto& new_scope = context_.name_scopes().Get(new_interface.scope_id); + const auto& import_scope = + import_ir_.name_scopes().Get(import_interface.scope_id); + new_interface.body_block_id = + AddNameScopeImportRefs(import_scope, new_scope); + CARBON_CHECK(import_scope.extended_scopes.empty()) + << "Interfaces don't currently have extended scopes to support."; + + new_interface.defined = true; + } + + // Write the interface ID into the InterfaceDecl. + interface_decl.interface_id = context_.interfaces().Add(new_interface); + context_.ReplaceInstBeforeConstantUse( + interface_decl_id, {Parse::NodeId::Invalid, interface_decl}); + + return context_.constant_values().Get(interface_decl_id); + } + + auto TryResolveTypedInst(SemIR::InterfaceType inst) -> SemIR::ConstantId { + CARBON_CHECK(inst.type_id == SemIR::TypeId::TypeType); + // InterfaceType uses a straight reference to the constant ID generated as + // part of pulling in the InterfaceDecl, so there's no need to phase logic. + return GetLocalConstantId( + import_ir_.interfaces().Get(inst.interface_id).decl_id); + } + auto TryResolveTypedInst(SemIR::PointerType inst) -> SemIR::ConstantId { auto initial_work = work_stack_.size(); CARBON_CHECK(inst.type_id == SemIR::TypeId::TypeType); @@ -665,12 +725,6 @@ auto TryResolveImportRefUnused(Context& context, SemIR::InstId inst_id) auto type_id = resolver.ResolveType(import_inst.type_id()); auto constant_id = resolver.Resolve(import_ref->inst_id); - // TODO: Once ClassDecl/InterfaceDecl are supported (no longer return Error), - // remove this. - if (constant_id == SemIR::ConstantId::Error) { - type_id = SemIR::TypeId::Error; - } - // Replace the ImportRefUnused instruction with an ImportRefUsed. This doesn't // use ReplaceInstBeforeConstantUse because it would trigger TryEvalInst, and // we're instead doing constant evaluation here in order to minimize recursion diff --git a/toolchain/check/testdata/interface/import.carbon b/toolchain/check/testdata/interface/import.carbon index b6c7f2926732..f6e0a6b4f0f5 100644 --- a/toolchain/check/testdata/interface/import.carbon +++ b/toolchain/check/testdata/interface/import.carbon @@ -17,26 +17,38 @@ interface ForwardDeclared { fn F(); } +var f_ref: {.f: ForwardDeclared}; + // --- b.carbon library "b" api; import library "a"; -// TODO: When ready, consider tests of basic import functionality. +fn UseEmpty(e: Empty) {} +fn UseForwardDeclared(f: ForwardDeclared) {} + +var f: ForwardDeclared* = &f_ref.f; // CHECK:STDOUT: --- a.carbon // CHECK:STDOUT: // CHECK:STDOUT: constants { // CHECK:STDOUT: %.1: type = interface_type @Empty [template] // CHECK:STDOUT: %.2: type = interface_type @ForwardDeclared [template] +// CHECK:STDOUT: %.3: type = struct_type {.f: ForwardDeclared} [template] +// CHECK:STDOUT: %.4: type = tuple_type () [template] +// CHECK:STDOUT: %.5: type = struct_type {.f: ()} [template] // CHECK:STDOUT: } // CHECK:STDOUT: // CHECK:STDOUT: file { -// CHECK:STDOUT: package: = namespace {.Empty = %Empty.decl, .ForwardDeclared = %ForwardDeclared.decl.loc7} [template] +// CHECK:STDOUT: package: = namespace {.Empty = %Empty.decl, .ForwardDeclared = %ForwardDeclared.decl.loc7, .f_ref = %f_ref} [template] // CHECK:STDOUT: %Empty.decl = interface_decl @Empty, () [template = constants.%.1] // CHECK:STDOUT: %ForwardDeclared.decl.loc7 = interface_decl @ForwardDeclared, () [template = constants.%.2] // CHECK:STDOUT: %ForwardDeclared.decl.loc9 = interface_decl @ForwardDeclared, () [template = constants.%.2] +// CHECK:STDOUT: %ForwardDeclared.ref: type = name_ref ForwardDeclared, %ForwardDeclared.decl.loc7 [template = constants.%.2] +// CHECK:STDOUT: %.loc13: type = struct_type {.f: ForwardDeclared} [template = constants.%.3] +// CHECK:STDOUT: %f_ref.var: ref {.f: ForwardDeclared} = var f_ref +// CHECK:STDOUT: %f_ref: ref {.f: ForwardDeclared} = bind_name f_ref, %f_ref.var // CHECK:STDOUT: } // CHECK:STDOUT: // CHECK:STDOUT: interface @Empty { @@ -55,9 +67,56 @@ import library "a"; // CHECK:STDOUT: // CHECK:STDOUT: --- b.carbon // CHECK:STDOUT: +// CHECK:STDOUT: constants { +// CHECK:STDOUT: %.1: type = interface_type @Empty [template] +// CHECK:STDOUT: %.2: type = tuple_type () [template] +// CHECK:STDOUT: %.3: type = interface_type @ForwardDeclared [template] +// CHECK:STDOUT: %.4: type = ptr_type ForwardDeclared [template] +// CHECK:STDOUT: %.5: type = struct_type {.f: ForwardDeclared} [template] +// CHECK:STDOUT: %.6: type = struct_type {.f: ()} [template] +// CHECK:STDOUT: } +// CHECK:STDOUT: // CHECK:STDOUT: file { -// CHECK:STDOUT: package: = namespace {.Empty = %import_ref.1, .ForwardDeclared = %import_ref.2} [template] -// CHECK:STDOUT: %import_ref.1 = import_ref ir1, inst+1, unused -// CHECK:STDOUT: %import_ref.2 = import_ref ir1, inst+3, unused +// CHECK:STDOUT: package: = namespace {.Empty = %import_ref.1, .ForwardDeclared = %import_ref.2, .f_ref = %import_ref.3, .UseEmpty = %UseEmpty, .UseForwardDeclared = %UseForwardDeclared, .f = %f} [template] +// CHECK:STDOUT: %import_ref.1: type = import_ref ir1, inst+1, used [template = constants.%.1] +// CHECK:STDOUT: %import_ref.2: type = import_ref ir1, inst+3, used [template = constants.%.3] +// CHECK:STDOUT: %import_ref.3: ref {.f: ForwardDeclared} = import_ref ir1, inst+16, used +// CHECK:STDOUT: %UseEmpty: = fn_decl @UseEmpty [template] +// CHECK:STDOUT: %UseForwardDeclared: = fn_decl @UseForwardDeclared [template] +// CHECK:STDOUT: %ForwardDeclared.ref: type = name_ref ForwardDeclared, %import_ref.2 [template = constants.%.3] +// CHECK:STDOUT: %.loc9: type = ptr_type ForwardDeclared [template = constants.%.4] +// CHECK:STDOUT: %f.var: ref ForwardDeclared* = var f +// CHECK:STDOUT: %f: ref ForwardDeclared* = bind_name f, %f.var +// CHECK:STDOUT: } +// CHECK:STDOUT: +// CHECK:STDOUT: interface @Empty { +// CHECK:STDOUT: +// CHECK:STDOUT: !members: +// CHECK:STDOUT: } +// CHECK:STDOUT: +// CHECK:STDOUT: interface @ForwardDeclared { +// CHECK:STDOUT: %import_ref = import_ref ir1, inst+6, unused +// CHECK:STDOUT: +// CHECK:STDOUT: !members: +// CHECK:STDOUT: .F = %import_ref +// CHECK:STDOUT: } +// CHECK:STDOUT: +// CHECK:STDOUT: fn @UseEmpty(%e: Empty) { +// CHECK:STDOUT: !entry: +// CHECK:STDOUT: return +// CHECK:STDOUT: } +// CHECK:STDOUT: +// CHECK:STDOUT: fn @UseForwardDeclared(%f: ForwardDeclared) { +// CHECK:STDOUT: !entry: +// CHECK:STDOUT: return +// CHECK:STDOUT: } +// CHECK:STDOUT: +// CHECK:STDOUT: fn @__global_init() { +// CHECK:STDOUT: !entry: +// CHECK:STDOUT: %f_ref.ref: ref {.f: ForwardDeclared} = name_ref f_ref, file.%import_ref.3 +// CHECK:STDOUT: %.loc9_33: ref ForwardDeclared = struct_access %f_ref.ref, element0 +// CHECK:STDOUT: %.loc9_27: ForwardDeclared* = addr_of %.loc9_33 +// CHECK:STDOUT: assign file.%f.var, %.loc9_27 +// CHECK:STDOUT: return // CHECK:STDOUT: } // CHECK:STDOUT: