From 3636898ac088844479f80e67f03e05317bf11749 Mon Sep 17 00:00:00 2001 From: Christoffer Lerno Date: Tue, 29 Apr 2025 11:53:32 +0200 Subject: [PATCH] Fixed enum regression after 0.7.0 enum change. --- lib/std/collections/enummap.c3 | 1 + releasenotes.md | 1 + src/compiler/sema_decls.c | 1 - src/compiler/sema_expr.c | 34 +++- src/compiler/sema_stmts.c | 160 +++++++++++++----- .../enumerations/enum_map_overload.c3t | 51 ++++++ test/test_suite/statements/foreach_errors.c3 | 2 +- .../test_suite/statements/foreach_r_errors.c3 | 2 +- .../statements/foreach_wrong_index.c3 | 2 +- 9 files changed, 196 insertions(+), 58 deletions(-) create mode 100644 test/test_suite/enumerations/enum_map_overload.c3t diff --git a/lib/std/collections/enummap.c3 b/lib/std/collections/enummap.c3 index 66730cef8..8e5c26db3 100644 --- a/lib/std/collections/enummap.c3 +++ b/lib/std/collections/enummap.c3 @@ -3,6 +3,7 @@ *> module std::collections::enummap{Enum, ValueType}; import std::io; + struct EnumMap (Printable) { ValueType[Enum.len] values; diff --git a/releasenotes.md b/releasenotes.md index 604e4b0e9..aa23e029f 100644 --- a/releasenotes.md +++ b/releasenotes.md @@ -37,6 +37,7 @@ - Compiler crash when passing an untyped list as an argument to `assert` #2108. - `@ensure` should be allowed to read "out" variables. #2107 - Error message for casting generic to incompatible type does not work properly with nested generics #1953 +- Fixed enum regression after 0.7.0 enum change. ### Stdlib changes - Hash functions for integer vectors and arrays. diff --git a/src/compiler/sema_decls.c b/src/compiler/sema_decls.c index 40a60ff33..93f27c71c 100755 --- a/src/compiler/sema_decls.c +++ b/src/compiler/sema_decls.c @@ -4766,7 +4766,6 @@ static bool sema_generate_parameterized_name_to_scratch(SemaContext *context, Mo else { Type *type = param->type->canonical; - bool is_enum_or_fault = type_kind_is_enum_or_fault(type->type_kind); if (type == type_bool) { if (mangled) diff --git a/src/compiler/sema_expr.c b/src/compiler/sema_expr.c index 562f8da0b..44e5c84af 100644 --- a/src/compiler/sema_expr.c +++ b/src/compiler/sema_expr.c @@ -3289,6 +3289,27 @@ static inline bool sema_expr_resolve_subscript_index(SemaContext *context, Expr if (!overload) current_type = type_flatten(current_expr->type); } ASSERT(current_type == current_type->canonical); + Type *index_type = NULL; + if (overload) + { + index_type = overload->func_decl.signature.params[1]->type; + if (!sema_analyse_inferred_expr(context, index_type, index)) + { + expr_poison(index); + return false; + } + } + else + { + if (!sema_analyse_expr_value(context, index)) + { + expr_poison(index); + return false; + } + } + + // Analyse the index. + int64_t index_value = -1; bool start_from_end = expr->subscript_expr.index.start_from_end; if (start_from_end && (current_type->type_kind == TYPE_POINTER || current_type->type_kind == TYPE_FLEXIBLE_ARRAY)) @@ -3361,9 +3382,7 @@ static inline bool sema_expr_analyse_subscript_lvalue(SemaContext *context, Expr if (!sema_expr_check_assign(context, expr, NULL)) return false; - // 2. Evaluate the index. Expr *index = exprptr(expr->subscript_expr.index.expr); - if (!sema_analyse_expr(context, index)) return false; // 3. Check failability due to value. bool optional = IS_OPTIONAL(subscripted); @@ -3375,7 +3394,7 @@ static inline bool sema_expr_analyse_subscript_lvalue(SemaContext *context, Expr int64_t index_value; if (!sema_expr_resolve_subscript_index(context, expr, subscripted, index, ¤t_type, ¤t_expr, &subscript_type, &overload, &index_value, false, OVERLOAD_ELEMENT_SET, check_valid)) { - if (check_valid) + if (check_valid && expr_ok(index)) { expr_poison(expr); return true; @@ -3468,13 +3487,12 @@ static inline bool sema_expr_analyse_subscript(SemaContext *context, Expr *expr, Expr *subscripted = exprptr(expr->subscript_expr.expr); if (!sema_analyse_expr_check(context, subscripted, CHECK_VALUE)) return false; - // 2. Evaluate the index. - Expr *index = exprptr(expr->subscript_expr.index.expr); - if (!sema_analyse_expr(context, index)) return false; - // 3. Check failability due to value. bool optional = IS_OPTIONAL(subscripted); + // 2. Evaluate the index. + Expr *index = exprptr(expr->subscript_expr.index.expr); + Decl *overload = NULL; Type *subscript_type = NULL; Expr *current_expr; @@ -3484,7 +3502,7 @@ static inline bool sema_expr_analyse_subscript(SemaContext *context, Expr *expr, if (!sema_expr_resolve_subscript_index(context, expr, subscripted, index, ¤t_type, ¤t_expr, &subscript_type, &overload, &index_value, is_eval_ref, overload_type, check_valid)) { - if (check_valid) + if (check_valid && expr_ok(index)) { expr_poison(expr); return true; diff --git a/src/compiler/sema_stmts.c b/src/compiler/sema_stmts.c index 7684f12ed..9f3900f9d 100644 --- a/src/compiler/sema_stmts.c +++ b/src/compiler/sema_stmts.c @@ -1426,6 +1426,7 @@ static inline bool sema_analyse_foreach_stmt(SemaContext *context, Ast *statemen bool is_reverse = statement->foreach_stmt.is_reverse; bool value_by_ref = statement->foreach_stmt.value_by_ref; bool iterator_was_initializer = enumerator->expr_kind == EXPR_INITIALIZER_LIST; + // Check the type if needed TypeInfo *variable_type_info = vartype(var); if (variable_type_info && !sema_resolve_type_info(context, variable_type_info, RESOLVE_TYPE_DEFAULT)) return false; @@ -1433,7 +1434,6 @@ static inline bool sema_analyse_foreach_stmt(SemaContext *context, Ast *statemen // Conditional scope start SCOPE_START - // In the case of foreach (int x : { 1, 2, 3 }) we will infer the int[] type, so pick out the number of elements. Type *inferred_type = variable_type_info ? type_get_inferred_array(type_no_optional(variable_type_info->type)) : NULL; @@ -1448,18 +1448,22 @@ static inline bool sema_analyse_foreach_stmt(SemaContext *context, Ast *statemen // And pop the cond scope. SCOPE_END; + // Trying to iterate over an optional is meaningless, it should always be handled + // So check if it's a foreach (x : may_fail()) if (IS_OPTIONAL(enumerator)) { RETURN_SEMA_ERROR(enumerator, "The foreach iterable expression may not be optional."); } + // We handle the case of `foreach(&i, v : foo)` here, as it gives the chance + // to give better errors if (statement->foreach_stmt.index_by_ref) { - ASSERT(index); + ASSERT_SPAN(statement, index); RETURN_SEMA_ERROR(index, "The index cannot be held by reference, did you accidentally add a '&'?"); } - // Insert a single deref as needed. + // We might have an untyped list, if we failed the conversion. Type *canonical = enumerator->type->canonical; if (canonical->type_kind == TYPE_UNTYPED_LIST) { @@ -1469,6 +1473,8 @@ static inline bool sema_analyse_foreach_stmt(SemaContext *context, Ast *statemen } RETURN_SEMA_ERROR(var, "Add an explicit type to the variable if you want to iterate over an initializer list."); } + + // In the case of a single `*`, then we will implicitly dereference that pointer. if (canonical->type_kind == TYPE_POINTER) { // Something like Foo** will not be dereferenced, only Foo* @@ -1480,51 +1486,82 @@ static inline bool sema_analyse_foreach_stmt(SemaContext *context, Ast *statemen } // At this point we should have dereferenced any pointer or bailed. - ASSERT(!type_is_pointer(enumerator->type)); - - // Check that we can even index this expression. + ASSERT_SPAN(enumerator, !type_is_pointer(enumerator->type)); + // Check that we can even index this expression, this will dig into the flattened type. Type *value_type = type_get_indexed_type(enumerator->type); + + // However, if we have something distinct, that flattens to a pointer, we should never take the + // the underlying pointee type. if (canonical->type_kind == TYPE_DISTINCT && type_flatten(canonical)->type_kind == TYPE_POINTER) { value_type = NULL; } + // If we have a value type and it's by ref, then we get the pointer to that type. if (value_type && value_by_ref) value_type = type_get_ptr(value_type); Decl *len = NULL; - Decl *index_macro = NULL; + Decl *index_function = NULL; Type *index_type = type_usz; + bool is_enum_iterator = false; + // Now we lower the foreach... + // If we can't find a value, or this is distinct, then we assume there is an overload. if (!value_type || canonical->type_kind == TYPE_DISTINCT) { + // Get the overload for .len len = sema_find_untyped_operator(context, enumerator->type, OVERLOAD_LEN, NULL); + // For foo[] Decl *by_val = sema_find_untyped_operator(context, enumerator->type, OVERLOAD_ELEMENT_AT, NULL); + // For &foo[] Decl *by_ref = sema_find_untyped_operator(context, enumerator->type, OVERLOAD_ELEMENT_REF, NULL); + + // If we don't have .len, or there is neither by val nor by ref if (!len || (!by_val && !by_ref)) { + // If we found an underlying type we can iterate over, use that. if (value_type) goto SKIP_OVERLOAD; + + // Otherwise this is an error. RETURN_SEMA_ERROR(enumerator, "It's not possible to enumerate an expression of type %s.", type_quoted_error_string(enumerator->type)); } + // If we want the value "by ref" and there isn't a &[], then this is an error. if (!by_ref && value_by_ref) { RETURN_SEMA_ERROR(enumerator, "%s does not support 'foreach' by reference, but you iterate by value.", type_quoted_error_string(enumerator->type)); } + + // If there was an error in either of those declarations, + // bail here. if (!decl_ok(len) || !decl_ok(by_val) || !decl_ok(by_ref)) return false; - index_macro = value_by_ref ? by_ref : by_val; - ASSERT(index_macro); - index_type = index_macro->func_decl.signature.params[1]->type; - if (!type_is_integer(index_type)) + + // Get the proper macro + index_function = value_by_ref ? by_ref : by_val; + ASSERT_SPAN(statement, index_function); + + // The index type is the second parameter. + index_type = index_function->func_decl.signature.params[1]->type; + + // If it's an enum this is handled in a special way. + is_enum_iterator = index_type->canonical->type_kind == TYPE_ENUM; + + // We check that the index is either using integer or enums. + if (!type_is_integer(index_type) && !is_enum_iterator) { - RETURN_SEMA_ERROR(enumerator, "Only integer indexed types may be used with foreach."); + RETURN_SEMA_ERROR(enumerator, "Only types indexed by integers or enums may be used with foreach."); } - TypeInfoId rtype = index_macro->func_decl.signature.rtype; + + // The return type is the value type if it is known (it's inferred for macros) + TypeInfoId rtype = index_function->func_decl.signature.rtype; value_type = rtype ? type_infoptr(rtype)->type : NULL; } SKIP_OVERLOAD:; + // Get the type of the variable, if available (e.g. foreach (Foo x : y) has a type TypeInfo *type_info = vartype(var); + // Set up the value, assigning the type as needed. // Element *value @noinit if (!type_info) @@ -1538,6 +1575,7 @@ SKIP_OVERLOAD:; Type *index_var_type = NULL; if (index) { + // The index may or may not have a type, infer it as needed. TypeInfo *idx_type_info = vartype(index); if (!idx_type_info) { @@ -1545,15 +1583,28 @@ SKIP_OVERLOAD:; index->var.type_info = type_infoid(idx_type_info); } if (!sema_resolve_type_info(context, idx_type_info, RESOLVE_TYPE_DEFAULT)) return false; + + // The resulting type is our "index_var_type" index_var_type = idx_type_info->type; + + // Check that we don't have `foreach(int? i, y : z)` if (type_is_optional(index_var_type)) { RETURN_SEMA_ERROR(idx_type_info, "The index may not be an optional."); } - if (!type_is_integer(type_flatten(index_var_type))) + // If we have an enum iterator, the enums must match. + if (is_enum_iterator) { + if (index_var_type->canonical != index_type->canonical) + { + RETURN_SEMA_ERROR(idx_type_info, "The index value must be the enum %s.", type_quoted_error_string(index_type)); + } + } + else if (!type_is_integer(type_flatten(index_var_type))) + { + // Otherwise make sure it's an integer. RETURN_SEMA_ERROR(idx_type_info, - "Index must be an integer type, '%s' is not valid.", + "The index must be an integer type, '%s' is not valid.", type_to_error_string(index_var_type)); } } @@ -1561,27 +1612,22 @@ SKIP_OVERLOAD:; // We either have "foreach (x : some_var)" or "foreach (x : some_call())" // So we grab the former by address (implicit &) and the latter as the value. + // Generate the temp as needed. ASSERT(enumerator->resolve_status == RESOLVE_DONE); bool is_addr = false; - bool is_variable = false; + Decl *temp = NULL; if (enumerator->expr_kind == EXPR_IDENTIFIER) { enumerator->ident_expr->var.is_written = true; - is_variable = true; - } - else if (expr_may_addr(enumerator)) - { - is_addr = true; - expr_insert_addr(enumerator); - } - - Decl *temp = NULL; - if (is_variable) - { temp = enumerator->ident_expr; } else { + if (expr_may_addr(enumerator)) + { + is_addr = true; + expr_insert_addr(enumerator); + } // Store either "Foo* __enum$ = &some_var;" or "Foo __enum$ = some_call()" temp = decl_new_generated_var(enumerator->type, VARDECL_LOCAL, enumerator->span); vec_add(expressions, expr_generate_decl(temp, enumerator)); @@ -1623,12 +1669,15 @@ SKIP_OVERLOAD:; is_reverse = false; } - Decl *idx_decl = decl_new_generated_var(index_type, VARDECL_LOCAL, index ? index->span : enumerator->span); + // Find the actual index type -> flattening the enum + Type *actual_index_type = is_enum_iterator ? index_type->canonical->decl->enums.type_info->type : index_type; - // IndexType __len$ = (IndexType)(@__enum$.len()) + // Generate the index variable + Decl *idx_decl = decl_new_generated_var(actual_index_type, VARDECL_LOCAL, index ? index->span : enumerator->span); + + // IndexType __len$ = (ActualIndexType)(@__enum$.len()) Decl *len_decl = NULL; - if (is_reverse) { if (!len_call) @@ -1636,36 +1685,53 @@ SKIP_OVERLOAD:; // Create const len if missing. len_call = expr_new_const_int(enumerator->span, type_isz, array_len); } - if (!cast_implicit(context, len_call, index_type, false)) return false; - // __idx$ = (IndexType)(@__enum$.len()) (or const) + if (is_enum_iterator) + { + if (!cast_explicit(context, len_call, actual_index_type)) return false; + } + else + { + if (!cast_implicit(context, len_call, actual_index_type, false)) return false; + } + // __idx$ = (ActualIndexType)(@__enum$.len()) (or const) vec_add(expressions, expr_generate_decl(idx_decl, len_call)); } else { if (len_call) { - len_decl = decl_new_generated_var(index_type, VARDECL_LOCAL, enumerator->span); - if (!cast_implicit_silent(context, len_call, index_type, false)) + len_decl = decl_new_generated_var(actual_index_type, VARDECL_LOCAL, enumerator->span); + bool success; + if (is_enum_iterator) + { + success = cast_explicit_silent(context, len_call, actual_index_type); + } + else + { + success = cast_implicit_silent(context, len_call, actual_index_type, false); + } + if (!success) { SEMA_ERROR(enumerator, "'foreach' is not supported, as the length %s cannot " - "be cast implicitly cast to %s - please update your definition.", - type_quoted_error_string(len_call->type), type_quoted_error_string(index_type)); + "be %s to %s - please update your definition.", + type_quoted_error_string(len_call->type), is_enum_iterator ? "cast" : "implicitly cast", + type_quoted_error_string(actual_index_type)); if (len) { SEMA_NOTE(len, "The definition of 'len()' is here."); decl_poison(len); } - if (index_macro) + if (index_function) { - SEMA_NOTE(index_macro, "The index definition is here."); - decl_poison(index_macro); + SEMA_NOTE(index_function, "The index definition is here."); + decl_poison(index_function); } return false; } vec_add(expressions, expr_generate_decl(len_decl, len_call)); } - Expr *idx_init = expr_new_const_int(idx_decl->span, index_type, 0); + Expr *idx_init = expr_new_const_int(idx_decl->span, actual_index_type, 0); vec_add(expressions, expr_generate_decl(idx_decl, idx_init)); } @@ -1726,6 +1792,10 @@ SKIP_OVERLOAD:; Ast *declare_ast = new_ast(AST_DECLARE_STMT, var->span); declare_ast->declare_stmt = index; Expr *load_idx = expr_variable(idx_decl); + if (is_enum_iterator) + { + load_idx->type = index_var_type; + } if (!cast_explicit(context, load_idx, index_var_type)) return false; index->var.init_expr = load_idx; ast_append(&succ, declare_ast); @@ -1740,14 +1810,12 @@ SKIP_OVERLOAD:; enum_val->span = enumerator->span; if (is_addr) expr_rewrite_insert_deref(enum_val); subscript->subscript_expr.expr = exprid(enum_val); - if (array_len == 1) + Expr *index_expr = array_len == 1 ? expr_new_const_int(var->span, idx_decl->type, 0) : expr_variable(idx_decl); + if (is_enum_iterator) { - subscript->subscript_expr.index.expr = exprid(expr_new_const_int(var->span, idx_decl->type, 0)); - } - else - { - subscript->subscript_expr.index.expr = exprid(expr_variable(idx_decl)); + expr_rewrite_enum_from_ord(index_expr, index_type); } + subscript->subscript_expr.index.expr = exprid(index_expr); if (value_by_ref) { Expr *addr = expr_new(EXPR_UNARY, subscript->span); diff --git a/test/test_suite/enumerations/enum_map_overload.c3t b/test/test_suite/enumerations/enum_map_overload.c3t new file mode 100644 index 000000000..287bd15e3 --- /dev/null +++ b/test/test_suite/enumerations/enum_map_overload.c3t @@ -0,0 +1,51 @@ +// #target: macos-x64 +module test; +import std; + +enum Foo +{ + ABC, DEF +} +EnumMap{Foo, int} map; +fn void main() +{ + map[ABC] = 123; + map[Foo.DEF] = -1; + foreach (i, v : map) + { + } +} + +/* #expect: test.ll + +define void @test.main() #0 { +entry: + %.anon = alloca i32, align 4 + %i = alloca i32, align 4 + %v = alloca i32, align 4 + call void @"std_collections_enummap$test.Foo$int$.EnumMap.set"(ptr @test.map, i32 0, i32 123) #1 + call void @"std_collections_enummap$test.Foo$int$.EnumMap.set"(ptr @test.map, i32 1, i32 -1) #1 + %0 = call i64 @"std_collections_enummap$test.Foo$int$.EnumMap.len"(ptr @test.map) #1 + %trunc = trunc i64 %0 to i32 + store i32 0, ptr %.anon, align 4 + br label %loop.cond + +loop.cond: ; preds = %loop.body, %entry + %1 = load i32, ptr %.anon, align 4 + %lt = icmp slt i32 %1, %trunc + br i1 %lt, label %loop.body, label %loop.exit + +loop.body: ; preds = %loop.cond + %2 = load i32, ptr %.anon, align 4 + store i32 %2, ptr %i, align 4 + %3 = load i32, ptr %.anon, align 4 + %4 = call i32 @"std_collections_enummap$test.Foo$int$.EnumMap.get"(ptr @test.map, i32 %3) #1 + store i32 %4, ptr %v, align 4 + %5 = load i32, ptr %.anon, align 4 + %addnsw = add nsw i32 %5, 1 + store i32 %addnsw, ptr %.anon, align 4 + br label %loop.cond + +loop.exit: ; preds = %loop.cond + ret void +} diff --git a/test/test_suite/statements/foreach_errors.c3 b/test/test_suite/statements/foreach_errors.c3 index 669bc74ef..df9394fa6 100644 --- a/test/test_suite/statements/foreach_errors.c3 +++ b/test/test_suite/statements/foreach_errors.c3 @@ -16,7 +16,7 @@ fn void test2() { foreach (a : z) foo(); foreach (i, a : z) foo(); - foreach (double i, a : z); // #error: Index must be an integer type, 'double' is not valid. + foreach (double i, a : z); // #error: must be an integer type, 'double' is not valid. } fn void test3() diff --git a/test/test_suite/statements/foreach_r_errors.c3 b/test/test_suite/statements/foreach_r_errors.c3 index da0b00fa7..bb47d570a 100644 --- a/test/test_suite/statements/foreach_r_errors.c3 +++ b/test/test_suite/statements/foreach_r_errors.c3 @@ -16,7 +16,7 @@ fn void test2() { foreach_r (a : z) foo(); foreach_r (i, a : z) foo(); - foreach_r (double i, a : z); // #error: Index must be an integer type, 'double' is not valid. + foreach_r (double i, a : z); // #error: must be an integer type, 'double' is not valid. } fn void test3() diff --git a/test/test_suite/statements/foreach_wrong_index.c3 b/test/test_suite/statements/foreach_wrong_index.c3 index 67f1580b3..46a6d6049 100644 --- a/test/test_suite/statements/foreach_wrong_index.c3 +++ b/test/test_suite/statements/foreach_wrong_index.c3 @@ -20,7 +20,7 @@ fn void main() Foo f; io::printfn("%s", f[12.2]); - foreach (int i, value : f) // #error: Only integer + foreach (int i, value : f) // #error: Only types indexed by integers or enums may { io::printfn("v[%s] = %s", i, value); }