From eed33c3b8cfd81844cc5e1ebc746408fbb8dff64 Mon Sep 17 00:00:00 2001 From: Richard Smith Date: Thu, 22 Feb 2024 12:26:36 -0800 Subject: [PATCH] Redeclaration support for `impl` declarations. (#3717) Maintain a mapping from (self type, constraint) to `ImplId` on the `Impl` value store so that we can perform redeclaration lookup. --- toolchain/check/context.h | 2 +- toolchain/check/handle_impl.cpp | 20 ++--- .../testdata/impl/fail_redefinition.carbon | 41 +++++++++ ...definition.carbon => redeclaration.carbon} | 25 ++++-- toolchain/sem_ir/BUILD | 1 + toolchain/sem_ir/file.h | 40 +-------- toolchain/sem_ir/ids.h | 3 + toolchain/sem_ir/impl.h | 83 +++++++++++++++++++ 8 files changed, 157 insertions(+), 58 deletions(-) create mode 100644 toolchain/check/testdata/impl/fail_redefinition.carbon rename toolchain/check/testdata/impl/{todo_redefinition.carbon => redeclaration.carbon} (60%) create mode 100644 toolchain/sem_ir/impl.h diff --git a/toolchain/check/context.h b/toolchain/check/context.h index 384a56f53ba2..a00268b2cb56 100644 --- a/toolchain/check/context.h +++ b/toolchain/check/context.h @@ -365,7 +365,7 @@ class Context { auto interfaces() -> ValueStore& { return sem_ir().interfaces(); } - auto impls() -> ValueStore& { return sem_ir().impls(); } + auto impls() -> SemIR::ImplStore& { return sem_ir().impls(); } auto import_irs() -> ValueStore& { return sem_ir().import_irs(); } diff --git a/toolchain/check/handle_impl.cpp b/toolchain/check/handle_impl.cpp index 8895891083fc..d57515a4b77b 100644 --- a/toolchain/check/handle_impl.cpp +++ b/toolchain/check/handle_impl.cpp @@ -153,21 +153,17 @@ static auto BuildImplDecl(Context& context, Parse::AnyImplDeclId parse_node) auto name_context = context.decl_name_stack().FinishImplName(); CARBON_CHECK(name_context.state == DeclNameStack::NameContext::State::Empty); - // Add the impl declaration. - auto impl_decl = SemIR::ImplDecl{SemIR::ImplId::Invalid, decl_block_id}; - auto impl_decl_id = context.AddPlaceholderInst({parse_node, impl_decl}); + // TODO: Check for an orphan `impl`. - // TODO: Check whether this is a redeclaration. + // TODO: Check parameters. Store them on the `Impl` in some form. static_cast(params_id); - // Create a new impl if this isn't a valid redeclaration. - if (!impl_decl.impl_id.is_valid()) { - impl_decl.impl_id = context.impls().Add( - {.self_id = self_type_id, .constraint_id = constraint_type_id}); - } - - // Write the impl ID into the ImplDecl. - context.ReplaceInstBeforeConstantUse(impl_decl_id, {parse_node, impl_decl}); + // Add the impl declaration. + // TODO: Does lookup in an impl file need to look for a prior impl declaration + // in the api file? + auto impl_id = context.impls().LookupOrAdd(self_type_id, constraint_type_id); + auto impl_decl = SemIR::ImplDecl{impl_id, decl_block_id}; + auto impl_decl_id = context.AddInst({parse_node, impl_decl}); // For an `extend impl` declaration, mark the impl as extending this `impl`. if (!!(context.decl_state_stack().innermost().modifier_set & diff --git a/toolchain/check/testdata/impl/fail_redefinition.carbon b/toolchain/check/testdata/impl/fail_redefinition.carbon new file mode 100644 index 000000000000..78e2d75384c0 --- /dev/null +++ b/toolchain/check/testdata/impl/fail_redefinition.carbon @@ -0,0 +1,41 @@ +// 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 + +interface I {} + +impl i32 as I {} + +// CHECK:STDERR: fail_redefinition.carbon:[[@LINE+6]]:1: ERROR: Redefinition of `impl i32 as I`. +// CHECK:STDERR: impl i32 as I {} +// CHECK:STDERR: ^~~~~~~~~~~~~~~ +// CHECK:STDERR: fail_redefinition.carbon:[[@LINE-5]]:1: Previous definition was here. +// CHECK:STDERR: impl i32 as I {} +// CHECK:STDERR: ^~~~~~~~~~~~~~~ +impl i32 as I {} + +// CHECK:STDOUT: --- fail_redefinition.carbon +// CHECK:STDOUT: +// CHECK:STDOUT: constants { +// CHECK:STDOUT: %.1: type = interface_type @I [template] +// CHECK:STDOUT: } +// CHECK:STDOUT: +// CHECK:STDOUT: file { +// CHECK:STDOUT: package: = namespace {.I = %I.decl} [template] +// CHECK:STDOUT: %I.decl = interface_decl @I, () +// CHECK:STDOUT: impl_decl @impl, () +// CHECK:STDOUT: impl_decl @impl, () +// CHECK:STDOUT: } +// CHECK:STDOUT: +// CHECK:STDOUT: interface @I { +// CHECK:STDOUT: +// CHECK:STDOUT: !members: +// CHECK:STDOUT: } +// CHECK:STDOUT: +// CHECK:STDOUT: impl @impl: i32 as I { +// CHECK:STDOUT: +// CHECK:STDOUT: !members: +// CHECK:STDOUT: } +// CHECK:STDOUT: diff --git a/toolchain/check/testdata/impl/todo_redefinition.carbon b/toolchain/check/testdata/impl/redeclaration.carbon similarity index 60% rename from toolchain/check/testdata/impl/todo_redefinition.carbon rename to toolchain/check/testdata/impl/redeclaration.carbon index 7d6072e13f47..386ac6810907 100644 --- a/toolchain/check/testdata/impl/todo_redefinition.carbon +++ b/toolchain/check/testdata/impl/redeclaration.carbon @@ -6,22 +6,28 @@ interface I {} +impl i32 as I; + +class X { + impl i32 as I; +} + impl i32 as I {} -// TODO: Reject this redefinition. -impl i32 as I {} - -// CHECK:STDOUT: --- todo_redefinition.carbon +// CHECK:STDOUT: --- redeclaration.carbon // CHECK:STDOUT: // CHECK:STDOUT: constants { // CHECK:STDOUT: %.1: type = interface_type @I [template] +// CHECK:STDOUT: %X: type = class_type @X [template] +// CHECK:STDOUT: %.2: type = struct_type {} [template] // CHECK:STDOUT: } // CHECK:STDOUT: // CHECK:STDOUT: file { -// CHECK:STDOUT: package: = namespace {.I = %I.decl} [template] +// CHECK:STDOUT: package: = namespace {.I = %I.decl, .X = %X.decl} [template] // CHECK:STDOUT: %I.decl = interface_decl @I, () -// CHECK:STDOUT: impl_decl @impl.1, () -// CHECK:STDOUT: impl_decl @impl.2, () +// CHECK:STDOUT: impl_decl @impl, () +// CHECK:STDOUT: %X.decl = class_decl @X, () +// CHECK:STDOUT: impl_decl @impl, () // CHECK:STDOUT: } // CHECK:STDOUT: // CHECK:STDOUT: interface @I { @@ -29,12 +35,13 @@ impl i32 as I {} // CHECK:STDOUT: !members: // CHECK:STDOUT: } // CHECK:STDOUT: -// CHECK:STDOUT: impl @impl.1: i32 as I { +// CHECK:STDOUT: impl @impl: i32 as I { // CHECK:STDOUT: // CHECK:STDOUT: !members: // CHECK:STDOUT: } // CHECK:STDOUT: -// CHECK:STDOUT: impl @impl.2: i32 as I { +// CHECK:STDOUT: class @X { +// CHECK:STDOUT: impl_decl @impl, () // CHECK:STDOUT: // CHECK:STDOUT: !members: // CHECK:STDOUT: } diff --git a/toolchain/sem_ir/BUILD b/toolchain/sem_ir/BUILD index b8450b1b4964..e0e7d61975a8 100644 --- a/toolchain/sem_ir/BUILD +++ b/toolchain/sem_ir/BUILD @@ -69,6 +69,7 @@ cc_library( ], hdrs = [ "file.h", + "impl.h", "value_stores.h", ], deps = [ diff --git a/toolchain/sem_ir/file.h b/toolchain/sem_ir/file.h index 62217d715731..2497fe128553 100644 --- a/toolchain/sem_ir/file.h +++ b/toolchain/sem_ir/file.h @@ -13,6 +13,7 @@ #include "toolchain/base/value_store.h" #include "toolchain/base/yaml.h" #include "toolchain/sem_ir/ids.h" +#include "toolchain/sem_ir/impl.h" #include "toolchain/sem_ir/type_info.h" #include "toolchain/sem_ir/value_stores.h" @@ -175,39 +176,6 @@ struct Interface : public Printable { bool defined = false; }; -// An implementation of a constraint. -struct Impl : public Printable { - auto Print(llvm::raw_ostream& out) const -> void { - out << "{self: " << self_id << ", constraint: " << constraint_id << "}"; - } - - // Determines whether this impl has been fully defined. This is false until we - // reach the `}` of the impl definition. - auto is_defined() const -> bool { return defined; } - - // The following members always have values, and do not change throughout the - // lifetime of the interface. - - // TODO: Track the generic parameters for `impl forall`. - // The type for which the impl is implementing a constraint. - TypeId self_id; - // The constraint that the impl implements. - TypeId constraint_id; - - // The following members are set at the `{` of the impl definition. - - // The definition of the impl. This is an ImplDecl. - InstId definition_id = InstId::Invalid; - // The impl scope. - NameScopeId scope_id = NameScopeId::Invalid; - // The first block of the impl body. - // TODO: Handle control flow in the impl body, such as if-expressions. - InstBlockId body_block_id = InstBlockId::Invalid; - - // The following members are set at the `}` of the impl definition. - bool defined = false; -}; - // Provides semantic analysis on a Parse::Tree. class File : public Printable { public: @@ -302,8 +270,8 @@ class File : public Printable { auto interfaces() const -> const ValueStore& { return interfaces_; } - auto impls() -> ValueStore& { return impls_; } - auto impls() const -> const ValueStore& { return impls_; } + auto impls() -> ImplStore& { return impls_; } + auto impls() const -> const ImplStore& { return impls_; } auto import_irs() -> ValueStore& { return import_irs_; } auto import_irs() const -> const ValueStore& { return import_irs_; @@ -378,7 +346,7 @@ class File : public Printable { ValueStore interfaces_; // Storage for impls. - ValueStore impls_; + ImplStore impls_; // Related IRs. There will always be at least one entry, the builtin IR (used // for references of builtins). diff --git a/toolchain/sem_ir/ids.h b/toolchain/sem_ir/ids.h index 202a17a5b3cb..8761d91f3dd5 100644 --- a/toolchain/sem_ir/ids.h +++ b/toolchain/sem_ir/ids.h @@ -487,5 +487,8 @@ struct llvm::DenseMapInfo template <> struct llvm::DenseMapInfo : public Carbon::IndexMapInfo {}; +template <> +struct llvm::DenseMapInfo + : public Carbon::IndexMapInfo {}; #endif // CARBON_TOOLCHAIN_SEM_IR_IDS_H_ diff --git a/toolchain/sem_ir/impl.h b/toolchain/sem_ir/impl.h new file mode 100644 index 000000000000..fa36a5f546b2 --- /dev/null +++ b/toolchain/sem_ir/impl.h @@ -0,0 +1,83 @@ +// 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 + +#ifndef CARBON_TOOLCHAIN_SEM_IR_IMPL_H_ +#define CARBON_TOOLCHAIN_SEM_IR_IMPL_H_ + +#include "llvm/ADT/DenseMap.h" +#include "toolchain/sem_ir/ids.h" + +namespace Carbon::SemIR { + +// An implementation of a constraint. +struct Impl : public Printable { + auto Print(llvm::raw_ostream& out) const -> void { + out << "{self: " << self_id << ", constraint: " << constraint_id << "}"; + } + + // Determines whether this impl has been fully defined. This is false until we + // reach the `}` of the impl definition. + auto is_defined() const -> bool { return defined; } + + // The following members always have values, and do not change throughout the + // lifetime of the interface. + + // TODO: Track the generic parameters for `impl forall`. + // The type for which the impl is implementing a constraint. + TypeId self_id; + // The constraint that the impl implements. + TypeId constraint_id; + + // The following members are set at the `{` of the impl definition. + + // The definition of the impl. This is an ImplDecl. + InstId definition_id = InstId::Invalid; + // The impl scope. + NameScopeId scope_id = NameScopeId::Invalid; + // The first block of the impl body. + // TODO: Handle control flow in the impl body, such as if-expressions. + InstBlockId body_block_id = InstBlockId::Invalid; + + // The following members are set at the `}` of the impl definition. + bool defined = false; +}; + +// A collection of `Impl`s, which can be accessed by the self type and +// constraint implemented. +class ImplStore { + public: + // Looks up the impl with this self type and constraint, or creates a new + // `Impl` if none exists. + // TODO: Handle parameters. + auto LookupOrAdd(TypeId self_id, TypeId constraint_id) -> ImplId { + auto [it, added] = + lookup_.insert({{self_id, constraint_id}, ImplId::Invalid}); + if (added) { + it->second = + values_.Add({.self_id = self_id, .constraint_id = constraint_id}); + } + return it->second; + } + + // Returns a mutable value for an ID. + auto Get(ImplId id) -> Impl& { return values_.Get(id); } + + // Returns the value for an ID. + auto Get(ImplId id) const -> const Impl& { return values_.Get(id); } + + auto OutputYaml() const -> Yaml::OutputMapping { + return values_.OutputYaml(); + } + + auto array_ref() const -> llvm::ArrayRef { return values_.array_ref(); } + auto size() const -> size_t { return values_.size(); } + + private: + ValueStore values_; + llvm::DenseMap, ImplId> lookup_; +}; + +} // namespace Carbon::SemIR + +#endif // CARBON_TOOLCHAIN_SEM_IR_IMPL_H_