From 5aae6a1ca54fa4fd6a4123cb09e6dba9048deadd Mon Sep 17 00:00:00 2001 From: Dana Jansens Date: Thu, 25 Jun 2026 18:37:14 -0400 Subject: [PATCH] Ensure a location for monomorphization diagnostics in call argument deduction (#7401) If the deduction fails while forming the parameter type, ensure that we print an actual diagnostic saying what went wrong. And always ensure that an invalid array bounds error points at a location. The `inst_id` given to `EvalConstantInst` always has a location, but the `bounds_id` instruction inside it may be canonical when it's coming from inside a larger type. So when it is, fall back to using the location of the whole array inst. --- toolchain/check/deduce.cpp | 26 ++++++--- toolchain/check/eval_inst.cpp | 17 ++++-- .../identify_specific_facet_type.carbon | 56 +++++++++++++++++++ toolchain/diagnostics/kind.def | 1 + 4 files changed, 87 insertions(+), 13 deletions(-) diff --git a/toolchain/check/deduce.cpp b/toolchain/check/deduce.cpp index 9ae7c705ea93..ea0fc8dbc267 100644 --- a/toolchain/check/deduce.cpp +++ b/toolchain/check/deduce.cpp @@ -543,13 +543,6 @@ auto DeductionContext::CheckDeductionIsComplete() -> bool { // that incorrectly. auto binding_type_id = context().insts().Get(binding_id).type_id(); if (binding_type_id.is_symbolic()) { - auto param_type_const_id = - SubstConstant(context(), SemIR::LocId(binding_id), - binding_type_id.AsConstantId(), substitutions_); - CARBON_CHECK(param_type_const_id.has_value()); - binding_type_id = - context().types().GetTypeIdForTypeConstantId(param_type_const_id); - Diagnostics::AnnotationScope annotate_diagnostics( &context().emitter(), [&](auto& builder) { if (diagnose_) { @@ -557,6 +550,25 @@ auto DeductionContext::CheckDeductionIsComplete() -> bool { builder); } }); + + { + Diagnostics::ContextScope diag_context( + &context().emitter(), [&](auto& builder) { + CARBON_DIAGNOSTIC( + SubstitutingGenericParamType, Context, + "constructed invalid specific for {0} from argument", + SemIR::TypeId); + builder.Context(loc_id_, SubstitutingGenericParamType, + binding_type_id); + }); + auto param_type_const_id = + SubstConstant(context(), SemIR::LocId(binding_id), + binding_type_id.AsConstantId(), substitutions_); + CARBON_CHECK(param_type_const_id.has_value()); + binding_type_id = + context().types().GetTypeIdForTypeConstantId(param_type_const_id); + } + auto converted_arg_id = diagnose_ ? ConvertToValueOfType(context(), loc_id_, deduced_arg_id, binding_type_id) diff --git a/toolchain/check/eval_inst.cpp b/toolchain/check/eval_inst.cpp index 747689bef86f..8de144ec5b61 100644 --- a/toolchain/check/eval_inst.cpp +++ b/toolchain/check/eval_inst.cpp @@ -66,6 +66,13 @@ auto EvalConstantInst(Context& context, SemIR::InstId inst_id, "Unexpected inst {0} for template constant int", bound_inst); return ConstantEvalResult::NewSamePhase(inst); } + + auto orig_inst = context.insts().GetAs(inst_id); + auto error_loc = + context.insts().GetCanonicalLocId(orig_inst.bound_id).has_value() + ? orig_inst.bound_id + : inst_id; + // TODO: We should check that the size of the resulting array type // fits in 64 bits, not just that the bound does. Should we use a // 32-bit limit for 32-bit targets? @@ -74,17 +81,15 @@ auto EvalConstantInst(Context& context, SemIR::InstId inst_id, bound_val.isNegative()) { CARBON_DIAGNOSTIC(ArrayBoundNegative, Error, "array bound of {0} is negative", TypedInt); - context.emitter().Emit( - context.insts().GetAs(inst_id).bound_id, - ArrayBoundNegative, {.type = int_bound->type_id, .value = bound_val}); + context.emitter().Emit(error_loc, ArrayBoundNegative, + {.type = int_bound->type_id, .value = bound_val}); return ConstantEvalResult::Error; } if (bound_val.getActiveBits() > 64) { CARBON_DIAGNOSTIC(ArrayBoundTooLarge, Error, "array bound of {0} is too large", TypedInt); - context.emitter().Emit( - context.insts().GetAs(inst_id).bound_id, - ArrayBoundTooLarge, {.type = int_bound->type_id, .value = bound_val}); + context.emitter().Emit(error_loc, ArrayBoundTooLarge, + {.type = int_bound->type_id, .value = bound_val}); return ConstantEvalResult::Error; } return ConstantEvalResult::NewSamePhase(inst); diff --git a/toolchain/check/testdata/generic/identify_specific_facet_type.carbon b/toolchain/check/testdata/generic/identify_specific_facet_type.carbon index c3e56df788c0..c0165bdbb633 100644 --- a/toolchain/check/testdata/generic/identify_specific_facet_type.carbon +++ b/toolchain/check/testdata/generic/identify_specific_facet_type.carbon @@ -93,3 +93,59 @@ fn G(T:! Y) { // CHECK:STDERR: F(-1, T); }; + +// --- fail_monomorphization_identify_in_call.carbon +library "[[@TEST_NAME]]"; + +interface Y {} +interface Z { + let Z1:! type; +} + +constraint S(N:! Core.IntLiteral) { + extend require impls Z where .Z1 = array((), N); +} + +fn F(N:! Core.IntLiteral, _:! S(N)) {} + +fn G(T:! Z) { + // CHECK:STDERR: fail_monomorphization_identify_in_call.carbon:[[@LINE+10]]:3: error: facet type `S(-1)` can not be identified [ImplLookupInUnidentifiedFacetType] + // CHECK:STDERR: F(-1, T); + // CHECK:STDERR: ^~~~~~~~ + // CHECK:STDERR: fail_monomorphization_identify_in_call.carbon:[[@LINE-9]]:24: note: array bound of -1 is negative [ArrayBoundNegative] + // CHECK:STDERR: extend require impls Z where .Z1 = array((), N); + // CHECK:STDERR: ^~~~~~~~~~~~~~~~~~~~~~~~~~ + // CHECK:STDERR: fail_monomorphization_identify_in_call.carbon:[[@LINE-9]]:1: note: while deducing parameters of generic declared here [DeductionGenericHere] + // CHECK:STDERR: fn F(N:! Core.IntLiteral, _:! S(N)) {} + // CHECK:STDERR: ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~ + // CHECK:STDERR: + F(-1, T); +}; + +// --- fail_monomorphization_during_deduction_in_call.carbon +library "[[@TEST_NAME]]"; + +interface Y {} +interface Z { + let Z1:! type; +} + +constraint S(N:! Core.IntLiteral) { + extend require impls Z; +} + +fn F(N:! Core.IntLiteral, _:! S(N) where .Z1 = array((), N)) {} + +fn G(T:! Z) { + // CHECK:STDERR: fail_monomorphization_during_deduction_in_call.carbon:[[@LINE+10]]:3: error: constructed invalid specific for `S(N) where .(Z.Z1) = array((), N)` from argument [SubstitutingGenericParamType] + // CHECK:STDERR: F(-1, T); + // CHECK:STDERR: ^~~~~~~~ + // CHECK:STDERR: fail_monomorphization_during_deduction_in_call.carbon:[[@LINE-6]]:27: note: array bound of -1 is negative [ArrayBoundNegative] + // CHECK:STDERR: fn F(N:! Core.IntLiteral, _:! S(N) where .Z1 = array((), N)) {} + // CHECK:STDERR: ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~ + // CHECK:STDERR: fail_monomorphization_during_deduction_in_call.carbon:[[@LINE-9]]:1: note: while deducing parameters of generic declared here [DeductionGenericHere] + // CHECK:STDERR: fn F(N:! Core.IntLiteral, _:! S(N) where .Z1 = array((), N)) {} + // CHECK:STDERR: ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~ + // CHECK:STDERR: + F(-1, T); +}; diff --git a/toolchain/diagnostics/kind.def b/toolchain/diagnostics/kind.def index 0a48d0212ea6..cc334e19109c 100644 --- a/toolchain/diagnostics/kind.def +++ b/toolchain/diagnostics/kind.def @@ -325,6 +325,7 @@ CARBON_DIAGNOSTIC_KIND(DeductionIncomplete) CARBON_DIAGNOSTIC_KIND(DeductionInconsistent) CARBON_DIAGNOSTIC_KIND(DeductionGenericHere) CARBON_DIAGNOSTIC_KIND(InitializingGenericParam) +CARBON_DIAGNOSTIC_KIND(SubstitutingGenericParamType) CARBON_DIAGNOSTIC_KIND(CompTimeArgumentNotConstant) CARBON_DIAGNOSTIC_KIND(RuntimeConversionDuringCompTimeDeduction)