Skip to content

Commit

Permalink
Allow checking to continue after 'impl as' outside class
Browse files Browse the repository at this point in the history 
Currently it returns false which just ends typechecking. Instead
handle the error state later and avoid firing overlapping diagnostics
in 'extend impl as'.
  • Loading branch information
@danakj
danakj committed Feb 12, 2025
1 parent d6ce8f1 commit 224290d
Show file tree
Hide file tree
Showing 7 changed files with 293 additions and 52 deletions.
8 changes: 4 additions & 4 deletions toolchain/check/handle_impl.cpp
View file
Original file line number Diff line number Diff line change
Expand Up @@ -104,7 +104,6 @@ auto HandleParseNode(Context& context, Parse::DefaultSelfImplAsId node_id)
"`impl as` can only be used in a class");
context.emitter().Emit(node_id, ImplAsOutsideClass);
self_type_id = SemIR::ErrorInst::SingletonTypeId;
return false;
}

// Build the implicit access to the enclosing `Self`.
Expand Down Expand Up @@ -146,14 +145,14 @@ static auto ExtendImpl(Context& context, Parse::NodeId extend_node,
DiagnoseExtendImplOutsideClass(context, node_id);
return false;
}
auto& parent_scope = context.name_scopes().Get(parent_scope_id);

// TODO: This is also valid in a mixin.
if (!TryAsClassScope(context, parent_scope_id)) {
DiagnoseExtendImplOutsideClass(context, node_id);
return false;
}

auto& parent_scope = context.name_scopes().Get(parent_scope_id);

if (params_node.has_value()) {
CARBON_DIAGNOSTIC(ExtendImplForall, Error,
"cannot `extend` a parameterized `impl`");
Expand Down Expand Up @@ -387,7 +386,8 @@ static auto BuildImplDecl(Context& context, Parse::AnyImplDeclId node_id,
context.ReplaceInstBeforeConstantUse(impl_decl_id, impl_decl);

// For an `extend impl` declaration, mark the impl as extending this `impl`.
if (introducer.modifier_set.HasAnyOf(KeywordModifierSet::Extend)) {
if (self_type_id != SemIR::ErrorInst::SingletonTypeId &&
introducer.modifier_set.HasAnyOf(KeywordModifierSet::Extend)) {
auto extend_node = introducer.modifier_node_id(ModifierOrder::Decl);
if (impl_info.generic_id.has_value()) {
SemIR::TypeId type_id = context.insts().Get(constraint_inst_id).type_id();
Expand Down
4 changes: 4 additions & 0 deletions toolchain/check/impl.cpp
View file
Original file line number Diff line number Diff line change
Expand Up @@ -84,6 +84,10 @@ auto ImplWitnessForDeclaration(Context& context, const SemIR::Impl& impl)
-> SemIR::InstId {
CARBON_CHECK(!impl.has_definition_started());

if (impl.self_id == SemIR::ErrorInst::SingletonInstId) {
// When 'impl as' is invalid, the self type is an error.
return SemIR::ErrorInst::SingletonInstId;
}
auto facet_type_id = context.GetTypeIdForTypeInst(impl.constraint_id);
if (facet_type_id == SemIR::ErrorInst::SingletonTypeId) {
return SemIR::ErrorInst::SingletonInstId;
Expand Down
17 changes: 9 additions & 8 deletions toolchain/check/impl_lookup.cpp
View file
Original file line number Diff line number Diff line change
Expand Up @@ -145,14 +145,11 @@ auto LookupImplWitness(Context& context, SemIR::LocId loc_id,
// constraint, the unique `interface` declaration.)

auto specific_id = SemIR::SpecificId::None;
// This check comes first to avoid deduction when the witness_id is missing.
// We use an error value to indicate an error during creation of the impl,
// such as a recursive impl which will cause deduction to recurse
// infinitely.
if (!impl.witness_id.has_value() ||
impl.witness_id == SemIR::ErrorInst::SingletonInstId) {
// TODO: Diagnose if the impl isn't defined yet?
return SemIR::InstId::None;
// This check comes first to avoid deduction with an invalid impl. We use an
// error value to indicate an error during creation of the impl, such as a
// recursive impl which will cause deduction to recurse infinitely.
if (impl.witness_id == SemIR::ErrorInst::SingletonInstId) {
continue;
}
if (impl.generic_id.has_value()) {
specific_id = DeduceImplArguments(context, loc_id, impl, type_const_id,
Expand All @@ -175,6 +172,10 @@ auto LookupImplWitness(Context& context, SemIR::LocId loc_id,
// the constraint's interfaces.
continue;
}
if (!impl.witness_id.has_value()) {
// TODO: Diagnose if the impl isn't defined yet?
return SemIR::InstId::None;
}
LoadImportRef(context, impl.witness_id);
if (specific_id.has_value()) {
// We need a definition of the specific `impl` so we can access its
Expand Down
60 changes: 54 additions & 6 deletions toolchain/check/testdata/function/declaration/no_prelude/name_poisoning.carbon
View file
Original file line number Diff line number Diff line change
Expand Up @@ -311,10 +311,21 @@ interface A {
class X {
extend impl as A {
fn F() { return; }
// CHECK:STDERR: fail_using_poisoned_name_in_impl_outside_class.carbon:[[@LINE+4]]:10: error: `impl as` can only be used in a class [ImplAsOutsideClass]
// CHECK:STDERR: fail_using_poisoned_name_in_impl_outside_class.carbon:[[@LINE+15]]:10: error: `impl as` can only be used in a class [ImplAsOutsideClass]
// CHECK:STDERR: impl as B {}
// CHECK:STDERR: ^~
// CHECK:STDERR:
// CHECK:STDERR: fail_using_poisoned_name_in_impl_outside_class.carbon:[[@LINE+11]]:13: error: `Core.ImplicitAs` implicitly referenced here, but package `Core` not found [CoreNotFound]
// CHECK:STDERR: impl as B {}
// CHECK:STDERR: ^
// CHECK:STDERR:
// CHECK:STDERR: fail_using_poisoned_name_in_impl_outside_class.carbon:[[@LINE-10]]:3: error: missing implementation of B in impl of interface A [ImplMissingFunction]
// CHECK:STDERR: extend impl as A {
// CHECK:STDERR: ^~~~~~~~~~~~~~~~~~
// CHECK:STDERR: fail_using_poisoned_name_in_impl_outside_class.carbon:[[@LINE-16]]:3: note: associated function B declared here [AssociatedFunctionHere]
// CHECK:STDERR: fn B();
// CHECK:STDERR: ^~~~~~~
// CHECK:STDERR:
impl as B {}
}
}
Expand Down Expand Up @@ -1189,9 +1200,21 @@ fn F() {
// CHECK:STDOUT: %A.assoc_type: type = assoc_entity_type %A.type [template]
// CHECK:STDOUT: %assoc0: %A.assoc_type = assoc_entity element0, @A.%B.decl [template]
// CHECK:STDOUT: %X: type = class_type @X [template]
// CHECK:STDOUT: %impl_witness: <witness> = impl_witness (<error>) [template]
// CHECK:STDOUT: %F.type: type = fn_type @F [template]
// CHECK:STDOUT: %F: %F.type = struct_value () [template]
// CHECK:STDOUT: %empty_struct_type: type = struct_type {} [template]
// CHECK:STDOUT: %complete_type: <witness> = complete_type_witness %empty_struct_type [template]
// CHECK:STDOUT: }
// CHECK:STDOUT:
// CHECK:STDOUT: file {}
// CHECK:STDOUT: file {
// CHECK:STDOUT: package: <namespace> = namespace [template] {
// CHECK:STDOUT: .A = %A.decl
// CHECK:STDOUT: .X = %X.decl
// CHECK:STDOUT: }
// CHECK:STDOUT: %A.decl: type = interface_decl @A [template = constants.%A.type] {} {}
// CHECK:STDOUT: %X.decl: type = class_decl @X [template = constants.%X] {} {}
// CHECK:STDOUT: }
// CHECK:STDOUT:
// CHECK:STDOUT: interface @A {
// CHECK:STDOUT: %Self: %A.type = bind_symbolic_name Self, 0 [symbolic = constants.%Self]
Expand All @@ -1204,21 +1227,46 @@ fn F() {
// CHECK:STDOUT: witness = (%B.decl)
// CHECK:STDOUT: }
// CHECK:STDOUT:
// CHECK:STDOUT: impl @impl: <unexpected>.inst26.loc8_15 as <unexpected>.inst27.loc8_18;
// CHECK:STDOUT: impl @impl.1: %Self.ref as %A.ref {
// CHECK:STDOUT: %F.decl: %F.type = fn_decl @F [template = constants.%F] {} {}
// CHECK:STDOUT: %.loc25: type = converted @impl.2.%B.ref, <error> [template = <error>]
// CHECK:STDOUT: impl_decl @impl.2 [template] {} {
// CHECK:STDOUT: %Self.ref: type = name_ref Self, <error> [template = <error>]
// CHECK:STDOUT: %B.ref: %A.assoc_type = name_ref B, @A.%assoc0 [template = constants.%assoc0]
// CHECK:STDOUT: }
// CHECK:STDOUT:
// CHECK:STDOUT: !members:
// CHECK:STDOUT: .F = %F.decl
// CHECK:STDOUT: witness = @X.%impl_witness
// CHECK:STDOUT: }
// CHECK:STDOUT:
// CHECK:STDOUT: impl @impl.2: %Self.ref as <error> {
// CHECK:STDOUT: !members:
// CHECK:STDOUT: witness = <error>
// CHECK:STDOUT: }
// CHECK:STDOUT:
// CHECK:STDOUT: class @X {
// CHECK:STDOUT: complete_type_witness = invalid
// CHECK:STDOUT: impl_decl @impl.1 [template] {} {
// CHECK:STDOUT: %Self.ref: type = name_ref Self, constants.%X [template = constants.%X]
// CHECK:STDOUT: %A.ref: type = name_ref A, file.%A.decl [template = constants.%A.type]
// CHECK:STDOUT: }
// CHECK:STDOUT: %impl_witness: <witness> = impl_witness (<error>) [template = constants.%impl_witness]
// CHECK:STDOUT: %complete_type: <witness> = complete_type_witness %empty_struct_type [template = constants.%complete_type]
// CHECK:STDOUT: complete_type_witness = %complete_type
// CHECK:STDOUT:
// CHECK:STDOUT: !members:
// CHECK:STDOUT: .Self = constants.%X
// CHECK:STDOUT: extend <unexpected>.inst27.loc8_18
// CHECK:STDOUT: extend @impl.1.%A.ref
// CHECK:STDOUT: }
// CHECK:STDOUT:
// CHECK:STDOUT: generic fn @B(@A.%Self: %A.type) {
// CHECK:STDOUT: fn();
// CHECK:STDOUT: }
// CHECK:STDOUT:
// CHECK:STDOUT: fn @F();
// CHECK:STDOUT: fn @F() {
// CHECK:STDOUT: !entry:
// CHECK:STDOUT: return
// CHECK:STDOUT: }
// CHECK:STDOUT:
// CHECK:STDOUT: specific @B(constants.%Self) {}
// CHECK:STDOUT:
Expand Down
107 changes: 94 additions & 13 deletions toolchain/check/testdata/impl/fail_impl_as_scope.carbon
View file
Original file line number Diff line number Diff line change
Expand Up @@ -47,17 +47,39 @@ fn Function() {
// CHECK:STDOUT: constants {
// CHECK:STDOUT: %Simple.type: type = facet_type <@Simple> [template]
// CHECK:STDOUT: %Self: %Simple.type = bind_symbolic_name Self, 0 [symbolic]
// CHECK:STDOUT: %F.type: type = fn_type @F [template]
// CHECK:STDOUT: %F: %F.type = struct_value () [template]
// CHECK:STDOUT: %F.type.e2e: type = fn_type @F.1 [template]
// CHECK:STDOUT: %F.df8: %F.type.e2e = struct_value () [template]
// CHECK:STDOUT: %Simple.assoc_type: type = assoc_entity_type %Simple.type [template]
// CHECK:STDOUT: %assoc0: %Simple.assoc_type = assoc_entity element0, @Simple.%F.decl [template]
// CHECK:STDOUT: %impl_witness: <witness> = impl_witness (@impl.%F.decl) [template]
// CHECK:STDOUT: %F.type.fe5: type = fn_type @F.2 [template]
// CHECK:STDOUT: %F.32d: %F.type.fe5 = struct_value () [template]
// CHECK:STDOUT: }
// CHECK:STDOUT:
// CHECK:STDOUT: file {}
// CHECK:STDOUT: imports {
// CHECK:STDOUT: %Core: <namespace> = namespace file.%Core.import, [template] {
// CHECK:STDOUT: import Core//prelude
// CHECK:STDOUT: import Core//prelude/...
// CHECK:STDOUT: }
// CHECK:STDOUT: }
// CHECK:STDOUT:
// CHECK:STDOUT: file {
// CHECK:STDOUT: package: <namespace> = namespace [template] {
// CHECK:STDOUT: .Core = imports.%Core
// CHECK:STDOUT: .Simple = %Simple.decl
// CHECK:STDOUT: }
// CHECK:STDOUT: %Core.import = import Core
// CHECK:STDOUT: %Simple.decl: type = interface_decl @Simple [template = constants.%Simple.type] {} {}
// CHECK:STDOUT: impl_decl @impl [template] {} {
// CHECK:STDOUT: %Self.ref: type = name_ref Self, <error> [template = <error>]
// CHECK:STDOUT: %Simple.ref: type = name_ref Simple, file.%Simple.decl [template = constants.%Simple.type]
// CHECK:STDOUT: }
// CHECK:STDOUT: %impl_witness: <witness> = impl_witness (@impl.%F.decl) [template = constants.%impl_witness]
// CHECK:STDOUT: }
// CHECK:STDOUT:
// CHECK:STDOUT: interface @Simple {
// CHECK:STDOUT: %Self: %Simple.type = bind_symbolic_name Self, 0 [symbolic = constants.%Self]
// CHECK:STDOUT: %F.decl: %F.type = fn_decl @F [template = constants.%F] {} {}
// CHECK:STDOUT: %F.decl: %F.type.e2e = fn_decl @F.1 [template = constants.%F.df8] {} {}
// CHECK:STDOUT: %assoc0: %Simple.assoc_type = assoc_entity element0, %F.decl [template = constants.%assoc0]
// CHECK:STDOUT:
// CHECK:STDOUT: !members:
Expand All @@ -66,28 +88,64 @@ fn Function() {
// CHECK:STDOUT: witness = (%F.decl)
// CHECK:STDOUT: }
// CHECK:STDOUT:
// CHECK:STDOUT: generic fn @F(@Simple.%Self: %Simple.type) {
// CHECK:STDOUT: impl @impl: %Self.ref as %Simple.ref {
// CHECK:STDOUT: %F.decl: %F.type.fe5 = fn_decl @F.2 [template = constants.%F.32d] {} {}
// CHECK:STDOUT:
// CHECK:STDOUT: !members:
// CHECK:STDOUT: .F = %F.decl
// CHECK:STDOUT: witness = file.%impl_witness
// CHECK:STDOUT: }
// CHECK:STDOUT:
// CHECK:STDOUT: generic fn @F.1(@Simple.%Self: %Simple.type) {
// CHECK:STDOUT: fn();
// CHECK:STDOUT: }
// CHECK:STDOUT:
// CHECK:STDOUT: specific @F(constants.%Self) {}
// CHECK:STDOUT: fn @F.2() {
// CHECK:STDOUT: !entry:
// CHECK:STDOUT: return
// CHECK:STDOUT: }
// CHECK:STDOUT:
// CHECK:STDOUT: specific @F.1(constants.%Self) {}
// CHECK:STDOUT:
// CHECK:STDOUT: specific @F.1(<error>) {}
// CHECK:STDOUT:
// CHECK:STDOUT: --- fail_function.carbon
// CHECK:STDOUT:
// CHECK:STDOUT: constants {
// CHECK:STDOUT: %Simple.type: type = facet_type <@Simple> [template]
// CHECK:STDOUT: %Self: %Simple.type = bind_symbolic_name Self, 0 [symbolic]
// CHECK:STDOUT: %F.type: type = fn_type @F [template]
// CHECK:STDOUT: %F: %F.type = struct_value () [template]
// CHECK:STDOUT: %F.type.e2e: type = fn_type @F.1 [template]
// CHECK:STDOUT: %F.df8: %F.type.e2e = struct_value () [template]
// CHECK:STDOUT: %Simple.assoc_type: type = assoc_entity_type %Simple.type [template]
// CHECK:STDOUT: %assoc0: %Simple.assoc_type = assoc_entity element0, @Simple.%F.decl [template]
// CHECK:STDOUT: %Function.type: type = fn_type @Function [template]
// CHECK:STDOUT: %Function: %Function.type = struct_value () [template]
// CHECK:STDOUT: %impl_witness: <witness> = impl_witness (@impl.%F.decl) [template]
// CHECK:STDOUT: %F.type.bc7: type = fn_type @F.2 [template]
// CHECK:STDOUT: %F.0a2: %F.type.bc7 = struct_value () [template]
// CHECK:STDOUT: }
// CHECK:STDOUT:
// CHECK:STDOUT: file {}
// CHECK:STDOUT: imports {
// CHECK:STDOUT: %Core: <namespace> = namespace file.%Core.import, [template] {
// CHECK:STDOUT: import Core//prelude
// CHECK:STDOUT: import Core//prelude/...
// CHECK:STDOUT: }
// CHECK:STDOUT: }
// CHECK:STDOUT:
// CHECK:STDOUT: file {
// CHECK:STDOUT: package: <namespace> = namespace [template] {
// CHECK:STDOUT: .Core = imports.%Core
// CHECK:STDOUT: .Simple = %Simple.decl
// CHECK:STDOUT: .Function = %Function.decl
// CHECK:STDOUT: }
// CHECK:STDOUT: %Core.import = import Core
// CHECK:STDOUT: %Simple.decl: type = interface_decl @Simple [template = constants.%Simple.type] {} {}
// CHECK:STDOUT: %Function.decl: %Function.type = fn_decl @Function [template = constants.%Function] {} {}
// CHECK:STDOUT: }
// CHECK:STDOUT:
// CHECK:STDOUT: interface @Simple {
// CHECK:STDOUT: %Self: %Simple.type = bind_symbolic_name Self, 0 [symbolic = constants.%Self]
// CHECK:STDOUT: %F.decl: %F.type = fn_decl @F [template = constants.%F] {} {}
// CHECK:STDOUT: %F.decl: %F.type.e2e = fn_decl @F.1 [template = constants.%F.df8] {} {}
// CHECK:STDOUT: %assoc0: %Simple.assoc_type = assoc_entity element0, %F.decl [template = constants.%assoc0]
// CHECK:STDOUT:
// CHECK:STDOUT: !members:
Expand All @@ -96,11 +154,34 @@ fn Function() {
// CHECK:STDOUT: witness = (%F.decl)
// CHECK:STDOUT: }
// CHECK:STDOUT:
// CHECK:STDOUT: generic fn @F(@Simple.%Self: %Simple.type) {
// CHECK:STDOUT: impl @impl: %Self.ref as %Simple.ref {
// CHECK:STDOUT: %F.decl: %F.type.bc7 = fn_decl @F.2 [template = constants.%F.0a2] {} {}
// CHECK:STDOUT:
// CHECK:STDOUT: !members:
// CHECK:STDOUT: .F = %F.decl
// CHECK:STDOUT: witness = @Function.%impl_witness
// CHECK:STDOUT: }
// CHECK:STDOUT:
// CHECK:STDOUT: generic fn @F.1(@Simple.%Self: %Simple.type) {
// CHECK:STDOUT: fn();
// CHECK:STDOUT: }
// CHECK:STDOUT:
// CHECK:STDOUT: fn @Function();
// CHECK:STDOUT: fn @Function() {
// CHECK:STDOUT: !entry:
// CHECK:STDOUT: impl_decl @impl [template] {} {
// CHECK:STDOUT: %Self.ref: type = name_ref Self, <error> [template = <error>]
// CHECK:STDOUT: %Simple.ref: type = name_ref Simple, file.%Simple.decl [template = constants.%Simple.type]
// CHECK:STDOUT: }
// CHECK:STDOUT: %impl_witness: <witness> = impl_witness (@impl.%F.decl) [template = constants.%impl_witness]
// CHECK:STDOUT: return
// CHECK:STDOUT: }
// CHECK:STDOUT:
// CHECK:STDOUT: fn @F.2() {
// CHECK:STDOUT: !entry:
// CHECK:STDOUT: return
// CHECK:STDOUT: }
// CHECK:STDOUT:
// CHECK:STDOUT: specific @F.1(constants.%Self) {}
// CHECK:STDOUT:
// CHECK:STDOUT: specific @F(constants.%Self) {}
// CHECK:STDOUT: specific @F.1(<error>) {}
// CHECK:STDOUT:
Loading

0 comments on commit 224290d

Please sign in to comment.