From f67da4f3156519d7f14e52554a4b2af2bb8fe575 Mon Sep 17 00:00:00 2001 From: Christoffer Lerno Date: Wed, 25 Jun 2025 12:33:17 +0200 Subject: [PATCH] Refactoring, optimize negation in if statement. --- src/compiler/compiler_internal.h | 6 +- src/compiler/llvm_codegen_stmt.c | 5 +- src/compiler/sema_expr.c | 20 +++---- src/compiler/sema_stmts.c | 58 ++++++++++--------- src/compiler/semantic_analyser.c | 4 +- .../initializer_lists/subarrays.c3t | 9 ++- 6 files changed, 52 insertions(+), 50 deletions(-) diff --git a/src/compiler/compiler_internal.h b/src/compiler/compiler_internal.h index 27329f310..19d82605a 100644 --- a/src/compiler/compiler_internal.h +++ b/src/compiler/compiler_internal.h @@ -1570,7 +1570,7 @@ typedef struct DynamicScope_ ScopeId scope_id; bool allow_dead_code : 1; bool is_dead : 1; - bool is_invalid : 1; + bool is_poisoned : 1; EndJump end_jump; ScopeFlags flags; unsigned label_start; @@ -1739,9 +1739,7 @@ struct SemaContext_ Ast *yield_body; BlockExit** block_exit_ref; Type *expected_block_type; - Ast **returns; - // Reusable returns cache. - Ast **returns_cache; + Ast **block_returns; Expr **macro_varargs; Decl **macro_params; bool macro_has_ensures; diff --git a/src/compiler/llvm_codegen_stmt.c b/src/compiler/llvm_codegen_stmt.c index 359b399ba..e61709070 100644 --- a/src/compiler/llvm_codegen_stmt.c +++ b/src/compiler/llvm_codegen_stmt.c @@ -368,7 +368,7 @@ static void llvm_emit_if_stmt(GenContext *c, Ast *ast) LLVMBasicBlockRef then_block = exit_block; LLVMBasicBlockRef else_block = exit_block; - Ast *then_body = astptr(ast->if_stmt.then_body); + Ast *then_body = astptrzero(ast->if_stmt.then_body); // Only generate a target if if (ast_is_not_empty(then_body)) { @@ -376,8 +376,7 @@ static void llvm_emit_if_stmt(GenContext *c, Ast *ast) } // We have an optional else block. - AstId else_id = ast->if_stmt.else_body; - Ast *else_body = else_id ? astptr(else_id) : NULL; + Ast *else_body = astptrzero(ast->if_stmt.else_body); if (ast_is_not_empty(else_body)) { else_block = llvm_basic_block_new(c, "if.else"); diff --git a/src/compiler/sema_expr.c b/src/compiler/sema_expr.c index 4f29436a1..d926ea3d0 100644 --- a/src/compiler/sema_expr.c +++ b/src/compiler/sema_expr.c @@ -2294,11 +2294,11 @@ static inline Type *context_unify_returns(SemaContext *context) // 1. Loop through the returns. bool optional = false; - unsigned returns = vec_size(context->returns); + unsigned returns = vec_size(context->block_returns); if (!returns) return type_void; for (unsigned i = 0; i < returns; i++) { - Ast *return_stmt = context->returns[i]; + Ast *return_stmt = context->block_returns[i]; Type *rtype; if (!return_stmt) { @@ -2334,7 +2334,7 @@ static inline Type *context_unify_returns(SemaContext *context) ASSERT(return_stmt); SEMA_ERROR(return_stmt, "Cannot find a common parent type of %s and %s", type_quoted_error_string(rtype), type_quoted_error_string(common_type)); - Ast *prev = context->returns[i - 1]; + Ast *prev = context->block_returns[i - 1]; ASSERT(prev); SEMA_NOTE(prev, "The previous return was here."); return NULL; @@ -2351,7 +2351,7 @@ static inline Type *context_unify_returns(SemaContext *context) if (all_returns_need_casts) { ASSERT(common_type != type_wildcard); - FOREACH(Ast *, return_stmt, context->returns) + FOREACH(Ast *, return_stmt, context->block_returns) { if (!return_stmt) continue; Expr *ret_expr = return_stmt->return_stmt.expr; @@ -2676,7 +2676,7 @@ bool sema_expr_analyse_macro_call(SemaContext *context, Expr *call_expr, Expr *s params = macro_context.macro_params; bool is_no_return = sig->attrs.noreturn; - if (!vec_size(macro_context.returns)) + if (!vec_size(macro_context.block_returns)) { if (rtype && rtype != type_void && !macro_context.active_scope.end_jump.active) { @@ -2688,14 +2688,14 @@ bool sema_expr_analyse_macro_call(SemaContext *context, Expr *call_expr, Expr *s } else if (is_no_return) { - SEMA_ERROR(context->returns[0], "Return used despite macro being marked '@noreturn'."); + SEMA_ERROR(context->block_returns[0], "Return used despite macro being marked '@noreturn'."); goto EXIT_FAIL; } if (rtype) { bool inferred_len = type_len_is_inferred(rtype); - FOREACH(Ast *, return_stmt, macro_context.returns) + FOREACH(Ast *, return_stmt, macro_context.block_returns) { if (!return_stmt) { @@ -2772,7 +2772,7 @@ bool sema_expr_analyse_macro_call(SemaContext *context, Expr *call_expr, Expr *s { must_use = sig->attrs.nodiscard || (optional_return && !sig->attrs.maydiscard); } - unsigned returns_found = vec_size(macro_context.returns); + unsigned returns_found = vec_size(macro_context.block_returns); // We may have zero normal macro returns but the active scope still has a "jump end". // In this case it is triggered by the @body() if (!returns_found && macro_context.active_scope.end_jump.active) @@ -2781,7 +2781,7 @@ bool sema_expr_analyse_macro_call(SemaContext *context, Expr *call_expr, Expr *s } if (returns_found == 1 && !implicit_void_return) { - Ast *ret = macro_context.returns[0]; + Ast *ret = macro_context.block_returns[0]; Expr *result = ret ? ret->return_stmt.expr : NULL; if (!result) goto NOT_CT; if (!expr_is_runtime_const(result)) goto NOT_CT; @@ -8845,7 +8845,7 @@ static inline bool sema_expr_analyse_rethrow(SemaContext *context, Expr *expr) RETURN_SEMA_ERROR(expr, "Rethrow is only allowed in macros with an optional or inferred return type. " "Did you mean to use '!!' instead?"); } - vec_add(context->returns, NULL); + vec_add(context->block_returns, NULL); expr->rethrow_expr.in_block = context->block_exit_ref; expr->rethrow_expr.cleanup = context_get_defers(context, context->active_scope.defer_last, context->block_return_defer, false); } diff --git a/src/compiler/sema_stmts.c b/src/compiler/sema_stmts.c index e9f4f6941..9cdcfe2a4 100644 --- a/src/compiler/sema_stmts.c +++ b/src/compiler/sema_stmts.c @@ -564,7 +564,6 @@ END: */ static inline bool sema_analyse_block_exit_stmt(SemaContext *context, Ast *statement) { - bool is_macro = (context->active_scope.flags & SCOPE_MACRO) != 0; ASSERT(context->active_scope.flags & SCOPE_MACRO); statement->ast_kind = AST_BLOCK_EXIT_STMT; SET_JUMP_END(context, statement); @@ -580,22 +579,20 @@ static inline bool sema_analyse_block_exit_stmt(SemaContext *context, Ast *state { if (!sema_analyse_expr(context, ret_expr)) return false; } - if (is_macro && !sema_check_return_matches_opt_returns(context, ret_expr)) return false; - + if (!sema_check_return_matches_opt_returns(context, ret_expr)) return false; } else { - if (block_type && type_no_optional(block_type) != type_void) + // What if we already had an expected type and we do an empty return. + if (block_type && !type_is_void(type_no_optional(block_type))) { - SEMA_ERROR(statement, "Expected a return value of type %s here.", type_quoted_error_string(block_type)); - return false; + RETURN_SEMA_ERROR(statement, "Expected a return value of type %s here.", type_quoted_error_string(block_type)); } } statement->return_stmt.block_exit_ref = context->block_exit_ref; sema_inline_return_defers(context, statement, context->active_scope.defer_last, context->block_return_defer); - - if (is_macro && !sema_analyse_macro_constant_ensures(context, ret_expr)) return false; - vec_add(context->returns, statement); + if (!sema_analyse_macro_constant_ensures(context, ret_expr)) return false; + vec_add(context->block_returns, statement); return true; } @@ -1052,7 +1049,7 @@ static inline bool sema_analyse_last_cond(SemaContext *context, Expr *expr, Cond } if (!sema_analyse_catch_unwrap(context, expr)) { - context->active_scope.is_invalid = true; + context->active_scope.is_poisoned = true; return false; } default: @@ -1891,14 +1888,8 @@ SKIP_OVERLOAD:; static inline bool sema_analyse_if_stmt(SemaContext *context, Ast *statement) { - // IMPROVE - // convert - // if (!x) A(); else B(); - // into - // if (x) B(); else A(); - bool else_jump; - bool then_jump; + bool then_jump = false; bool success; Expr *cond = exprptr(statement->if_stmt.cond); @@ -1911,10 +1902,15 @@ static inline bool sema_analyse_if_stmt(SemaContext *context, Ast *statement) Ast *else_body = else_id ? astptr(else_id) : NULL; CondResult result = COND_MISSING; bool is_invalid = false; + bool reverse = false; SCOPE_OUTER_START success = sema_analyse_cond(context, cond, COND_TYPE_UNWRAP_BOOL, &result); - + if (success && cond->expr_kind == EXPR_COND) + { + Expr **list = cond->cond_expr; + reverse = vec_size(list) == 1 && list[0]->expr_kind == EXPR_UNARY && list[0]->unary_expr.operator == UNARYOP_NOT; + } if (success && !ast_ok(then)) { SEMA_ERROR(then, @@ -1943,7 +1939,11 @@ static inline bool sema_analyse_if_stmt(SemaContext *context, Ast *statement) context->active_scope.allow_dead_code = true; bool warn = SEMA_WARN(statement, "This code will never execute."); sema_note_prev_at(context->active_scope.end_jump.span, "This code is preventing it from exectuting"); - if (!warn) return success = false; + if (!warn) + { + success = false; + goto END; + } } SCOPE_START_WITH_LABEL(statement->if_stmt.flow.label); @@ -1968,12 +1968,12 @@ static inline bool sema_analyse_if_stmt(SemaContext *context, Ast *statement) END: context_pop_defers_and_replace_ast(context, statement); - is_invalid = context->active_scope.is_invalid; + is_invalid = context->active_scope.is_poisoned; SCOPE_OUTER_END; - if (is_invalid) context->active_scope.is_invalid = is_invalid; + if (is_invalid) context->active_scope.is_poisoned = is_invalid; if (!success) { - if (then_jump && sema_catch_in_cond(cond)) context->active_scope.is_invalid = true; + if (then_jump && sema_catch_in_cond(cond)) context->active_scope.is_poisoned = true; return false; } if (then_jump) @@ -1992,6 +1992,12 @@ END: { SET_JUMP_END(context, statement); } + if (reverse) + { + cond->cond_expr[0] = cond->cond_expr[0]->unary_expr.expr; + statement->if_stmt.else_body = statement->if_stmt.then_body; + statement->if_stmt.then_body = else_id; + } return true; } @@ -3126,10 +3132,10 @@ static inline bool sema_analyse_statement_inner(SemaContext *context, Ast *state bool sema_analyse_statement(SemaContext *context, Ast *statement) { - if (context->active_scope.is_invalid) return false; + if (context->active_scope.is_poisoned) return false; if (statement->ast_kind == AST_POISONED) return false; EndJump end_jump = context->active_scope.end_jump; - unsigned returns = vec_size(context->returns); + unsigned returns = vec_size(context->block_returns); if (!sema_analyse_statement_inner(context, statement)) return ast_poison(statement); if (end_jump.active) { @@ -3145,7 +3151,7 @@ bool sema_analyse_statement(SemaContext *context, Ast *statement) } // Remove it } - vec_resize(context->returns, returns); + vec_resize(context->block_returns, returns); statement->ast_kind = AST_NOP_STMT; } return true; @@ -3268,7 +3274,7 @@ bool sema_analyse_function_body(SemaContext *context, Decl *func) vec_resize(context->ct_locals, 0); // Clear returns - vec_resize(context->returns, 0); + vec_resize(context->block_returns, 0); context->scope_id = 0; context->continue_target = NULL; context->next_target = 0; diff --git a/src/compiler/semantic_analyser.c b/src/compiler/semantic_analyser.c index 957d6708f..c5cc4d3dc 100644 --- a/src/compiler/semantic_analyser.c +++ b/src/compiler/semantic_analyser.c @@ -17,7 +17,7 @@ void context_change_scope_with_flags(SemaContext *context, ScopeFlags flags) } bool scope_is_dead = context->active_scope.is_dead; - bool scope_is_invalid = context->active_scope.is_invalid; + bool scope_is_poisoned = context->active_scope.is_poisoned; Ast *previous_defer = context->active_scope.in_defer; AstId parent_defer = context->active_scope.defer_last; unsigned last_local = context->active_scope.current_local; @@ -38,7 +38,7 @@ void context_change_scope_with_flags(SemaContext *context, ScopeFlags flags) .scope_id = ++context->scope_id, .allow_dead_code = false, .is_dead = scope_is_dead, - .is_invalid = scope_is_invalid, + .is_poisoned = scope_is_poisoned, .depth = depth, .current_local = last_local, .label_start = label_start, diff --git a/test/test_suite/initializer_lists/subarrays.c3t b/test/test_suite/initializer_lists/subarrays.c3t index 807a4fcff..5cc523fca 100644 --- a/test/test_suite/initializer_lists/subarrays.c3t +++ b/test/test_suite/initializer_lists/subarrays.c3t @@ -182,21 +182,20 @@ voiderr: ; preds = %noerr_block15, %gua store i8 0, ptr %xy, align 1 %25 = load i8, ptr %xy, align 1 %26 = trunc i8 %25 to i1 - %not = xor i1 %26, true - br i1 %not, label %if.then, label %if.exit + br i1 %26, label %if.exit, label %if.else -if.then: ; preds = %voiderr +if.else: ; preds = %voiderr %27 = call ptr @std.io.stdout() %28 = call i64 @std.io.File.write(ptr %retparam23, ptr %27, ptr @.str.10, i64 2) %not_err24 = icmp eq i64 %28, 0 %29 = call i1 @llvm.expect.i1(i1 %not_err24, i1 true) br i1 %29, label %after_check26, label %assign_optional25 -assign_optional25: ; preds = %if.then +assign_optional25: ; preds = %if.else store i64 %28, ptr %error_var21, align 8 br label %guard_block27 -after_check26: ; preds = %if.then +after_check26: ; preds = %if.else br label %noerr_block28 guard_block27: ; preds = %assign_optional25