From 041625381cfd6236de5bc5fea3a398123b7328ed Mon Sep 17 00:00:00 2001 From: gingerBill Date: Sat, 16 Jul 2022 17:36:03 +0100 Subject: [PATCH] Fix #1888 --- src/check_expr.cpp | 8 ++++++-- src/llvm_backend.hpp | 2 ++ src/llvm_backend_expr.cpp | 38 ++++++++++++++++++++++++++++---------- src/llvm_backend_proc.cpp | 5 +---- src/parser.cpp | 1 + src/parser.hpp | 4 +++- 6 files changed, 41 insertions(+), 17 deletions(-) diff --git a/src/check_expr.cpp b/src/check_expr.cpp index b42301cd6..13d6badbe 100644 --- a/src/check_expr.cpp +++ b/src/check_expr.cpp @@ -8643,6 +8643,8 @@ ExprKind check_selector_call_expr(CheckerContext *c, Operand *o, Ast *node, Type Ast *first_arg = x.expr->SelectorExpr.expr; GB_ASSERT(first_arg != nullptr); + first_arg->state_flags |= StateFlag_SelectorCallExpr; + Type *pt = base_type(x.type); GB_ASSERT(pt->kind == Type_Proc); Type *first_type = nullptr; @@ -8668,6 +8670,7 @@ ExprKind check_selector_call_expr(CheckerContext *c, Operand *o, Ast *node, Type y.mode = first_arg->tav.mode; y.type = first_arg->tav.type; y.value = first_arg->tav.value; + if (check_is_assignable_to(c, &y, first_type)) { // Do nothing, it's valid } else { @@ -10034,9 +10037,10 @@ gbString write_expr_to_string(gbString str, Ast *node, bool shorthand) { str = write_expr_to_string(str, ce->proc, shorthand); str = gb_string_appendc(str, "("); - for_array(i, ce->args) { + isize idx0 = cast(isize)ce->was_selector; + for (isize i = idx0; i < ce->args.count; i++) { Ast *arg = ce->args[i]; - if (i > 0) { + if (i > idx0) { str = gb_string_appendc(str, ", "); } str = write_expr_to_string(str, arg, shorthand); diff --git a/src/llvm_backend.hpp b/src/llvm_backend.hpp index f65e1079e..87a354a49 100644 --- a/src/llvm_backend.hpp +++ b/src/llvm_backend.hpp @@ -292,6 +292,8 @@ struct lbProcedure { LLVMMetadataRef debug_info; lbCopyElisionHint copy_elision_hint; + + PtrMap selector_values; }; diff --git a/src/llvm_backend_expr.cpp b/src/llvm_backend_expr.cpp index 1894e85f6..a95d884b0 100644 --- a/src/llvm_backend_expr.cpp +++ b/src/llvm_backend_expr.cpp @@ -2993,9 +2993,8 @@ lbValue lb_build_unary_and(lbProcedure *p, Ast *expr) { return lb_build_addr_ptr(p, ue->expr); } +lbValue lb_build_expr_internal(lbProcedure *p, Ast *expr); lbValue lb_build_expr(lbProcedure *p, Ast *expr) { - lbModule *m = p->module; - u16 prev_state_flags = p->state_flags; defer (p->state_flags = prev_state_flags); @@ -3022,6 +3021,32 @@ lbValue lb_build_expr(lbProcedure *p, Ast *expr) { p->state_flags = out; } + + // IMPORTANT NOTE(bill): + // Selector Call Expressions (foo->bar(...)) + // must only evaluate `foo` once as it gets transformed into + // `foo.bar(foo, ...)` + // And if `foo` is a procedure call or something more complex, storing the value + // once is a very good idea + // If a stored value is found, it must be removed from the cache + if (expr->state_flags & StateFlag_SelectorCallExpr) { + lbValue *pp = map_get(&p->selector_values, expr); + if (pp != nullptr) { + lbValue res = *pp; + map_remove(&p->selector_values, expr); + return res; + } + } + lbValue res = lb_build_expr_internal(p, expr); + if (expr->state_flags & StateFlag_SelectorCallExpr) { + map_set(&p->selector_values, expr, res); + } + return res; +} + +lbValue lb_build_expr_internal(lbProcedure *p, Ast *expr) { + lbModule *m = p->module; + expr = unparen_expr(expr); TokenPos expr_pos = ast_token(expr).pos; @@ -3119,14 +3144,7 @@ lbValue lb_build_expr(lbProcedure *p, Ast *expr) { case_ast_node(se, SelectorCallExpr, expr); GB_ASSERT(se->modified_call); - TypeAndValue tav = type_and_value_of_expr(expr); - GB_ASSERT(tav.mode != Addressing_Invalid); - lbValue res = lb_build_call_expr(p, se->call); - - ast_node(ce, CallExpr, se->call); - ce->sce_temp_data = gb_alloc_copy(permanent_allocator(), &res, gb_size_of(res)); - - return res; + return lb_build_call_expr(p, se->call); case_end; case_ast_node(te, TernaryIfExpr, expr); diff --git a/src/llvm_backend_proc.cpp b/src/llvm_backend_proc.cpp index 75ca77641..1c0ecabbe 100644 --- a/src/llvm_backend_proc.cpp +++ b/src/llvm_backend_proc.cpp @@ -113,6 +113,7 @@ lbProcedure *lb_create_procedure(lbModule *m, Entity *entity, bool ignore_body) p->branch_blocks.allocator = a; p->context_stack.allocator = a; p->scope_stack.allocator = a; + map_init(&p->selector_values, a); if (p->is_foreign) { lb_add_foreign_library_path(p->module, entity->Procedure.foreign_library); @@ -2832,10 +2833,6 @@ lbValue lb_build_call_expr(lbProcedure *p, Ast *expr) { expr = unparen_expr(expr); ast_node(ce, CallExpr, expr); - if (ce->sce_temp_data) { - return *(lbValue *)ce->sce_temp_data; - } - lbValue res = lb_build_call_expr_internal(p, expr); if (ce->optional_ok_one) { // TODO(bill): Minor hack for #optional_ok procedures diff --git a/src/parser.cpp b/src/parser.cpp index a6f30cdfd..b63c530d6 100644 --- a/src/parser.cpp +++ b/src/parser.cpp @@ -164,6 +164,7 @@ Ast *clone_ast(Ast *node) { case Ast_SelectorCallExpr: n->SelectorCallExpr.expr = clone_ast(n->SelectorCallExpr.expr); n->SelectorCallExpr.call = clone_ast(n->SelectorCallExpr.call); + GB_ASSERT(n->SelectorCallExpr.modified_call); break; case Ast_IndexExpr: n->IndexExpr.expr = clone_ast(n->IndexExpr.expr); diff --git a/src/parser.hpp b/src/parser.hpp index c167ef6d5..8719b5e56 100644 --- a/src/parser.hpp +++ b/src/parser.hpp @@ -282,6 +282,8 @@ enum StateFlag : u8 { StateFlag_type_assert = 1<<2, StateFlag_no_type_assert = 1<<3, + StateFlag_SelectorCallExpr = 1<<6, + StateFlag_BeenHandled = 1<<7, }; @@ -411,7 +413,7 @@ AST_KIND(_ExprBegin, "", bool) \ Token ellipsis; \ ProcInlining inlining; \ bool optional_ok_one; \ - void *sce_temp_data; \ + bool was_selector; \ }) \ AST_KIND(FieldValue, "field value", struct { Token eq; Ast *field, *value; }) \ AST_KIND(EnumFieldValue, "enum field value", struct { \