From 92ce6d79ec95800257d64c9939da339566d23129 Mon Sep 17 00:00:00 2001 From: Gavin Howard Date: Wed, 28 Dec 2022 14:24:16 -0700 Subject: [PATCH] Expand the bug fix from three commits ago to simplify code The bug fix (commit 2484c9b600) put a stack index into a BcLoc. I decided to also use this for variables. This allowed me to simplify function calls in two ways: * I don't have to check that a parameter that is used has already been pushed to. * I don't have to do anything special to push a parameter. The last one is especially important. `bc_program_copyToVar()`, the function that pushes parameters, is complex for bc, and it was *more* complex before this commit. Signed-off-by: Gavin Howard --- include/lang.h | 11 ++--- include/program.h | 9 ----- src/program.c | 100 +++++++++++++++++----------------------------- 3 files changed, 43 insertions(+), 77 deletions(-) diff --git a/include/lang.h b/include/lang.h index 1fdbe932..2d977653 100644 --- a/include/lang.h +++ b/include/lang.h @@ -352,14 +352,15 @@ typedef struct BcLoc /// The index of the var or array. size_t loc; + /// The index of the array or variable in the array stack. This is to + /// prevent a bug with getting the wrong array element or variable after a + /// function call. See the tests/bc/scripts/array.bc test for the array + /// case; the variable case is in various variable tests. + size_t stack_idx; + /// The index of the array element. Only used for array elements. size_t idx; - /// The index of the array in the array stack. Only used for array elements. - /// This is to prevent a bug with getting the wrong array element. See the - /// tests/bc/scripts/array.bc test. - size_t stack_idx; - } BcLoc; /// An entry for a constant. diff --git a/include/program.h b/include/program.h index 3acd5157..3a3ea6c9 100644 --- a/include/program.h +++ b/include/program.h @@ -220,20 +220,11 @@ typedef struct BcProgram #if !BC_ENABLED -/// This define disappears the parameter last because for dc only, last is -/// always true. -#define bc_program_copyToVar(p, name, t, last) \ - bc_program_copyToVar_impl(p, name, t) - /// Returns true if the calculator should pop after printing. #define BC_PROGRAM_POP(pop) (pop) #else // !BC_ENABLED -// This is here to quiet a compiler warning. -#define bc_program_copyToVar(p, name, t, last) \ - bc_program_copyToVar_impl(p, name, t, last) - /// Returns true if the calculator should pop after printing. #define BC_PROGRAM_POP(pop) (BC_IS_BC || (pop)) diff --git a/src/program.c b/src/program.c index f1468ad1..c20e3eee 100644 --- a/src/program.c +++ b/src/program.c @@ -376,8 +376,21 @@ bc_program_num(BcProgram* p, BcResult* r) n = bc_vec_item(v, idx); } // This is either a number (for a var) or an array (for an array). - // Because bc_vec_top() returns a void*, we don't need to cast. - else n = bc_vec_top(v); + // Because bc_vec_top() and bc_vec_item() return a void*, we don't + // need to cast. + else + { +#if BC_ENABLED + if (BC_IS_BC) + { + n = bc_vec_item(v, r->d.loc.stack_idx); + } + else +#endif // BC_ENABLED + { + n = bc_vec_top(v); + } + } break; } @@ -1181,18 +1194,12 @@ bc_program_assignStr(BcProgram* p, BcNum* num, BcVec* v, bool push) /** * Copies a value to a variable. This is used for storing in dc as well as to * set function parameters to arguments in bc. - * @param p The program. - * @param idx The index of the variable or array to copy to. - * @param t The type to copy to. This could be a variable or an array. - * @param last Whether to grab the last item on the variable stack or not (for - * bc function parameters). This is important because if a new - * value has been pushed to the variable already, we need to grab - * the value pushed before. This happens when you have a parameter - * named something like "x", and a variable "x" is passed to - * another parameter. + * @param p The program. + * @param idx The index of the variable or array to copy to. + * @param t The type to copy to. This could be a variable or an array. */ static void -bc_program_copyToVar(BcProgram* p, size_t idx, BcType t, bool last) +bc_program_copyToVar(BcProgram* p, size_t idx, BcType t) { BcResult *ptr = NULL, r; BcVec* vec; @@ -1217,13 +1224,6 @@ bc_program_copyToVar(BcProgram* p, size_t idx, BcType t, bool last) { // Type match the result. bc_program_type_match(ptr, t); - - // Get the variable or array, taking care to get the real item. We take - // care of last with arrays later. - if (!last && var) - { - n = bc_vec_item_rev(bc_program_vec(p, ptr->d.loc.loc, t), 1); - } } #endif // BC_ENABLED @@ -1265,19 +1265,8 @@ bc_program_copyToVar(BcProgram* p, size_t idx, BcType t, bool last) if (BC_IS_BC) { - BcVec* parent; bool ref, ref_size; - // We need to figure out if the parameter is a reference or not and - // construct the reference vector, if necessary. So this gets the - // parent stack for the array. - parent = bc_program_vec(p, ptr->d.loc.loc, t); - assert(parent != NULL); - - // This takes care of last for arrays. Mostly. - if (!last) v = bc_vec_item_rev(parent, !last); - assert(v != NULL); - // True if we are using a reference. ref = (v->size == sizeof(BcNum) && t == BC_TYPE_REF); @@ -1297,8 +1286,6 @@ bc_program_copyToVar(BcProgram* p, size_t idx, BcType t, bool last) // If this is true, then we need to construct a reference. if (ref) { - assert(parent->len >= (size_t) (!last + 1)); - // Make sure the pointer was not invalidated. vec = bc_program_vec(p, idx, t); @@ -1306,7 +1293,7 @@ bc_program_copyToVar(BcProgram* p, size_t idx, BcType t, bool last) // care of last; it ensures the reference goes to the right // place. bc_vec_pushIndex(rv, ptr->d.loc.loc); - bc_vec_pushIndex(rv, parent->len - !last - 1); + bc_vec_pushIndex(rv, ptr->d.loc.stack_idx); } // If we get here, we are copying a ref to a ref. Just push a // copy of all of the bytes. @@ -1600,18 +1587,25 @@ bc_program_pushVar(BcProgram* p, const char* restrict code, { BcResult r; size_t idx = bc_program_index(code, bgn); + BcVec* v; // Set the result appropriately. r.t = BC_RESULT_VAR; r.d.loc.loc = idx; + // Get the stack for the variable. This is used in both bc and dc. + v = bc_program_vec(p, idx, BC_TYPE_VAR); + +#if BC_ENABLED + r.d.loc.stack_idx = v->len - 1; +#endif // BC_ENABLED + #if DC_ENABLED // If this condition is true, then we have the hard case, where we have to // adjust dc registers. if (BC_IS_DC && (pop || copy)) { - // Get the stack for the variable and the number at the top. - BcVec* v = bc_program_vec(p, idx, BC_TYPE_VAR); + // Get the number at the top at the top of the stack. BcNum* num = bc_vec_top(v); // Ensure there are enough elements on the stack. @@ -1679,6 +1673,12 @@ bc_program_pushArray(BcProgram* p, const char* restrict code, // Get the index of the array. r.d.loc.loc = bc_program_index(code, bgn); + // We need the array to get its length. + v = bc_program_vec(p, r.d.loc.loc, BC_TYPE_ARRAY); + assert(v != NULL); + + r.d.loc.stack_idx = v->len - 1; + // Doing an array is easy; just set the result type and finish. if (inst == BC_INST_ARRAY) { @@ -1691,14 +1691,9 @@ bc_program_pushArray(BcProgram* p, const char* restrict code, bc_program_prep(p, &operand, &num, 0); temp = bc_num_bigdig(num); - v = bc_program_vec(p, r.d.loc.loc, BC_TYPE_ARRAY); - - assert(v != NULL); - // Set the result. r.t = BC_RESULT_ARRAY_ELEM; r.d.loc.idx = (size_t) temp; - r.d.loc.stack_idx = v->len - 1; BC_SIG_LOCK; @@ -1806,35 +1801,14 @@ bc_program_call(BcProgram* p, const char* restrict code, size_t* restrict bgn) // Push the arguments onto the stacks of their respective parameters. for (i = 0; i < nargs; ++i) { - size_t j; - bool last = true; - arg = bc_vec_top(&p->results); if (BC_ERR(arg->t == BC_RESULT_VOID)) bc_err(BC_ERR_EXEC_VOID_VAL); // Get the corresponding parameter. a = bc_vec_item(&f->autos, nargs - 1 - i); - // If I have already pushed to a var, I need to make sure I - // get the previous version, not the already pushed one. This condition - // must be true for that to even be possible. - if (arg->t == BC_RESULT_VAR || arg->t == BC_RESULT_ARRAY) - { - // Loop through all of the previous parameters. - for (j = 0; j < i && last; ++j) - { - BcAuto* aptr = bc_vec_item(&f->autos, nargs - 1 - j); - - // This condition is true if there is a previous parameter with - // the same name *and* type because variables and arrays do not - // interfere with each other. - last = (arg->d.loc.loc != aptr->idx || - (!aptr->type) != (arg->t == BC_RESULT_VAR)); - } - } - // Actually push the value onto the parameter's stack. - bc_program_copyToVar(p, a->idx, a->type, last); + bc_program_copyToVar(p, a->idx, a->type); } BC_SIG_LOCK; @@ -3656,7 +3630,7 @@ bc_program_exec(BcProgram* p) // clang-format on { idx = bc_program_index(code, &ip->idx); - bc_program_copyToVar(p, idx, BC_TYPE_VAR, true); + bc_program_copyToVar(p, idx, BC_TYPE_VAR); BC_PROG_JUMP(inst, code, ip); }