From 41412a86f2ca0baf908fe0b2e4bcc396f66989ae Mon Sep 17 00:00:00 2001 From: "Andrew J. Schorr" Date: Thu, 26 May 2016 12:13:02 -0400 Subject: Optimize API function argument retrieval to eliminate a duplicate call to get_argument. --- ChangeLog | 16 ++++++++++++++++ awk.h | 6 +++--- ext.c | 16 +--------------- gawkapi.c | 6 +++--- 4 files changed, 23 insertions(+), 21 deletions(-) diff --git a/ChangeLog b/ChangeLog index e066765e..8d408e14 100644 --- a/ChangeLog +++ b/ChangeLog @@ -1,3 +1,19 @@ +2016-05-26 Andrew J. Schorr + + * awk.h (get_actual_argument): Add an initial argument containing the + (NODE *) previously returned by get_argument. This allows us to + eliminate a call to get_argument from inside get_actual_argument. + (get_scalar_argument, get_array_argument): Change macro definition to + add an initial node argument to pass through to get_actual_argument. + * ext.c (get_actual_argument): Add initial (NODE *) argument to contain + the value previously returned by get_argument. This allows us to + avoid repeating the call to get_argument. We can also eliminate the + check for a NULL value, since the caller did that already. + * gawkapi.c (api_get_argument): Pass (NODE *) returned by get_argument + to get_array_argument and get_scalar_argument. + (api_set_argument): Pass (NODE *) returned by get_argument to + get_array_argument. + 2016-05-12 Arnold Robbins * str_array.c (str_lookup): Remove MAYBE_NUM from subscript flags. diff --git a/awk.h b/awk.h index 925a9d61..ecc87f10 100644 --- a/awk.h +++ b/awk.h @@ -1441,9 +1441,9 @@ extern void close_extensions(void); extern void make_old_builtin(const char *, NODE *(*)(int), int); extern awk_bool_t make_builtin(const awk_ext_func_t *); extern NODE *get_argument(int); -extern NODE *get_actual_argument(int, bool, bool); -#define get_scalar_argument(i, opt) get_actual_argument((i), (opt), false) -#define get_array_argument(i, opt) get_actual_argument((i), (opt), true) +extern NODE *get_actual_argument(NODE *, int, bool, bool); +#define get_scalar_argument(n, i, opt) get_actual_argument((n), (i), (opt), false) +#define get_array_argument(n, i, opt) get_actual_argument((n), (i), (opt), true) #endif /* field.c */ extern void init_fields(void); diff --git a/ext.c b/ext.c index cf813674..c3e34b1c 100644 --- a/ext.c +++ b/ext.c @@ -334,28 +334,14 @@ get_argument(int i) */ NODE * -get_actual_argument(int i, bool optional, bool want_array) +get_actual_argument(NODE *t, int i, bool optional, bool want_array) { - NODE *t; char *fname; - int pcount; INSTRUCTION *pc; pc = TOP()->code_ptr; /* Op_ext_builtin instruction */ fname = (pc + 1)->func_name; - pcount = (pc + 1)->expr_count; - t = get_argument(i); - if (t == NULL) { - if (i >= pcount) /* must be fatal */ - fatal(_("function `%s' defined to take no more than %d argument(s)"), - fname, pcount); - if (! optional) - fatal(_("function `%s': missing argument #%d"), - fname, i + 1); - return NULL; - } - if (t->type == Node_var_new) { if (want_array) return force_array(t, false); diff --git a/gawkapi.c b/gawkapi.c index 3b495452..779506cd 100644 --- a/gawkapi.c +++ b/gawkapi.c @@ -82,7 +82,7 @@ api_get_argument(awk_ext_id_t id, size_t count, array: /* get the array here */ - arg = get_array_argument(count, false); + arg = get_array_argument(arg, count, false); if (arg == NULL) return awk_false; @@ -90,7 +90,7 @@ array: scalar: /* at this point we have a real type that is not an array */ - arg = get_scalar_argument(count, false); + arg = get_scalar_argument(arg, count, false); if (arg == NULL) return awk_false; @@ -120,7 +120,7 @@ api_set_argument(awk_ext_id_t id, || arg->type != Node_var_new) return awk_false; - arg = get_array_argument(count, false); + arg = get_array_argument(arg, count, false); if (arg == NULL) return awk_false; -- cgit v1.2.3 From 2af19378be4e7eeec7754053070cf4bad035d7b1 Mon Sep 17 00:00:00 2001 From: "Arnold D. Robbins" Date: Thu, 26 May 2016 16:17:12 -0400 Subject: Improve definition of fatal macro. --- ChangeLog | 10 ++++++++-- awk.h | 2 +- 2 files changed, 9 insertions(+), 3 deletions(-) diff --git a/ChangeLog b/ChangeLog index 8d408e14..ee6ff430 100644 --- a/ChangeLog +++ b/ChangeLog @@ -1,7 +1,13 @@ +2016-05-26 Arnold D. Robbins + + * awk.h [fatal]: Make parentheses and use of indirection + consistent with warning and lintwarn. Thanks to Andrew Schorr + for pointing this out. + 2016-05-26 Andrew J. Schorr - * awk.h (get_actual_argument): Add an initial argument containing the - (NODE *) previously returned by get_argument. This allows us to + * awk.h (get_actual_argument): Add an initial argument containing + the (NODE *) previously returned by get_argument. This allows us to eliminate a call to get_argument from inside get_actual_argument. (get_scalar_argument, get_array_argument): Change macro definition to add an initial node argument to pass through to get_actual_argument. diff --git a/awk.h b/awk.h index ecc87f10..8f3b1d48 100644 --- a/awk.h +++ b/awk.h @@ -1271,7 +1271,7 @@ DEREF(NODE *r) #define efree(p) free(p) -#define fatal set_loc(__FILE__, __LINE__), r_fatal +#define fatal (*(set_loc(__FILE__, __LINE__), r_fatal)) extern jmp_buf fatal_tag; extern bool fatal_tag_valid; -- cgit v1.2.3 From 14d958c82b939d26ad37ab0c0debfedf780404d0 Mon Sep 17 00:00:00 2001 From: "Arnold D. Robbins" Date: Thu, 26 May 2016 16:22:59 -0400 Subject: Further fix for MAYBE_NUM values used as array subscripts. --- ChangeLog | 4 ++++ str_array.c | 22 +++++++++++++--------- 2 files changed, 17 insertions(+), 9 deletions(-) diff --git a/ChangeLog b/ChangeLog index ee6ff430..4b6e88db 100644 --- a/ChangeLog +++ b/ChangeLog @@ -3,6 +3,10 @@ * awk.h [fatal]: Make parentheses and use of indirection consistent with warning and lintwarn. Thanks to Andrew Schorr for pointing this out. + * str_array.c (str_lookup): Move test for MAYBE_NUM to where + we duplicate the subscript. Removing it across the board is + wrong if there are multiple references to the value. Thanks + to Andrew Schorr for discussion and test case. 2016-05-26 Andrew J. Schorr diff --git a/str_array.c b/str_array.c index 9514d515..1de432c7 100644 --- a/str_array.c +++ b/str_array.c @@ -137,7 +137,18 @@ str_lookup(NODE *symbol, NODE *subs) hash1 = code1 % (unsigned long) symbol->array_size; } - if (subs->stfmt != -1) { + + /* + * Repeat after me: "Array indices are always strings." + * "Array indices are always strings." + * "Array indices are always strings." + * "Array indices are always strings." + * .... + * If subs is a STRNUM, copy it; don't clear the MAYBE_NUM + * flag on it since other variables could be using the same + * reference-counted value. + */ + if (subs->stfmt != -1 || (subs->flags & MAYBE_NUM) != 0) { NODE *tmp; /* @@ -168,14 +179,7 @@ str_lookup(NODE *symbol, NODE *subs) subs = dupnode(subs); } - /* - * Repeat after me: "Array indices are always strings." - * "Array indices are always strings." - * "Array indices are always strings." - * "Array indices are always strings." - * .... - */ - subs->flags &= ~MAYBE_NUM; + assert((subs->flags & MAYBE_NUM) == 0); getbucket(b); b->ahnext = symbol->buckets[hash1]; -- cgit v1.2.3 From 4d634960a411622cfb2e757d8e98c84f7601e09e Mon Sep 17 00:00:00 2001 From: "Arnold D. Robbins" Date: Thu, 26 May 2016 16:27:43 -0400 Subject: New test, arrayind2. --- test/ChangeLog | 6 ++++++ test/Makefile.am | 4 +++- test/Makefile.in | 9 ++++++++- test/Maketests | 5 +++++ test/arrayind2.awk | 8 ++++++++ test/arrayind2.ok | 2 ++ 6 files changed, 32 insertions(+), 2 deletions(-) create mode 100644 test/arrayind2.awk create mode 100644 test/arrayind2.ok diff --git a/test/ChangeLog b/test/ChangeLog index c395fc69..b9689557 100644 --- a/test/ChangeLog +++ b/test/ChangeLog @@ -1,3 +1,9 @@ +2016-05-26 Arnold D. Robbins + + * Makefile.am (arrayind2): New test. + * arrayind2.awk, arrayind2.ok: New files. + Thanks to Andrew J. Schorr . + 2016-05-25 Arnold D. Robbins * arrayind1.awk: Flush writes to stderr. We hope this helps diff --git a/test/Makefile.am b/test/Makefile.am index 63990647..8d62e802 100644 --- a/test/Makefile.am +++ b/test/Makefile.am @@ -55,6 +55,8 @@ EXTRA_DIST = \ arrayind1.awk \ arrayind1.in \ arrayind1.ok \ + arrayind2.awk \ + arrayind2.ok \ arrayparm.awk \ arrayparm.ok \ arrayprm2.awk \ @@ -1094,7 +1096,7 @@ CLEANFILES = core core.* fmtspcl.ok # try to keep these sorted. each letter starts a new line BASIC_TESTS = \ - addcomma anchgsub argarray arrayind1 arrayparm arrayprm2 arrayprm3 \ + addcomma anchgsub argarray arrayind1 arrayind2 arrayparm arrayprm2 arrayprm3 \ arrayref arrymem1 arryref2 arryref3 arryref4 arryref5 arynasty \ arynocls aryprm1 aryprm2 aryprm3 aryprm4 aryprm5 aryprm6 aryprm7 \ aryprm8 aryprm9 arysubnm asgext awkpath \ diff --git a/test/Makefile.in b/test/Makefile.in index cc44371c..37e9b9b5 100644 --- a/test/Makefile.in +++ b/test/Makefile.in @@ -312,6 +312,8 @@ EXTRA_DIST = \ arrayind1.awk \ arrayind1.in \ arrayind1.ok \ + arrayind2.awk \ + arrayind2.ok \ arrayparm.awk \ arrayparm.ok \ arrayprm2.awk \ @@ -1350,7 +1352,7 @@ CLEANFILES = core core.* fmtspcl.ok # try to keep these sorted. each letter starts a new line BASIC_TESTS = \ - addcomma anchgsub argarray arrayind1 arrayparm arrayprm2 arrayprm3 \ + addcomma anchgsub argarray arrayind1 arrayind2 arrayparm arrayprm2 arrayprm3 \ arrayref arrymem1 arryref2 arryref3 arryref4 arryref5 arynasty \ arynocls aryprm1 aryprm2 aryprm3 aryprm4 aryprm5 aryprm6 aryprm7 \ aryprm8 aryprm9 arysubnm asgext awkpath \ @@ -2699,6 +2701,11 @@ arrayind1: @AWKPATH="$(srcdir)" $(AWK) -f $@.awk < "$(srcdir)"/$@.in >_$@ 2>&1 || echo EXIT CODE: $$? >>_$@ @-$(CMP) "$(srcdir)"/$@.ok _$@ && rm -f _$@ +arrayind2: + @echo $@ + @AWKPATH="$(srcdir)" $(AWK) -f $@.awk >_$@ 2>&1 || echo EXIT CODE: $$? >>_$@ + @-$(CMP) "$(srcdir)"/$@.ok _$@ && rm -f _$@ + arrayparm: @echo $@ @AWKPATH="$(srcdir)" $(AWK) -f $@.awk >_$@ 2>&1 || echo EXIT CODE: $$? >>_$@ diff --git a/test/Maketests b/test/Maketests index a831496c..0895d23d 100644 --- a/test/Maketests +++ b/test/Maketests @@ -15,6 +15,11 @@ arrayind1: @AWKPATH="$(srcdir)" $(AWK) -f $@.awk < "$(srcdir)"/$@.in >_$@ 2>&1 || echo EXIT CODE: $$? >>_$@ @-$(CMP) "$(srcdir)"/$@.ok _$@ && rm -f _$@ +arrayind2: + @echo $@ + @AWKPATH="$(srcdir)" $(AWK) -f $@.awk >_$@ 2>&1 || echo EXIT CODE: $$? >>_$@ + @-$(CMP) "$(srcdir)"/$@.ok _$@ && rm -f _$@ + arrayparm: @echo $@ @AWKPATH="$(srcdir)" $(AWK) -f $@.awk >_$@ 2>&1 || echo EXIT CODE: $$? >>_$@ diff --git a/test/arrayind2.awk b/test/arrayind2.awk new file mode 100644 index 00000000..200694eb --- /dev/null +++ b/test/arrayind2.awk @@ -0,0 +1,8 @@ +BEGIN { + pos[0] = 0 + posout[0] = 0 + split("00000779770060", f) # f[1] is a strnum + print typeof(f[1]) + pos[f[1]] = 1 + print typeof(f[1]) +} diff --git a/test/arrayind2.ok b/test/arrayind2.ok new file mode 100644 index 00000000..335b2005 --- /dev/null +++ b/test/arrayind2.ok @@ -0,0 +1,2 @@ +strnum +strnum -- cgit v1.2.3