carbon-language/carbon-lang
1
0
Fork 0
mirror of https://github.com/carbon-language/carbon-lang.git synced 2026-10-05 22:02:55 +01:00
Code Issues Packages Projects Releases 10 Wiki Activity

Defer RequireCompleteType to impl definition (Refactor Impl construction 7/7) (#6470)

Browse Source 
Explicitly run `RequireCompleteType` for an impl's facet type constraint
in two places:
- For a new `Impl` declaration that is `extend`
- At the start of the `Impl` definition

Stop trying to RequireCompleteType in the definition when constructing
the witness. If we have a rewrite of a name in `.Self`, then we can
construct a full witness, otherwise we defer to the definition.

Now GetOrAddImpl does not need to track `is_definition` anymore, so we
remove a lot of plumbing.

We inline the `AllocateFacetTypeImplWitness` since it has a single
caller and it is just 2 lines, to help improve understanding of the
steps and comments in setting up the impl definition.

Note that this puts the `RequreCompleteType` instruction into the
definition's generic eval block always, avoiding the issue of ensuring
that each generic redecl has the exact same instructions, and forcing
coordination to have `RequireCompleteType` inserted into every
declaration's eval block or none. The result also more closely matches
the design, with the complete type not being required until inside the
definition.

This is part of #6420 which is being split up into a chain of smaller
PRs. It is based on #6469.
This commit is contained in:
Dana Jansens
2025-12-11 23:50:10 +00:00
committed by GitHub
parent 463ba0e1db
commit fbcaf34494
56 changed files with 244 additions and 298 deletions
Download Patch File Download Diff File Expand all files Collapse all files
toolchain/check/impl.cpp
+55 -30
View File
@@ -86,8 +86,8 @@ auto CheckAssociatedFunctionImplementation(
}
// Builds an initial witness from the rewrites in the facet type, if any.
auto ImplWitnessForDeclaration(Context& context, const SemIR::Impl& impl,
bool has_definition) -> SemIR::InstId {
auto ImplWitnessForDeclaration(Context& context, const SemIR::Impl& impl)
-> SemIR::InstId {
CARBON_CHECK(!impl.has_definition_started());
auto self_type_id = context.types().GetTypeIdForTypeInstId(impl.self_id);
@@ -99,7 +99,7 @@ auto ImplWitnessForDeclaration(Context& context, const SemIR::Impl& impl,
return InitialFacetTypeImplWitness(
context, SemIR::LocId(impl.latest_decl_id()), impl.constraint_id,
impl.self_id, impl.interface,
context.generics().GetSelfSpecific(impl.generic_id), has_definition);
context.generics().GetSelfSpecific(impl.generic_id));
}
auto ImplWitnessStartDefinition(Context& context, SemIR::Impl& impl) -> void {
@@ -108,33 +108,53 @@ auto ImplWitnessStartDefinition(Context& context, SemIR::Impl& impl) -> void {
if (impl.witness_id == SemIR::ErrorInst::InstId) {
return;
}
if (!RequireCompleteType(
context, context.types().GetTypeIdForTypeInstId(impl.constraint_id),
SemIR::LocId(impl.constraint_id), [&] {
CARBON_DIAGNOSTIC(ImplAsIncompleteFacetTypeDefinition, Error,
"definition of impl as incomplete facet type {0}",
InstIdAsType);
return context.emitter().Build(SemIR::LocId(impl.latest_decl_id()),
ImplAsIncompleteFacetTypeDefinition,
impl.constraint_id);
})) {
FillImplWitnessWithErrors(context, impl);
return;
}
const auto& interface = context.interfaces().Get(impl.interface.interface_id);
auto assoc_entities =
context.inst_blocks().Get(interface.associated_entities_id);
for (auto decl_id : assoc_entities) {
LoadImportRef(context, decl_id);
}
auto witness = context.insts().GetAs<SemIR::ImplWitness>(impl.witness_id);
auto witness_table =
context.insts().GetAs<SemIR::ImplWitnessTable>(witness.witness_table_id);
auto witness_block =
context.inst_blocks().GetMutable(witness_table.elements_id);
// `witness_table.elements_id` will be `SemIR::InstBlockId::Empty` when the
// definition is the first declaration and the interface has no members. The
// other case where `witness_block` will be empty is when we are using a
// placeholder witness. This happens when there is a forward declaration of
// the impl and the facet type has no rewrite constraints and so it wasn't
// required to be complete.
if (witness_table.elements_id != SemIR::InstBlockId::Empty &&
witness_block.empty()) {
if (!RequireCompleteFacetTypeForImplDefinition(
context, SemIR::LocId(impl.latest_decl_id()), impl.constraint_id)) {
FillImplWitnessWithErrors(context, impl);
return;
}
AllocateFacetTypeImplWitness(context, impl.interface.interface_id,
witness_table.elements_id);
// The impl declaration may have created a placeholder witness table, or a
// full witness table. We can detect that the witness table is a placeholder
// table if it's not the `Empty` id, but it is empty still. If it was a
// placeholder, we can replace the placeholder here with a table of the proper
// size, since the interface must be complete for the impl definition.
bool witness_table_is_placeholder =
witness_table.elements_id != SemIR::InstBlockId::Empty &&
witness_block.empty();
if (witness_table_is_placeholder) {
// TODO: Since our `empty_table` repeats the same value throughout, we could
// skip an allocation here if there was a `ReplacePlaceholder` function that
// took a size and value instead of an array of values.
llvm::SmallVector<SemIR::InstId> empty_table(
assoc_entities.size(), SemIR::InstId::ImplWitnessTablePlaceholder);
context.inst_blocks().ReplacePlaceholder(witness_table.elements_id,
empty_table);
witness_block = context.inst_blocks().GetMutable(witness_table.elements_id);
}
const auto& interface = context.interfaces().Get(impl.interface.interface_id);
auto assoc_entities =
context.inst_blocks().Get(interface.associated_entities_id);
CARBON_CHECK(witness_block.size() == assoc_entities.size());
// Check we have a value for all non-function associated constants in the
// witness.
@@ -306,7 +326,8 @@ auto CheckConstraintIsInterface(Context& context, SemIR::InstId impl_decl_id,
}
// Returns true if impl redeclaration parameters match.
static auto CheckImplRedeclParamsMatch(Context& context, SemIR::Impl& new_impl,
static auto CheckImplRedeclParamsMatch(Context& context,
const SemIR::Impl& new_impl,
SemIR::ImplId prev_impl_id) -> bool {
auto& prev_impl = context.impls().Get(prev_impl_id);
@@ -324,7 +345,7 @@ static auto CheckImplRedeclParamsMatch(Context& context, SemIR::Impl& new_impl,
// Returns whether an impl can be redeclared. For example, defined impls
// cannot be redeclared.
static auto IsValidImplRedecl(Context& context, SemIR::Impl& new_impl,
static auto IsValidImplRedecl(Context& context, const SemIR::Impl& new_impl,
SemIR::ImplId prev_impl_id) -> bool {
auto& prev_impl = context.impls().Get(prev_impl_id);
@@ -488,11 +509,10 @@ static auto ApplyExtendImplAs(Context& context, SemIR::LocId loc_id,
auto GetOrAddImpl(Context& context, SemIR::LocId loc_id,
SemIR::LocId implicit_params_loc_id, SemIR::Impl impl,
bool is_definition, Parse::NodeId extend_node)
-> SemIR::ImplId {
Parse::NodeId extend_node) -> SemIR::ImplId {
auto impl_id = SemIR::ImplId::None;
// Add the impl declaration.
// Look for an existing matching declaration.
auto lookup_bucket_ref = context.impls().GetOrAddLookupBucket(impl);
// TODO: Detect two impl declarations with the same self type and interface,
// and issue an error if they don't match.
@@ -538,12 +558,17 @@ auto GetOrAddImpl(Context& context, SemIR::LocId loc_id,
}
if (impl.witness_id != SemIR::ErrorInst::InstId) {
impl.witness_id = ImplWitnessForDeclaration(context, impl, is_definition);
// This makes either a placeholder witness or a full witness table. The full
// witness table is deferred to the impl definition unless the declaration
// uses rewrite constraints to set values of associated constants in the
// interface.
impl.witness_id = ImplWitnessForDeclaration(context, impl);
}
FinishGenericDecl(context, SemIR::LocId(impl.latest_decl_id()),
impl.generic_id);
// From here on, use the `Impl` from the `ImplStore` instead of `impl`
// in order to make and see any changes to the `Impl`.
// From here on, use the `Impl` from the `ImplStore` instead of `impl` in
// order to make and see any changes to the `Impl`.
impl_id = context.impls().Add(impl);
lookup_bucket_ref.push_back(impl_id);
AssignImplIdInWitness(context, impl_id, impl.witness_id);