From 946c167bf1ba98dd850f29b22b117ee82e96f2ee Mon Sep 17 00:00:00 2001 From: Christoffer Lerno Date: Fri, 18 Apr 2025 17:19:04 +0200 Subject: [PATCH] Improve error for default args #2096. Deprecated old inference with slice copy. Copying must now ensure a slicing operator at the end of the right hand side: `foo[1..2] = bar[..]` rather than the old `foo[1..2] = bar`. The old behaviour can be mostly retained with `--use-old-slice-copy`). --- lib/std/compression/qoi.c3 | 4 +- lib/std/hash/hmac.c3 | 2 +- lib/std/hash/md5.c3 | 8 +- releasenotes.md | 2 + src/build/build.h | 2 + src/build/build_options.c | 1 + src/build/builder.c | 1 + src/compiler/sema_expr.c | 134 +++++++----------- .../expressions/bad_number_args_2096.c3 | 11 ++ .../functions/vararg_and_named_tests.c3 | 2 +- .../slices/slice_assign_designated_2095.c3 | 16 +++ test/test_suite/slices/slice_scalar.c3 | 7 + test/unit/regression/slice_assign.c3 | 2 +- 13 files changed, 101 insertions(+), 91 deletions(-) create mode 100644 test/test_suite/expressions/bad_number_args_2096.c3 create mode 100644 test/test_suite/slices/slice_assign_designated_2095.c3 create mode 100644 test/test_suite/slices/slice_scalar.c3 diff --git a/lib/std/compression/qoi.c3 b/lib/std/compression/qoi.c3 index cf95907bb..673edfafc 100644 --- a/lib/std/compression/qoi.c3 +++ b/lib/std/compression/qoi.c3 @@ -248,7 +248,7 @@ fn char[]? encode(Allocator allocator, char[] input, QOIDesc* desc) @nodiscard } // write end of stream - output[pos:END_OF_STREAM.len] = END_OF_STREAM; + output[pos:END_OF_STREAM.len] = END_OF_STREAM[..]; pos += END_OF_STREAM.len; return output[:pos]; @@ -364,7 +364,7 @@ fn char[]? decode(Allocator allocator, char[] data, QOIDesc* desc, QOIChannels c } // draw the pixel - if (channels == RGBA) { image[loc:4] = p.rgba; } else { image[loc:3] = p.rgb; } + if (channels == RGBA) { image[loc:4] = p.rgba[..]; } else { image[loc:3] = p.rgb[..]; } } return image; diff --git a/lib/std/hash/hmac.c3 b/lib/std/hash/hmac.c3 index 046ebc408..dd601f9da 100644 --- a/lib/std/hash/hmac.c3 +++ b/lib/std/hash/hmac.c3 @@ -93,7 +93,7 @@ macro @derive(Hmac *hmac_start, char[] salt, uint iterations, usz index, char[] UIntBE be = { (uint)index }; hmac.update(&&bitcast(be, char[4])); tmp = hmac.final(); - out[..] = tmp; + out[..] = tmp[..]; for (int it = 1; it < iterations; it++) { hmac = *hmac_start; diff --git a/lib/std/hash/md5.c3 b/lib/std/hash/md5.c3 index bcad6f5f8..31823e0f3 100644 --- a/lib/std/hash/md5.c3 +++ b/lib/std/hash/md5.c3 @@ -89,10 +89,10 @@ fn char[HASH_BYTES] Md5.final(&ctx) body(ctx, &ctx.buffer, 64); char[16] res @noinit; - res[0:4] = bitcast(ctx.a, char[4]); - res[4:4] = bitcast(ctx.b, char[4]); - res[8:4] = bitcast(ctx.c, char[4]); - res[12:4] = bitcast(ctx.d, char[4]); + res[0:4] = bitcast(ctx.a, char[4])[..]; + res[4:4] = bitcast(ctx.b, char[4])[..]; + res[8:4] = bitcast(ctx.c, char[4])[..]; + res[12:4] = bitcast(ctx.d, char[4])[..]; *ctx = {}; return res; } diff --git a/releasenotes.md b/releasenotes.md index 70a8a13f9..6e73d7124 100644 --- a/releasenotes.md +++ b/releasenotes.md @@ -15,6 +15,8 @@ - Allow `@if` on locals. - String str = "" is now guaranteed to be null terminated. #2083 - Improved error messages on `Foo { 3, abc }` #2099. +- `Foo[1..2] = { .baz = 123 }` inference now works. #2095 +- Deprecated old inference with slice copy. Copying must now ensure a slicing operator at the end of the right hand side: `foo[1..2] = bar[..]` rather than the old `foo[1..2] = bar`. The old behaviour can be mostly retained with `--use-old-slice-copy`). ### Fixes - Trying to cast an enum to int and back caused the compiler to crash. diff --git a/src/build/build.h b/src/build/build.h index e7994a215..23655a58f 100644 --- a/src/build/build.h +++ b/src/build/build.h @@ -549,6 +549,7 @@ typedef struct BuildOptions_ bool print_input; bool run_once; bool suppress_run; + bool old_slice_copy; int verbosity_level; const char *panicfn; const char *benchfn; @@ -674,6 +675,7 @@ typedef struct bool kernel_build; bool silence_deprecation; bool print_stats; + bool old_slice_copy; int build_threads; TrustLevel trust_level; OptimizationSetting optsetting; diff --git a/src/build/build_options.c b/src/build/build_options.c index 5bbc821e1..a508ee9dd 100644 --- a/src/build/build_options.c +++ b/src/build/build_options.c @@ -126,6 +126,7 @@ static void usage(bool full) print_opt("--single-module=", "Compile all modules together, enables more inlining."); print_opt("--show-backtrace=", "Show detailed backtrace on segfaults."); print_opt("--lsp", "Emit data about errors suitable for a LSP."); + print_opt("--use-old-slice-copy", "Use the old slice copy semantics."); } PRINTF(""); print_opt("-g", "Emit debug info."); diff --git a/src/build/builder.c b/src/build/builder.c index 974064d5d..75595b556 100644 --- a/src/build/builder.c +++ b/src/build/builder.c @@ -339,6 +339,7 @@ static void update_build_target_from_options(BuildTarget *target, BuildOptions * } target->backend = options->backend; + target->old_slice_copy = options->old_slice_copy; // Remove feature flags FOREACH(const char *, remove_feature, options->removed_feature_names) diff --git a/src/compiler/sema_expr.c b/src/compiler/sema_expr.c index 90cb65a3d..61d20563f 100644 --- a/src/compiler/sema_expr.c +++ b/src/compiler/sema_expr.c @@ -1798,6 +1798,8 @@ SPLAT_NORMAL:; } if (!has_named || !param->name) { + int missing = 1; + for (int j = i + 1; j < func_param_count; j++) if (!actual_args[j]) missing++; if (vec_size(callee->params) == 1) { if (param->type) @@ -1815,14 +1817,25 @@ SPLAT_NORMAL:; } if (!num_args) { + if (missing != needed) + { + RETURN_SEMA_FUNC_ERROR(callee->definition, call, "'%s' expects %d-%d parameters, but none was provided.", + callee->name, missing, needed); + } RETURN_SEMA_FUNC_ERROR(callee->definition, call, "'%s' expects %d parameter(s), but none was provided.", callee->name, needed); } if (!last) last = args[0]; int more_needed = func_param_count - i; + if (missing != more_needed) + { + RETURN_SEMA_FUNC_ERROR(callee->definition, last, + "%d-%d additional arguments were expected after this one, did you forget them?", + missing, more_needed); + } RETURN_SEMA_FUNC_ERROR(callee->definition, last, - "Expected %d more %s after this one, did you forget %s?", - more_needed, more_needed == 1 ? "argument" : "arguments", more_needed == 1 ? "it" : "them"); + "%d more %s expected after this one, did you forget %s?", + more_needed, more_needed == 1 ? "argument was" : "arguments were", more_needed == 1 ? "it" : "them"); } RETURN_SEMA_FUNC_ERROR(callee->definition, call, "The parameter '%s' must be set, did you forget it?", param->name); } @@ -5829,36 +5842,38 @@ static inline bool sema_expr_analyse_cast(SemaContext *context, Expr *expr, bool static bool sema_expr_analyse_slice_assign(SemaContext *context, Expr *expr, Type *left_type, Expr *right) { Expr *left = exprptr(expr->binary_expr.left); - if (!sema_analyse_expr(context, right)) return false; + Type *base = left_type->array.base; + if (right->expr_kind == EXPR_SLICE || (compiler.build.old_slice_copy && right->expr_kind == EXPR_INITIALIZER_LIST && right->initializer_list)) + { + if (!sema_analyse_inferred_expr(context, left_type, right)) return false; + if (type_flatten(left->type) == right->type || right->type == type_untypedlist) goto SLICE_COPY; + } + else + { + if (right->expr_kind == EXPR_INITIALIZER_LIST && right->initializer_list) + { + Type *flat = type_flatten(base); + if (!type_is_user_defined(flat) || flat->type_kind == TYPE_INTERFACE) + { + RETURN_SEMA_ERROR(right, "You trying to assign this expression to each element in the slice, but the expression can't be cast to a value of type %s. Maybe you wanted to do a slice copy and forgot to add [..] at the end? Rather than 'a[..] = { ... }', try 'a[..] = { ... }[..]'.", + type_quoted_error_string(base)); + } + } + if (!sema_analyse_inferred_expr(context, base, right)) return false; + } + Type *right_type = right->type->canonical; + if (base->canonical != right_type && (type_is_arraylike(right_type) || right_type->type_kind == TYPE_SLICE)) + { + if (right_type->array.base->canonical == base->canonical) + { + RETURN_SEMA_ERROR(right, "You cannot assign a slice, vector or array to a slicing without making an explicit [..] operation, e.g. 'foo[..] = my_array[..]', so you can try adding an explicit slicing to this expression."); + } + } + if (!cast_implicit(context, right, base, false)) return false; if (IS_OPTIONAL(right)) { RETURN_SEMA_ERROR(right, "The right hand side may not be optional when using slice assign."); } - Type *base = type_flatten(left_type)->array.base; - Type *rhs_type = type_flatten(right->type); - - switch (rhs_type->type_kind) - { - case TYPE_UNTYPED_LIST: - break; - case TYPE_VECTOR: - case TYPE_ARRAY: - case TYPE_SLICE: - if (base == rhs_type->array.base) goto SLICE_COPY; - break; - default: - if (!cast_implicit(context, right, base, false)) return false; - goto SLICE_ASSIGN; - } - - // By default we need to make a silent attempt. - bool could_cast = cast_implicit_silent(context, right, base, false); - - // Failed, so let's go back to the original - if (!could_cast) goto SLICE_COPY; - // Use the copy - -SLICE_ASSIGN: expr->expr_kind = EXPR_SLICE_ASSIGN; expr->type = right->type; expr->slice_assign_expr.left = exprid(left); @@ -5866,73 +5881,28 @@ SLICE_ASSIGN: return true; SLICE_COPY:; + if (!sema_analyse_expr_rhs(context, left_type, right, false, NULL, false)) return false; Range *left_range = &left->slice_expr.range; IndexDiff left_len = range_const_len(left_range); - switch (rhs_type->type_kind) - { - case TYPE_ARRAY: - case TYPE_SLICE: - case TYPE_VECTOR: - if (rhs_type->array.base != base) goto EXPECTED; - break; - case TYPE_UNTYPED_LIST: - { - ASSERT_SPAN(expr, right->const_expr.const_kind == CONST_UNTYPED_LIST); - // Zero sized lists cannot be right. - unsigned count = vec_size(right->const_expr.untyped_list); - if (!count) goto EXPECTED; - // Cast to an array of the length. - rhs_type = type_get_array(base, count); - if (!cast_implicit(context, right, rhs_type, false)) return false; - break; - } - default: - goto EXPECTED; - } - ASSERT_SPAN(expr, right->expr_kind != EXPR_SLICE || rhs_type->type_kind == TYPE_SLICE); - - // If we have a slice operation on the right hand side, check the ranges. - if (right->expr_kind == EXPR_SLICE) + IndexDiff right_len = 0; + if (!expr_is_const_slice(right)) { Range *right_range = &right->slice_expr.range; - IndexDiff right_len = range_const_len(right_range); + right_len = range_const_len(right_range); + } + else + { + right_len = sema_len_from_const(right); + } if (left_len >= 0 && right_len >= 0 && left_len != right_len) { RETURN_SEMA_ERROR(expr, "Length mismatch between slices."); } - } - else - { - // Otherwise we want to make a slice. - ArraySize len = rhs_type->array.len; - if (len > 0 && left_len > 0 && left_len != len) - { - RETURN_SEMA_ERROR(left, "Length mismatch, expected left hand slice to be %d elements.", left_len); - } - Expr *inner = expr_copy(right); - right->expr_kind = EXPR_SLICE; - Expr *const_zero = expr_new_const_int(inner->span, type_uint, 0); - Range range; - if (len > 0) - { - range = (Range) { .status = RESOLVE_DONE, .range_type = RANGE_CONST_RANGE, .is_range = true, .is_len = true, .start_index = 0, .len_index = len }; - } - else - { - range = (Range) { .start = exprid(const_zero), .is_range = true, .is_len = true }; - } - right->slice_expr = (ExprSlice ) { .range = range, .expr = exprid(inner) }; - right->resolve_status = RESOLVE_NOT_DONE; - if (!sema_analyse_expr(context, right)) return false; - } expr->expr_kind = EXPR_SLICE_COPY; expr->type = left->type; expr->slice_assign_expr.left = exprid(left); expr->slice_assign_expr.right = exprid(right); return true; -EXPECTED: - RETURN_SEMA_ERROR(right, "Expected an array, vector or slice with element type %s.", - type_quoted_error_string(base)); } bool sema_expr_analyse_assign_right_side(SemaContext *context, Expr *expr, Type *left_type, Expr *right, diff --git a/test/test_suite/expressions/bad_number_args_2096.c3 b/test/test_suite/expressions/bad_number_args_2096.c3 new file mode 100644 index 000000000..8497901ff --- /dev/null +++ b/test/test_suite/expressions/bad_number_args_2096.c3 @@ -0,0 +1,11 @@ +fn void foo(int a, int b, bool c = false) {} +fn void foo2(int a, int b, bool c) {} + +fn int main() +{ + foo(2); // #error: 1-2 additional arguments were expected + foo2(2);// #error: 2 more arguments + foo(); // #error: expects 2-3 parameters + foo2(); // #error: expects 3 parameter(s) + return 0; +} \ No newline at end of file diff --git a/test/test_suite/functions/vararg_and_named_tests.c3 b/test/test_suite/functions/vararg_and_named_tests.c3 index eb9b038e2..3247b100f 100644 --- a/test/test_suite/functions/vararg_and_named_tests.c3 +++ b/test/test_suite/functions/vararg_and_named_tests.c3 @@ -44,7 +44,7 @@ fn void multiple(int x, int y) {} fn void f() { - multiple(1); // #error: Expected 1 more argument after this one + multiple(1); // #error:1 more argument was expected after this one } fn void g() diff --git a/test/test_suite/slices/slice_assign_designated_2095.c3 b/test/test_suite/slices/slice_assign_designated_2095.c3 new file mode 100644 index 000000000..c904767cb --- /dev/null +++ b/test/test_suite/slices/slice_assign_designated_2095.c3 @@ -0,0 +1,16 @@ +module test; + +struct Struct +{ + int field; +} + +fn int main() +{ + Struct[4] arr; + + arr[1] = {.field = 1}; + arr[1..2] = {1}; + arr[1..2] = {.field = 1}; + return 0; +} diff --git a/test/test_suite/slices/slice_scalar.c3 b/test/test_suite/slices/slice_scalar.c3 new file mode 100644 index 000000000..16fa1ff32 --- /dev/null +++ b/test/test_suite/slices/slice_scalar.c3 @@ -0,0 +1,7 @@ +fn void assign_slice() +{ + int[8] a; + a[2..3] = { 1, 2 }; // #error: Maybe you wanted to do a slice copy + a[5..7] = 5; + assert(a == (int[8]){ 0, 0, 1, 2, 0, 5, 5, 5}); +} diff --git a/test/unit/regression/slice_assign.c3 b/test/unit/regression/slice_assign.c3 index f07970f5a..fa5cf79e1 100644 --- a/test/unit/regression/slice_assign.c3 +++ b/test/unit/regression/slice_assign.c3 @@ -3,7 +3,7 @@ module slice_assign @test; fn void assign_slice() { int[8] a; - a[2..3] = { 1, 2 }; + a[2..3] = { 1, 2 }[..]; a[5..7] = 5; assert(a == (int[8]){ 0, 0, 1, 2, 0, 5, 5, 5}); } \ No newline at end of file