From 81c2b3be1a9bbc491926cc48f816d4bd9e78f3e1 Mon Sep 17 00:00:00 2001 From: Jon Ross-Perkins Date: Fri, 3 Oct 2025 07:31:45 -0700 Subject: [PATCH] Handle some more errors in interfaces without crashing. (#6155) This fixes and tests two crashes related to what I was observing [on #toolchain](https://discord.com/channels/655572317891461132/655578254970716160/1422723976483831848). The approach to Cpp imports taken in #6086 is problematic because it returns before the work stack is completed, and doesn't store the resulting constants. This fixes the approach taken in that PR. --- toolchain/check/import_ref.cpp | 14 +- toolchain/check/interface.cpp | 18 ++- .../impl/using_invalid_interface.carbon | 124 ++++++++++++++++++ 3 files changed, 146 insertions(+), 10 deletions(-) create mode 100644 toolchain/check/testdata/impl/using_invalid_interface.carbon diff --git a/toolchain/check/import_ref.cpp b/toolchain/check/import_ref.cpp index 8c4225d46206..0b66e37d12c5 100644 --- a/toolchain/check/import_ref.cpp +++ b/toolchain/check/import_ref.cpp @@ -450,9 +450,6 @@ class ImportRefResolver : public ImportContext { // Step 1: check for a constant value. auto existing = FindResolvedConstId(work.inst_id); - if (existing.const_id == SemIR::ErrorInst::ConstantId) { - return existing.const_id; - } if (existing.const_id.has_value() && !work.retry_with_constant_value) { work_stack_.pop_back(); continue; @@ -592,7 +589,11 @@ class ImportRefResolver : public ImportContext { if (ir_inst.ir_id() == SemIR::ImportIRId::Cpp) { local_context().TODO(SemIR::LocId::None, "Unsupported: Importing C++ indirectly"); - return {.const_id = SemIR::ErrorInst::ConstantId}; + SetResolvedConstId(inst_id, result.indirect_insts, + SemIR::ErrorInst::ConstantId); + result.const_id = SemIR::ErrorInst::ConstantId; + result.indirect_insts.clear(); + return result; } const auto* prev_ir = cursor_ir; @@ -1794,6 +1795,11 @@ static auto TryResolveTypedInst(ImportRefResolver& resolver, auto class_const_id = GetLocalConstantId( resolver, resolver.import_classes().Get(inst.class_id).first_owning_decl_id); + if (class_const_id == SemIR::ErrorInst::ConstantId) { + // TODO: It should be possible to remove this once C++ imports work. + return ResolveResult::Done(class_const_id); + } + auto specific_data = GetLocalSpecificData(resolver, inst.specific_id); if (resolver.HasNewWork()) { return ResolveResult::Retry(); diff --git a/toolchain/check/interface.cpp b/toolchain/check/interface.cpp index 09e1eca75869..2b73a056fb51 100644 --- a/toolchain/check/interface.cpp +++ b/toolchain/check/interface.cpp @@ -181,8 +181,13 @@ auto GetTypeForSpecificAssociatedEntity(Context& context, SemIR::LocId loc_id, SemIR::TypeId self_type_id, SemIR::InstId self_witness_id) -> SemIR::TypeId { - auto decl = - context.insts().Get(context.constant_values().GetConstantInstId(decl_id)); + auto decl_constant_inst_id = + context.constant_values().GetConstantInstId(decl_id); + if (decl_constant_inst_id == SemIR::ErrorInst::InstId) { + return SemIR::ErrorInst::TypeId; + } + + auto decl = context.insts().Get(decl_constant_inst_id); if (auto assoc_const = decl.TryAs()) { // Form a specific for the associated constant, and grab the type from // there. @@ -195,8 +200,9 @@ auto GetTypeForSpecificAssociatedEntity(Context& context, SemIR::LocId loc_id, auto const_specific_id = MakeSpecific(context, loc_id, generic_id, arg_ids); return SemIR::GetTypeOfInstInSpecific(context.sem_ir(), const_specific_id, decl_id); - } else if (auto fn = context.types().TryGetAs( - decl.type_id())) { + } + + if (auto fn = context.types().TryGetAs(decl.type_id())) { // Form the type of the function within the interface, and attach the `Self` // type. auto interface_fn_type_id = SemIR::GetTypeOfInstInSpecific( @@ -208,9 +214,9 @@ auto GetTypeForSpecificAssociatedEntity(Context& context, SemIR::LocId loc_id, return GetFunctionTypeWithSelfType( context, context.types().GetInstId(interface_fn_type_id), self_facet_id); - } else { - CARBON_FATAL("Unexpected kind for associated constant {0}", decl); } + + CARBON_FATAL("Unexpected kind for associated constant {0}", decl); } } // namespace Carbon::Check diff --git a/toolchain/check/testdata/impl/using_invalid_interface.carbon b/toolchain/check/testdata/impl/using_invalid_interface.carbon new file mode 100644 index 000000000000..67000a125f49 --- /dev/null +++ b/toolchain/check/testdata/impl/using_invalid_interface.carbon @@ -0,0 +1,124 @@ +// 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 +// +// INCLUDE-FILE: toolchain/testing/testdata/min_prelude/convert.carbon +// +// AUTOUPDATE +// TIP: To test this file alone, run: +// TIP: bazel test //toolchain/testing:file_test --test_arg=--file_tests=toolchain/check/testdata/impl/using_invalid_interface.carbon +// TIP: To dump output, run: +// TIP: bazel run //toolchain/testing:file_test -- --dump_output --file_tests=toolchain/check/testdata/impl/using_invalid_interface.carbon + +// --- fail_invalid_interface.carbon +library "[[@TEST_NAME]]"; + +interface I {} + +class ArgToI(T:! I) {} + +interface IMisuse { + let Arg:! type; + // CHECK:STDERR: fail_invalid_interface.carbon:[[@LINE+7]]:26: error: cannot convert type `Arg` into type implementing `I` [ConversionFailureTypeToFacet] + // CHECK:STDERR: fn Op[self: Self]() -> ArgToI(Arg); + // CHECK:STDERR: ^~~~~~~~~~~ + // CHECK:STDERR: fail_invalid_interface.carbon:[[@LINE-7]]:14: note: initializing generic parameter `T` declared here [InitializingGenericParam] + // CHECK:STDERR: class ArgToI(T:! I) {} + // CHECK:STDERR: ^ + // CHECK:STDERR: + fn Op[self: Self]() -> ArgToI(Arg); +} + +// --- fail_misusing_invalid_interface.carbon +library "[[@TEST_NAME]]"; +// This import boundary is significant. +import library "invalid_interface"; + +class C { + // CHECK:STDERR: fail_misusing_invalid_interface.carbon:[[@LINE+4]]:32: error: name `T` not found [NameNotFound] + // CHECK:STDERR: impl as IMisuse where .Arg = T { + // CHECK:STDERR: ^ + // CHECK:STDERR: + impl as IMisuse where .Arg = T { + // CHECK:STDERR: fail_misusing_invalid_interface.carbon:[[@LINE+4]]:35: error: name `T` not found [NameNotFound] + // CHECK:STDERR: fn Op[self: Self]() -> ArgToI(T) { + // CHECK:STDERR: ^ + // CHECK:STDERR: + fn Op[self: Self]() -> ArgToI(T) { + return {}; + } + } +} + +// CHECK:STDERR: fail_misusing_invalid_interface.carbon:[[@LINE+8]]:21: error: missing object argument in method call [MissingObjectInMethodCall] +// CHECK:STDERR: fn F(T:! IMisuse) { T.Op(); } +// CHECK:STDERR: ^~~~~~ +// CHECK:STDERR: fail_misusing_invalid_interface.carbon:[[@LINE-21]]:1: in import [InImport] +// CHECK:STDERR: fail_invalid_interface.carbon:16:3: note: calling function declared here [InCallToFunction] +// CHECK:STDERR: fn Op[self: Self]() -> ArgToI(Arg); +// CHECK:STDERR: ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~ +// CHECK:STDERR: +fn F(T:! IMisuse) { T.Op(); } + +fn G() { + var c: C; + // CHECK:STDERR: fail_misusing_invalid_interface.carbon:[[@LINE+10]]:3: error: cannot implicitly convert non-type value of type `C` into type implementing `IMisuse` [ConversionFailureNonTypeToFacet] + // CHECK:STDERR: F(c); + // CHECK:STDERR: ^~~~ + // CHECK:STDERR: fail_misusing_invalid_interface.carbon:[[@LINE+7]]:3: note: type `C` does not implement interface `Core.ImplicitAs(IMisuse)` [MissingImplInMemberAccessNote] + // CHECK:STDERR: F(c); + // CHECK:STDERR: ^~~~ + // CHECK:STDERR: fail_misusing_invalid_interface.carbon:[[@LINE-10]]:6: note: initializing generic parameter `T` declared here [InitializingGenericParam] + // CHECK:STDERR: fn F(T:! IMisuse) { T.Op(); } + // CHECK:STDERR: ^ + // CHECK:STDERR: + F(c); +} + +// --- fail_using_invalid_interface.carbon +library "[[@TEST_NAME]]"; +// This import boundary is significant. +import library "invalid_interface"; + +class ArgC {} + +class C { + impl as IMisuse where .Arg = ArgC { + // CHECK:STDERR: fail_using_invalid_interface.carbon:[[@LINE+8]]:28: error: cannot convert type `ArgC` into type implementing `I` [ConversionFailureTypeToFacet] + // CHECK:STDERR: fn Op[self: Self]() -> ArgToI(ArgC) { + // CHECK:STDERR: ^~~~~~~~~~~~ + // CHECK:STDERR: fail_using_invalid_interface.carbon:[[@LINE-9]]:1: in import [InImport] + // CHECK:STDERR: fail_invalid_interface.carbon:5:14: note: initializing generic parameter `T` declared here [InitializingGenericParam] + // CHECK:STDERR: class ArgToI(T:! I) {} + // CHECK:STDERR: ^ + // CHECK:STDERR: + fn Op[self: Self]() -> ArgToI(ArgC) { + return {}; + } + } +} + +// CHECK:STDERR: fail_using_invalid_interface.carbon:[[@LINE+8]]:21: error: missing object argument in method call [MissingObjectInMethodCall] +// CHECK:STDERR: fn F(T:! IMisuse) { T.Op(); } +// CHECK:STDERR: ^~~~~~ +// CHECK:STDERR: fail_using_invalid_interface.carbon:[[@LINE-23]]:1: in import [InImport] +// CHECK:STDERR: fail_invalid_interface.carbon:16:3: note: calling function declared here [InCallToFunction] +// CHECK:STDERR: fn Op[self: Self]() -> ArgToI(Arg); +// CHECK:STDERR: ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~ +// CHECK:STDERR: +fn F(T:! IMisuse) { T.Op(); } + +fn G() { + var c: C; + // CHECK:STDERR: fail_using_invalid_interface.carbon:[[@LINE+10]]:3: error: cannot implicitly convert non-type value of type `C` into type implementing `IMisuse` [ConversionFailureNonTypeToFacet] + // CHECK:STDERR: F(c); + // CHECK:STDERR: ^~~~ + // CHECK:STDERR: fail_using_invalid_interface.carbon:[[@LINE+7]]:3: note: type `C` does not implement interface `Core.ImplicitAs(IMisuse)` [MissingImplInMemberAccessNote] + // CHECK:STDERR: F(c); + // CHECK:STDERR: ^~~~ + // CHECK:STDERR: fail_using_invalid_interface.carbon:[[@LINE-10]]:6: note: initializing generic parameter `T` declared here [InitializingGenericParam] + // CHECK:STDERR: fn F(T:! IMisuse) { T.Op(); } + // CHECK:STDERR: ^ + // CHECK:STDERR: + F(c); +}