diff --git a/toolchain/check/eval.cpp b/toolchain/check/eval.cpp index b766e541745b..f6c12d4687fe 100644 --- a/toolchain/check/eval.cpp +++ b/toolchain/check/eval.cpp @@ -126,12 +126,14 @@ static auto ReplaceFieldWithConstantValue(Context& context, InstT* inst, // If the specified fields of the given typed instruction have constant values, // replaces the fields with their constant values and builds a corresponding -// constant value. The constant value is then checked by calling -// `validate_fn(typed_inst)`, which should return a `bool` indicating whether -// the new constant is valid. If validation passes, a corresponding ConstantId -// for the new constant is returned. Otherwise returns -// `ConstantId::NotConstant`. Returns `ConstantId::Error` if any subexpression -// is an error. +// constant value. Otherwise returns `ConstantId::NotConstant`. Returns +// `ConstantId::Error` if any subexpression is an error. +// +// The constant value is then checked by calling `validate_fn(typed_inst)`, +// which should return a `bool` indicating whether the new constant is valid. If +// validation passes, a corresponding ConstantId for the new constant is +// returned. If validation fails, it should produce a suitable error message. +// `ConstantId::Error` is returned. template static auto RebuildAndValidateIfFieldsAreConstant( Context& context, SemIR::Inst inst, ValidateFn validate_fn, @@ -142,8 +144,10 @@ static auto RebuildAndValidateIfFieldsAreConstant( Phase phase = Phase::Template; if ((ReplaceFieldWithConstantValue(context, &typed_inst, each_field_id, &phase) && - ...) && - validate_fn(typed_inst)) { + ...)) { + if (!validate_fn(typed_inst)) { + return SemIR::ConstantId::Error; + } return MakeConstantResult(context, typed_inst, phase); } return MakeNonConstantResult(phase); @@ -277,13 +281,29 @@ auto TryEvalInst(Context& context, SemIR::InstId inst_id, SemIR::Inst inst) return RebuildAndValidateIfFieldsAreConstant( context, inst, [&](SemIR::ArrayType result) { - // TODO: If the bound doesn't fit in 64 bits or is negative, - // produce an error rather than a non-constant type. - // TODO: Support a symbolic bound here. This will require fixing - // callers of `GetArrayBoundValue`. - auto int_bound = - context.insts().TryGetAs(result.bound_id); - return context.ints().Get(int_bound->int_id).getActiveBits() <= 64; + auto bound_id = inst.As().bound_id; + if (auto int_bound = context.insts().TryGetAs( + result.bound_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? + // TODO: Also check for a negative bound, once that's something we + // can represent. + const auto& bound_val = context.ints().Get(int_bound->int_id); + if (bound_val.getActiveBits() > 64) { + CARBON_DIAGNOSTIC(ArrayBoundTooLarge, Error, + "Array bound of {0} is too large.", + llvm::APInt); + context.emitter().Emit(bound_id, ArrayBoundTooLarge, bound_val); + return false; + } + } else { + // TODO: Permit symbolic array bounds. This will require fixing + // callers of `GetArrayBoundValue`. + context.TODO(context.insts().GetParseNode(bound_id), + "symbolic array bound"); + } + return true; }, &SemIR::ArrayType::bound_id); case SemIR::BoundMethod::Kind: diff --git a/toolchain/check/handle_array.cpp b/toolchain/check/handle_array.cpp index 73530d9f30da..d4cd6b164d53 100644 --- a/toolchain/check/handle_array.cpp +++ b/toolchain/check/handle_array.cpp @@ -41,8 +41,7 @@ auto HandleArrayExpr(Context& context, Parse::ArrayExprId parse_node) -> bool { if (!bound_inst.is_constant()) { CARBON_DIAGNOSTIC(InvalidArrayExpr, Error, "Array bound is not a constant."); - context.emitter().Emit(context.insts().GetParseNode(bound_inst_id), - InvalidArrayExpr); + context.emitter().Emit(bound_inst_id, InvalidArrayExpr); context.node_stack().Push(parse_node, SemIR::InstId::BuiltinError); return true; } diff --git a/toolchain/check/testdata/array/fail_bound_overflow.carbon b/toolchain/check/testdata/array/fail_bound_overflow.carbon index 24aec16c9149..19ad7f81b122 100644 --- a/toolchain/check/testdata/array/fail_bound_overflow.carbon +++ b/toolchain/check/testdata/array/fail_bound_overflow.carbon @@ -4,17 +4,17 @@ // // AUTOUPDATE -// CHECK:STDERR: fail_bound_overflow.carbon:[[@LINE+3]]:8: ERROR: Cannot evaluate type expression. +// CHECK:STDERR: fail_bound_overflow.carbon:[[@LINE+3]]:14: ERROR: Array bound of 39999999999999999993 is too large. // CHECK:STDERR: var a: [i32; 39999999999999999993]; -// CHECK:STDERR: ^~~~~~~~~~~~~~~~~~~~~~~~~~~ +// CHECK:STDERR: ^~~~~~~~~~~~~~~~~~~~ var a: [i32; 39999999999999999993]; // CHECK:STDERR: fail_bound_overflow.carbon:[[@LINE+6]]:8: ERROR: Cannot implicitly convert from `i32` to `type`. // CHECK:STDERR: var b: [1; 39999999999999999993]; // CHECK:STDERR: ^~~~~~~~~~~~~~~~~~~~~~~~~ -// CHECK:STDERR: fail_bound_overflow.carbon:[[@LINE+3]]:8: ERROR: Cannot evaluate type expression. +// CHECK:STDERR: fail_bound_overflow.carbon:[[@LINE+3]]:12: ERROR: Array bound of 39999999999999999993 is too large. // CHECK:STDERR: var b: [1; 39999999999999999993]; -// CHECK:STDERR: ^~~~~~~~~~~~~~~~~~~~~~~~~ +// CHECK:STDERR: ^~~~~~~~~~~~~~~~~~~~ var b: [1; 39999999999999999993]; // CHECK:STDOUT: --- fail_bound_overflow.carbon @@ -27,12 +27,12 @@ var b: [1; 39999999999999999993]; // CHECK:STDOUT: file { // CHECK:STDOUT: package: = namespace package, {.a = %a, .b = %b} [template] // CHECK:STDOUT: %.loc10_14: i32 = int_literal 39999999999999999993 [template = constants.%.1] -// CHECK:STDOUT: %.loc10_34: type = array_type %.loc10_14, i32 +// CHECK:STDOUT: %.loc10_34: type = array_type %.loc10_14, i32 [template = ] // CHECK:STDOUT: %a.var: ref = var a // CHECK:STDOUT: %a: ref = bind_name a, %a.var // CHECK:STDOUT: %.loc18_9: i32 = int_literal 1 [template = constants.%.2] // CHECK:STDOUT: %.loc18_12: i32 = int_literal 39999999999999999993 [template = constants.%.1] -// CHECK:STDOUT: %.loc18_32: type = array_type %.loc18_12, +// CHECK:STDOUT: %.loc18_32: type = array_type %.loc18_12, [template = ] // CHECK:STDOUT: %b.var: ref = var b // CHECK:STDOUT: %b: ref = bind_name b, %b.var // CHECK:STDOUT: } diff --git a/toolchain/diagnostics/diagnostic_kind.def b/toolchain/diagnostics/diagnostic_kind.def index a7e2454c654a..64c46b0f39a6 100644 --- a/toolchain/diagnostics/diagnostic_kind.def +++ b/toolchain/diagnostics/diagnostic_kind.def @@ -166,6 +166,7 @@ CARBON_DIAGNOSTIC_KIND(InterfaceRedefinition) CARBON_DIAGNOSTIC_KIND(AddrOfEphemeralRef) CARBON_DIAGNOSTIC_KIND(AddrOfNonRef) CARBON_DIAGNOSTIC_KIND(AddrOnNonSelfParam) +CARBON_DIAGNOSTIC_KIND(ArrayBoundTooLarge) CARBON_DIAGNOSTIC_KIND(ArrayIndexOutOfBounds) CARBON_DIAGNOSTIC_KIND(ArrayInitFromLiteralArgCountMismatch) CARBON_DIAGNOSTIC_KIND(ArrayInitFromExprArgCountMismatch)