Skip to content

Commit

Permalink
Support associated constants in impl witnesses (#4770)
Browse files Browse the repository at this point in the history
With this change, we now support impl of interfaces with non-function
associated constants.

Also:
* Make impl diagnostics use more consistent names
* Make some impl tests "no_prelude"

Still to do:
* Facet type resolution as a separate, reusable step
* Using the assigned values of associated constants (see
`fail_todo_use_assoc_const.carbon`)

---------

Co-authored-by: Josh L <[email protected]>
Co-authored-by: Richard Smith <[email protected]>
  • Loading branch information
3 people authored Jan 11, 2025
1 parent fb1a9ba commit 230a8ee
Show file tree
Hide file tree
Showing 20 changed files with 1,828 additions and 198 deletions.
4 changes: 2 additions & 2 deletions toolchain/check/check_unit.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -402,9 +402,9 @@ auto CheckUnit::CheckRequiredDefinitions() -> void {
auto& impl = context_.impls().Get(impl_decl.impl_id);
if (!impl.is_defined()) {
FillImplWitnessWithErrors(context_, impl);
CARBON_DIAGNOSTIC(MissingImplDefinition, Error,
CARBON_DIAGNOSTIC(ImplMissingDefinition, Error,
"impl declared but not defined");
emitter_.Emit(decl_inst_id, MissingImplDefinition);
emitter_.Emit(decl_inst_id, ImplMissingDefinition);
}
break;
}
Expand Down
11 changes: 8 additions & 3 deletions toolchain/check/handle_impl.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -321,7 +321,8 @@ static auto BuildImplDecl(Context& context, Parse::AnyImplDeclId node_id,
// TODO: Check that its constant value is a constraint.
auto [constraint_inst_id, constraint_type_id] =
ExprAsType(context, constraint_node, constraint_id);
// TODO: Do facet type resolution here.
// TODO: Do facet type resolution here, and enforce that the constraint
// extends a single interface.
// TODO: Determine `interface_id` and `specific_id` once and save it in the
// resolved facet type, instead of in multiple functions called below.
// TODO: Skip work below if facet type resolution fails, so we don't have a
Expand Down Expand Up @@ -373,12 +374,16 @@ static auto BuildImplDecl(Context& context, Parse::AnyImplDeclId node_id,
if (!impl_decl.impl_id.is_valid()) {
impl_info.generic_id = BuildGeneric(context, impl_decl_id);
impl_info.witness_id = ImplWitnessForDeclaration(context, impl_info);
AddConstantsToImplWitnessFromConstraint(
context, impl_info, impl_info.witness_id, impl_decl.impl_id);
FinishGenericDecl(context, impl_decl_id, impl_info.generic_id);
impl_decl.impl_id = context.impls().Add(impl_info);
lookup_bucket_ref.push_back(impl_decl.impl_id);
} else {
FinishGenericRedecl(context, impl_decl_id,
context.impls().Get(impl_decl.impl_id).generic_id);
const auto& first_impl = context.impls().Get(impl_decl.impl_id);
AddConstantsToImplWitnessFromConstraint(
context, impl_info, first_impl.witness_id, impl_decl.impl_id);
FinishGenericRedecl(context, impl_decl_id, first_impl.generic_id);
}

// Write the impl ID into the ImplDecl.
Expand Down
263 changes: 212 additions & 51 deletions toolchain/check/impl.cpp

Large diffs are not rendered by default.

6 changes: 5 additions & 1 deletion toolchain/check/impl.h
Original file line number Diff line number Diff line change
Expand Up @@ -14,7 +14,11 @@ namespace Carbon::Check {
auto ImplWitnessForDeclaration(Context& context, const SemIR::Impl& impl)
-> SemIR::InstId;

// TODO: AddConstantsToImplWitnessFromConstraint()
auto AddConstantsToImplWitnessFromConstraint(Context& context,
const SemIR::Impl& impl,
SemIR::InstId witness_id,
SemIR::ImplId prev_decl_id)
-> void;

// Update `impl`'s witness at the start of a definition.
auto ImplWitnessStartDefinition(Context& context, SemIR::Impl& impl) -> void;
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -37,7 +37,7 @@ class E {
// CHECK:STDERR: extend impl Self as I {}
// CHECK:STDERR: ^~~~
// CHECK:STDERR:
// CHECK:STDERR: fail_extend_impl_type_as.carbon:[[@LINE-11]]:3: error: impl declared but not defined [MissingImplDefinition]
// CHECK:STDERR: fail_extend_impl_type_as.carbon:[[@LINE-11]]:3: error: impl declared but not defined [ImplMissingDefinition]
// CHECK:STDERR: extend impl D as I;
// CHECK:STDERR: ^~~~~~~~~~~~~~~~~~~
extend impl Self as I {}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -17,7 +17,7 @@ class C {
// CHECK:STDERR: extend impl as i32;
// CHECK:STDERR: ^~~~~~~~~~~~~~~~~~~
// CHECK:STDERR:
// CHECK:STDERR: fail_extend_non_interface.carbon:[[@LINE+3]]:3: error: impl declared but not defined [MissingImplDefinition]
// CHECK:STDERR: fail_extend_non_interface.carbon:[[@LINE+3]]:3: error: impl declared but not defined [ImplMissingDefinition]
// CHECK:STDERR: extend impl as i32;
// CHECK:STDERR: ^~~~~~~~~~~~~~~~~~~
extend impl as i32;
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -17,7 +17,7 @@ interface I {
// CHECK:STDERR: interface I {
// CHECK:STDERR: ^~~~~~~~~~~~~
// CHECK:STDERR:
// CHECK:STDERR: fail_extend_partially_defined_interface.carbon:[[@LINE+3]]:5: error: impl declared but not defined [MissingImplDefinition]
// CHECK:STDERR: fail_extend_partially_defined_interface.carbon:[[@LINE+3]]:5: error: impl declared but not defined [ImplMissingDefinition]
// CHECK:STDERR: extend impl as I;
// CHECK:STDERR: ^~~~~~~~~~~~~~~~~
extend impl as I;
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -43,7 +43,7 @@ fn F(c: C) {
// CHECK:STDERR: extend impl as I;
// CHECK:STDERR: ^
// CHECK:STDERR:
// CHECK:STDERR: fail_extend_undefined_interface.carbon:[[@LINE-25]]:3: error: impl declared but not defined [MissingImplDefinition]
// CHECK:STDERR: fail_extend_undefined_interface.carbon:[[@LINE-25]]:3: error: impl declared but not defined [ImplMissingDefinition]
// CHECK:STDERR: extend impl as I;
// CHECK:STDERR: ^~~~~~~~~~~~~~~~~
c.F();
Expand Down
6 changes: 3 additions & 3 deletions toolchain/check/testdata/impl/fail_impl_bad_assoc_fn.carbon
Original file line number Diff line number Diff line change
Expand Up @@ -14,7 +14,7 @@ class NoF {
// CHECK:STDERR: fail_impl_bad_assoc_fn.carbon:[[@LINE+7]]:3: error: missing implementation of F in impl of interface I [ImplMissingFunction]
// CHECK:STDERR: impl as I {}
// CHECK:STDERR: ^~~~~~~~~~~
// CHECK:STDERR: fail_impl_bad_assoc_fn.carbon:[[@LINE-6]]:15: note: associated function F declared here [ImplAssociatedFunctionHere]
// CHECK:STDERR: fail_impl_bad_assoc_fn.carbon:[[@LINE-6]]:15: note: associated function F declared here [AssociatedFunctionHere]
// CHECK:STDERR: interface I { fn F(); }
// CHECK:STDERR: ^~~~~~~
// CHECK:STDERR:
Expand All @@ -26,7 +26,7 @@ class FNotFunction {
// CHECK:STDERR: fail_impl_bad_assoc_fn.carbon:[[@LINE+7]]:5: error: associated function F implemented by non-function [ImplFunctionWithNonFunction]
// CHECK:STDERR: class F;
// CHECK:STDERR: ^~~~~~~~
// CHECK:STDERR: fail_impl_bad_assoc_fn.carbon:[[@LINE-18]]:15: note: associated function F declared here [ImplAssociatedFunctionHere]
// CHECK:STDERR: fail_impl_bad_assoc_fn.carbon:[[@LINE-18]]:15: note: associated function F declared here [AssociatedFunctionHere]
// CHECK:STDERR: interface I { fn F(); }
// CHECK:STDERR: ^~~~~~~
// CHECK:STDERR:
Expand All @@ -42,7 +42,7 @@ class FAlias {
// CHECK:STDERR: fail_impl_bad_assoc_fn.carbon:[[@LINE+7]]:11: error: associated function F implemented by non-function [ImplFunctionWithNonFunction]
// CHECK:STDERR: alias F = PossiblyF;
// CHECK:STDERR: ^
// CHECK:STDERR: fail_impl_bad_assoc_fn.carbon:[[@LINE-34]]:15: note: associated function F declared here [ImplAssociatedFunctionHere]
// CHECK:STDERR: fail_impl_bad_assoc_fn.carbon:[[@LINE-34]]:15: note: associated function F declared here [AssociatedFunctionHere]
// CHECK:STDERR: interface I { fn F(); }
// CHECK:STDERR: ^~~~~~~
// CHECK:STDERR:
Expand Down
82 changes: 0 additions & 82 deletions toolchain/check/testdata/impl/fail_todo_impl_assoc_const.carbon

This file was deleted.

Loading

0 comments on commit 230a8ee

Please sign in to comment.