* Valgrind testing, ideas @ 2017-05-26 5:04 Sebastian Gniazdowski 2017-05-26 5:50 ` Bart Schaefer 2017-05-28 19:43 ` mikachu/badarrays (Re: Valgrind testing, ideas) Bart Schaefer 0 siblings, 2 replies; 19+ messages in thread From: Sebastian Gniazdowski @ 2017-05-26 5:04 UTC (permalink / raw) To: zsh-workers [-- Attachment #1: Type: text/plain, Size: 1601 bytes --] Hello The arrlen branch seems to faint. I had this idea of employing valgrind, to get certain about code in the branch. Currently I do this for myself, for my module, as follows: diff --git a/module/ValTest/runtests.zsh b/module/ValTest/runtests.zsh integer success failure skipped retval for file in "${(f)ZTST_testlist}"; do - $ZTST_exe +Z -f $ZTST_srcdir/ztst.zsh $file + if (( ${+commands[colour-valgrind]} )); then + colour-valgrind --leak-check=full $ZTST_exe +Z -f $ZTST_srcdir/ztst.zsh $file + else + valgrind --leak-check=full $ZTST_exe +Z -f $ZTST_srcdir/ztst.zsh $file + fi retval=$? Found 4 errors, instantly. The coloring may seem ridiculous, but the attached screenshot shows it is serious thing. It's like viewing data from XML, the data is logically divided, no need to parse with eyes. The python module colour-valgrind should be rewritten to Zsh. With extended-glob, I see no problems with this, although it's a piece of work. Next is the most important thing: known errors should be removed from output. Like the error from screenshot: array holding database-tied parameters should be left for OS to collect. Has anyone ideas of definition of an error, that could be used to remove blocks from valgrind output? Also I get multiple PIDs from valgrind. I now know that single Zsh instance is used for whole test file. So the multiple PIDs are weird, not sure from where they come from. Last thing, there should be much more test files. Single test per-file, I think. -- Sebastian Gniazdowski psprint /at/ zdharma.org [-- Attachment #2: colovalg.png --] [-- Type: image/png, Size: 217618 bytes --] ^ permalink raw reply [flat|nested] 19+ messages in thread
* Re: Valgrind testing, ideas 2017-05-26 5:04 Valgrind testing, ideas Sebastian Gniazdowski @ 2017-05-26 5:50 ` Bart Schaefer 2017-05-26 7:57 ` Sebastian Gniazdowski 2017-05-28 19:43 ` mikachu/badarrays (Re: Valgrind testing, ideas) Bart Schaefer 1 sibling, 1 reply; 19+ messages in thread From: Bart Schaefer @ 2017-05-26 5:50 UTC (permalink / raw) To: Sebastian Gniazdowski; +Cc: zsh-workers On Thu, May 25, 2017 at 10:04 PM, Sebastian Gniazdowski <psprint@zdharma.org> wrote: > > The arrlen branch seems to faint. Sorry, what's the "arrlen branch"? > Also I get multiple PIDs from valgrind. Not all tests use the single zsh. Some explicitly re-invoke $ZTST_exe, others run in subshells. In fact when I want to add valgrind to a "make check", I insert local ZTST_exe=(valgrind $ZTST_exe) > Last thing, there should be much more test files. Single test per-file, I think. I'm not thrilled with this suggestion. There are a lot of tests that depend on sharing a single %prep (and corresponding %cleanup) section, for example, and also a number of tests that depend on cascading one after the other. The amount of duplicated code that would result from attempting to make these into singleton files would lead to other problems. ^ permalink raw reply [flat|nested] 19+ messages in thread
* Re: Valgrind testing, ideas 2017-05-26 5:50 ` Bart Schaefer @ 2017-05-26 7:57 ` Sebastian Gniazdowski 2017-05-26 10:56 ` Daniel Shahaf 0 siblings, 1 reply; 19+ messages in thread From: Sebastian Gniazdowski @ 2017-05-26 7:57 UTC (permalink / raw) To: Bart Schaefer; +Cc: zsh-workers On 26 maja 2017 at 07:50:59, Bart Schaefer (schaefer@brasslantern.com) wrote: > On Thu, May 25, 2017 at 10:04 PM, Sebastian Gniazdowski > wrote: > > > > The arrlen branch seems to faint. > > Sorry, what's the "arrlen branch"? Mikachu's badarrays branch > Not all tests use the single zsh. Some explicitly re-invoke > $ZTST_exe, others run in subshells. In fact when I want to add > valgrind to a "make check", I insert > local ZTST_exe=(valgrind $ZTST_exe) Thanks > > Last thing, there should be much more test files. Single test per-file, I think. > > I'm not thrilled with this suggestion. There are a lot of tests that > depend on sharing a single %prep (and corresponding %cleanup) section, > for example, and also a number of tests that depend on cascading one > after the other. The amount of duplicated code that would result from > attempting to make these into singleton files would lead to other > problems. Yes it is problematic, I've blindly created 28 test files from redis-adapted-V11db_gdbm, and noted that they fail, because database contents is cascaded through test, I rely on some keys from previous test to exist, etc. So I run all-tests in single file too, through valgrind, currently. But if lessening of load for eyes is wanted, more automatic-pin-pointing-capable output is wanted, then single test case per file is rather the way to go. -- Sebastian Gniazdowski psprint /at/ zdharma.org ^ permalink raw reply [flat|nested] 19+ messages in thread
* Re: Valgrind testing, ideas 2017-05-26 7:57 ` Sebastian Gniazdowski @ 2017-05-26 10:56 ` Daniel Shahaf 2017-05-26 23:48 ` Bart Schaefer 0 siblings, 1 reply; 19+ messages in thread From: Daniel Shahaf @ 2017-05-26 10:56 UTC (permalink / raw) To: zsh-workers Sebastian Gniazdowski wrote on Fri, 26 May 2017 09:57 +0200: > But if lessening of load for eyes is wanted, more automatic-pin-pointing-capable output is wanted, then single test case per file is rather the way to go. Just teach the test runner to only run a single test from a file. ^ permalink raw reply [flat|nested] 19+ messages in thread
* Re: Valgrind testing, ideas 2017-05-26 10:56 ` Daniel Shahaf @ 2017-05-26 23:48 ` Bart Schaefer 0 siblings, 0 replies; 19+ messages in thread From: Bart Schaefer @ 2017-05-26 23:48 UTC (permalink / raw) To: zsh-workers On May 26, 10:56am, Daniel Shahaf wrote: } } Just teach the test runner to only run a single test from a file. There are a number of tests that cascade off one another; it's no more possible in the general case to run one test from a larger file than to break the tests up in to separate files. Running one test from a file might solve the %prep/%cleanup problem, but not all the problems. ^ permalink raw reply [flat|nested] 19+ messages in thread
* mikachu/badarrays (Re: Valgrind testing, ideas) 2017-05-26 5:04 Valgrind testing, ideas Sebastian Gniazdowski 2017-05-26 5:50 ` Bart Schaefer @ 2017-05-28 19:43 ` Bart Schaefer 2017-05-29 13:21 ` Mikael Magnusson 1 sibling, 1 reply; 19+ messages in thread From: Bart Schaefer @ 2017-05-28 19:43 UTC (permalink / raw) To: zsh-workers On May 26, 7:04am, Sebastian Gniazdowski wrote: } } The arrlen branch seems to faint. I had this idea of employing } valgrind, to get certain about code in the branch. } } Found 4 errors, instantly. I took a stab at merging mikachu/badarrays into the current master, and for the most part it was straightforward, but there are too many conflicting changes to Src/params.c (different numbers of values in the replacements for the whole suite of IPDEF macros, of which more have been added in the meantime; bug fixes in getarrvalue() that overlap the PM_CACHLEN changes). I gave up on attempting to follow what would need to be be edited to bring the latest code up to date. I could push the current broken (won't compile) state up to the origin (I branched my merge as schaefer/badarrays) if Mikachu is interested in cleaning up params.c. In the meantime it's probably not worthwhile to attempt memory checking on mikchu/badarrays. ^ permalink raw reply [flat|nested] 19+ messages in thread
* Re: mikachu/badarrays (Re: Valgrind testing, ideas) 2017-05-28 19:43 ` mikachu/badarrays (Re: Valgrind testing, ideas) Bart Schaefer @ 2017-05-29 13:21 ` Mikael Magnusson 2017-05-31 5:47 ` Bart Schaefer 0 siblings, 1 reply; 19+ messages in thread From: Mikael Magnusson @ 2017-05-29 13:21 UTC (permalink / raw) To: Bart Schaefer; +Cc: zsh workers On Sun, May 28, 2017 at 9:43 PM, Bart Schaefer <schaefer@brasslantern.com> wrote: > On May 26, 7:04am, Sebastian Gniazdowski wrote: > } > } The arrlen branch seems to faint. I had this idea of employing > } valgrind, to get certain about code in the branch. > } > } Found 4 errors, instantly. > > I took a stab at merging mikachu/badarrays into the current master, > and for the most part it was straightforward, but there are too many > conflicting changes to Src/params.c (different numbers of values in > the replacements for the whole suite of IPDEF macros, of which more > have been added in the meantime; bug fixes in getarrvalue() that > overlap the PM_CACHLEN changes). I gave up on attempting to follow > what would need to be be edited to bring the latest code up to date. > > I could push the current broken (won't compile) state up to the origin > (I branched my merge as schaefer/badarrays) if Mikachu is interested in > cleaning up params.c. In the meantime it's probably not worthwhile to > attempt memory checking on mikchu/badarrays. Well, it never worked in the first place, that's why i put "bad" in the name. It's basically just a documentation of the places I found that would need to be updated to use a cached length value, but since it doesn't work I guess I didn't find all of them, or I misunderstood some parts of the code that I changed... -- Mikael Magnusson ^ permalink raw reply [flat|nested] 19+ messages in thread
* Re: mikachu/badarrays (Re: Valgrind testing, ideas) 2017-05-29 13:21 ` Mikael Magnusson @ 2017-05-31 5:47 ` Bart Schaefer 2017-06-01 16:31 ` Sebastian Gniazdowski ` (2 more replies) 0 siblings, 3 replies; 19+ messages in thread From: Bart Schaefer @ 2017-05-31 5:47 UTC (permalink / raw) To: zsh workers On May 29, 3:21pm, Mikael Magnusson wrote: } } Well, it never worked in the first place, that's why i put "bad" in } the name. It's basically just a documentation of the places I found } that would need to be updated to use a cached length value, but since } it doesn't work I guess I didn't find all of them, or I misunderstood } some parts of the code that I changed... It also appears we've run out of bits for PM_CACHELEN and PM_CHECKEN, the 19 and 20 bit positions are are now occupied by PM_DONTIMPORT_SUID and PM_SINGLE, though I suppose PM_KSHSTORED and PM_ZSHSTORED could do double duty as they won't apply to arrays and functions at once. With respect to "it doesn't work" -- in commits f4ab07b4 and b7c2ddf6 you say you're adding typeset -C and typeset -c to control PM_CACHELEN and PM_CHECKLEN but looking at the diffs you only added the argument parsing; I can't find anywhere that PM_CACHELEN or PM_CHECKLEN are ever assigned to the parameter flags? I believe I've now merged params.c ... I tried forcing both of the new flags on for non-special PM_ARRAY params in createparam(), and all tests seem to pass except those involving the $match array, which implies one of us did something wrong in setarrvalue() for post_assignment_length. I think. -- Barton E. Schaefer ^ permalink raw reply [flat|nested] 19+ messages in thread
* Re: mikachu/badarrays (Re: Valgrind testing, ideas) 2017-05-31 5:47 ` Bart Schaefer @ 2017-06-01 16:31 ` Sebastian Gniazdowski 2017-06-01 20:35 ` Bart Schaefer 2017-06-01 21:22 ` Mikael Magnusson 2017-06-07 10:53 ` PM_ flags (Re: mikachu/badarrays ...) Oliver Kiddle 2 siblings, 1 reply; 19+ messages in thread From: Sebastian Gniazdowski @ 2017-06-01 16:31 UTC (permalink / raw) To: Bart Schaefer, zsh workers On 31 maja 2017 at 07:47:59, Bart Schaefer (schaefer@brasslantern.com) wrote: > With respect to "it doesn't work" -- in commits f4ab07b4 and b7c2ddf6 > you say you're adding typeset -C and typeset -c to control PM_CACHELEN > and PM_CHECKLEN but looking at the diffs you only added the argument > parsing; I can't find anywhere that PM_CACHELEN or PM_CHECKLEN are ever > assigned to the parameter flags? > > I believe I've now merged params.c ... I tried forcing both of the new > flags on for non-special PM_ARRAY params in createparam(), and all tests > seem to pass except those involving the $match array, which implies one > of us did something wrong in setarrvalue() for post_assignment_length. > I think. I've got slightly demotivated after Mikachu said the code doesn't work, but you say you have merged this and it works? As for my thesis on Valgrind testing, I was now running for a week the many-test-cases-in-single-test-run method, and I can now say this is a very plausible method. Valgrind messages appear, one just observes, scrolling terminal, and if a report appears, it doesn't have to be connected with distinct test case, it has its own context which is the source code and the single line pointed by Valgrind. I'm a happy Valgrind user with errors like pm->gsu.s = &stdscalar_gsu assigned instead of pm->gsu.a = &stdarray_gsu reported (interestingly this one indirectly, as memory leak). One tip maybe, I do: if [[ 2 -nt 1 ]]; then colour-valgrind $ZTST_exe +Z -f $ZTST_srcdir/ztst.zsh $file else colour-valgrind --leak-check=full $ZTST_exe +Z -f $ZTST_srcdir/ztst.zsh $file fi to easily switch by touch {1|2} between errors checking and leak checking. Actually have also file "3", to disable the "possibly lost" reports. -- Sebastian Gniazdowski psprint /at/ zdharma.org ^ permalink raw reply [flat|nested] 19+ messages in thread
* Re: mikachu/badarrays (Re: Valgrind testing, ideas) 2017-06-01 16:31 ` Sebastian Gniazdowski @ 2017-06-01 20:35 ` Bart Schaefer 2017-06-02 1:40 ` Sebastian Gniazdowski 0 siblings, 1 reply; 19+ messages in thread From: Bart Schaefer @ 2017-06-01 20:35 UTC (permalink / raw) To: zsh workers On Jun 1, 6:31pm, Sebastian Gniazdowski wrote: } Subject: Re: mikachu/badarrays (Re: Valgrind testing, ideas) } } I've got slightly demotivated after Mikachu said the code doesn't } work, but you say you have merged this and it works? I said it mostly passes tests. There's still something screwy about its interaction with the newer optimizations to re-use array storage when growing/shrinking an array, which for purposes of the tests is invoked whenever the $match array is changed by the pattern code; so all the tests that require $match are failing. ^ permalink raw reply [flat|nested] 19+ messages in thread
* Re: mikachu/badarrays (Re: Valgrind testing, ideas) 2017-06-01 20:35 ` Bart Schaefer @ 2017-06-02 1:40 ` Sebastian Gniazdowski 2017-06-02 22:00 ` Bart Schaefer 2017-06-04 0:49 ` Bart Schaefer 0 siblings, 2 replies; 19+ messages in thread From: Sebastian Gniazdowski @ 2017-06-02 1:40 UTC (permalink / raw) To: Bart Schaefer, zsh workers On 1 czerwca 2017 at 22:35:55, Bart Schaefer (schaefer@brasslantern.com) wrote: > I said it mostly passes tests. There's still something screwy about > its interaction with the newer optimizations to re-use array storage > when growing/shrinking an array, which for purposes of the tests is > invoked whenever the $match array is changed by the pattern code; so > all the tests that require $match are failing. Ah ok, mostly passes tests. I think the optimizations should agree with caching of arrlen(), the new array size is computed as before (the way it is computed didn't change), and cached array len can be stored. I'm curious how the screen saver zmorpho would perform, as far as I remember it was ~360s with no optimizations, 40s with them, 25s with hashes used to store data under integer indices. I would test cached arrlen(), would it be 30, 20, 10 seconds, that's a puzzle, arrlen() was top function in callgrind run. Is there code available somewhere? -- Sebastian Gniazdowski psprint /at/ zdharma.org ^ permalink raw reply [flat|nested] 19+ messages in thread
* Re: mikachu/badarrays (Re: Valgrind testing, ideas) 2017-06-02 1:40 ` Sebastian Gniazdowski @ 2017-06-02 22:00 ` Bart Schaefer 2017-06-04 0:49 ` Bart Schaefer 1 sibling, 0 replies; 19+ messages in thread From: Bart Schaefer @ 2017-06-02 22:00 UTC (permalink / raw) To: zsh workers On Jun 2, 3:40am, Sebastian Gniazdowski wrote: } } Is there code available somewhere? I'd like there to be, but I'm having some trouble creating the branch on the origin to which to push my local branch. Might be that my version of git is too old. ^ permalink raw reply [flat|nested] 19+ messages in thread
* Re: mikachu/badarrays (Re: Valgrind testing, ideas) 2017-06-02 1:40 ` Sebastian Gniazdowski 2017-06-02 22:00 ` Bart Schaefer @ 2017-06-04 0:49 ` Bart Schaefer 2017-06-04 7:08 ` Sebastian Gniazdowski 1 sibling, 1 reply; 19+ messages in thread From: Bart Schaefer @ 2017-06-04 0:49 UTC (permalink / raw) To: zsh workers On Jun 2, 3:40am, Sebastian Gniazdowski wrote: } } Is there code available somewhere? This has been pushed to the zsh git server as schaefer/badarrays ^ permalink raw reply [flat|nested] 19+ messages in thread
* Re: mikachu/badarrays (Re: Valgrind testing, ideas) 2017-06-04 0:49 ` Bart Schaefer @ 2017-06-04 7:08 ` Sebastian Gniazdowski 0 siblings, 0 replies; 19+ messages in thread From: Sebastian Gniazdowski @ 2017-06-04 7:08 UTC (permalink / raw) To: Bart Schaefer, zsh workers On 4 czerwca 2017 at 02:49:10, Bart Schaefer (schaefer@brasslantern.com) wrote: > On Jun 2, 3:40am, Sebastian Gniazdowski wrote: > } > } Is there code available somewhere? > > This has been pushed to the zsh git server as schaefer/badarrays Tested it with the screensaver that has screen-buffer (24x105 in the test; pushed the code to https://github.com/zdharma/hacking-private/tree/master/array_tests). It is 2 second speed up, from 10.15s to 8.01s. Expected more, but the numbers apparently just sum up this way, even though arrlen_le prevails in callgrind run (below). For hashes the speed is 7.91s, so it seems that hashes are already top-speed containers. Badarrays: 3,143,167,205 itype_end 2,132,125,016 _platform_memmove$VARIANT$Nehalem [/usr/lib/system/libsystem_platform.dylib] 1,269,228,237 stringsubst 1,196,046,124 stringsubst'2 1,112,113,319 ingetc 935,633,068 strcpy [/usr/lib/system/libsystem_c.dylib] 928,176,451 strtod_l [/usr/lib/system/libsystem_c.dylib] 877,921,877 dquote_parse Normal (HEAD): 13,503,068,671 arrlen_le 3,144,808,355 itype_end 2,133,150,428 _platform_memmove$VARIANT$Nehalem [/usr/lib/system/libsystem_platform.dylib] 1,487,072,023 arrlen 1,268,544,885 stringsubst 1,196,046,124 stringsubst'2 1,112,113,371 ingetc 935,739,112 strcpy [/usr/lib/system/libsystem_c.dylib] 927,736,913 strtod_l [/usr/lib/system/libsystem_c.dylib] 877,921,877 dquote_parse -- Sebastian Gniazdowski psprint /at/ zdharma.org ^ permalink raw reply [flat|nested] 19+ messages in thread
* Re: mikachu/badarrays (Re: Valgrind testing, ideas) 2017-05-31 5:47 ` Bart Schaefer 2017-06-01 16:31 ` Sebastian Gniazdowski @ 2017-06-01 21:22 ` Mikael Magnusson 2017-06-01 21:38 ` Bart Schaefer 2017-06-07 10:53 ` PM_ flags (Re: mikachu/badarrays ...) Oliver Kiddle 2 siblings, 1 reply; 19+ messages in thread From: Mikael Magnusson @ 2017-06-01 21:22 UTC (permalink / raw) To: Bart Schaefer; +Cc: zsh workers On Wed, May 31, 2017 at 7:47 AM, Bart Schaefer <schaefer@brasslantern.com> wrote: > On May 29, 3:21pm, Mikael Magnusson wrote: > } > } Well, it never worked in the first place, that's why i put "bad" in > } the name. It's basically just a documentation of the places I found > } that would need to be updated to use a cached length value, but since > } it doesn't work I guess I didn't find all of them, or I misunderstood > } some parts of the code that I changed... > > It also appears we've run out of bits for PM_CACHELEN and PM_CHECKEN, > the 19 and 20 bit positions are are now occupied by PM_DONTIMPORT_SUID > and PM_SINGLE, though I suppose PM_KSHSTORED and PM_ZSHSTORED could do > double duty as they won't apply to arrays and functions at once. > > With respect to "it doesn't work" -- in commits f4ab07b4 and b7c2ddf6 > you say you're adding typeset -C and typeset -c to control PM_CACHELEN > and PM_CHECKLEN but looking at the diffs you only added the argument > parsing; I can't find anywhere that PM_CACHELEN or PM_CHECKLEN are ever > assigned to the parameter flags? > > I believe I've now merged params.c ... I tried forcing both of the new > flags on for non-special PM_ARRAY params in createparam(), and all tests > seem to pass except those involving the $match array, which implies one > of us did something wrong in setarrvalue() for post_assignment_length. > I think. The existing code probably does something fun like assigning these flags based on the order of flags in the TYPESET_OPTSTR define. If you don't use typeset -c/-C for anything, then none of the new code should be activated and it is indeed expected that things would work. -- Mikael Magnusson ^ permalink raw reply [flat|nested] 19+ messages in thread
* Re: mikachu/badarrays (Re: Valgrind testing, ideas) 2017-06-01 21:22 ` Mikael Magnusson @ 2017-06-01 21:38 ` Bart Schaefer 0 siblings, 0 replies; 19+ messages in thread From: Bart Schaefer @ 2017-06-01 21:38 UTC (permalink / raw) To: zsh workers On Jun 1, 11:22pm, Mikael Magnusson wrote: } Subject: Re: mikachu/badarrays (Re: Valgrind testing, ideas) } } On Wed, May 31, 2017 at 7:47 AM, Bart Schaefer } <schaefer@brasslantern.com> wrote: } > It also appears we've run out of bits for PM_CACHELEN and PM_CHECKEN, } > the 19 and 20 bit positions are are now occupied by PM_DONTIMPORT_SUID } > and PM_SINGLE, though I suppose PM_KSHSTORED and PM_ZSHSTORED could do } > double duty as they won't apply to arrays and functions at once. } } The existing code probably does something fun like assigning these } flags based on the order of flags in the TYPESET_OPTSTR define. Indeed, that's the case. So that means it's now essentially impossible to add any more options to typeset, because bit positions 19+ are used by values that must be assigned in the IPDEF(...) structs for params used/set by the shell. In other words adding "cC" at the end of the TYPEST_OPTSTR is now going to be broken by other things, unless we add some special-case code. } If you don't use typeset -c/-C for anything, then none of the new code } should be activated and it is indeed expected that things would work. As I mentioned: } > I tried forcing both of the new } > flags on for non-special PM_ARRAY params in createparam() So they were getting used everywhere, even without "typeset -c". What remained broken after that was the code that resizes an existing array. ^ permalink raw reply [flat|nested] 19+ messages in thread
* PM_ flags (Re: mikachu/badarrays ...) 2017-05-31 5:47 ` Bart Schaefer 2017-06-01 16:31 ` Sebastian Gniazdowski 2017-06-01 21:22 ` Mikael Magnusson @ 2017-06-07 10:53 ` Oliver Kiddle 2017-06-07 18:15 ` Mikael Magnusson 2017-06-07 22:18 ` Bart Schaefer 2 siblings, 2 replies; 19+ messages in thread From: Oliver Kiddle @ 2017-06-07 10:53 UTC (permalink / raw) To: zsh workers On 30 May, Bart wrote: > It also appears we've run out of bits for PM_CACHELEN and PM_CHECKEN, > the 19 and 20 bit positions are are now occupied by PM_DONTIMPORT_SUID > and PM_SINGLE, though I suppose PM_KSHSTORED and PM_ZSHSTORED could do > double duty as they won't apply to arrays and functions at once. Should we perhaps change the flags field in struct param to be a 64-bit value to make room for more flags? Can we portably use something like uint64_t or is it not that simple? In addition to more overloading of function/parameter related flags there are also some mutually exclusive combinations. For example, only one of the PM_TYPE flags can be set so 3 bits could be used in place of 5 but it'd need the PM_TYPE mask adding in lots of places and only gain us 2 bits. There's also the 32nd bit available but to reliably and portably use that do we need to change flags to be an unsigned int? Oliver ^ permalink raw reply [flat|nested] 19+ messages in thread
* Re: PM_ flags (Re: mikachu/badarrays ...) 2017-06-07 10:53 ` PM_ flags (Re: mikachu/badarrays ...) Oliver Kiddle @ 2017-06-07 18:15 ` Mikael Magnusson 2017-06-07 22:18 ` Bart Schaefer 1 sibling, 0 replies; 19+ messages in thread From: Mikael Magnusson @ 2017-06-07 18:15 UTC (permalink / raw) To: Oliver Kiddle; +Cc: zsh workers On Wed, Jun 7, 2017 at 12:53 PM, Oliver Kiddle <okiddle@yahoo.co.uk> wrote: > On 30 May, Bart wrote: > >> It also appears we've run out of bits for PM_CACHELEN and PM_CHECKEN, >> the 19 and 20 bit positions are are now occupied by PM_DONTIMPORT_SUID >> and PM_SINGLE, though I suppose PM_KSHSTORED and PM_ZSHSTORED could do >> double duty as they won't apply to arrays and functions at once. > > Should we perhaps change the flags field in struct param to be a 64-bit > value to make room for more flags? Can we portably use something like > uint64_t or is it not that simple? > > In addition to more overloading of function/parameter related flags > there are also some mutually exclusive combinations. For example, only > one of the PM_TYPE flags can be set so 3 bits could be used in place > of 5 but it'd need the PM_TYPE mask adding in lots of places and only > gain us 2 bits. There's also the 32nd bit available but to reliably and > portably use that do we need to change flags to be an unsigned int? > > Oliver Just to make this clear in case it wasn't, the flags I used in that branch were only for debugging and not needed for the operation at all, so there's no urgent need to fix this unless someone wants to add another flag. -- Mikael Magnusson ^ permalink raw reply [flat|nested] 19+ messages in thread
* Re: PM_ flags (Re: mikachu/badarrays ...) 2017-06-07 10:53 ` PM_ flags (Re: mikachu/badarrays ...) Oliver Kiddle 2017-06-07 18:15 ` Mikael Magnusson @ 2017-06-07 22:18 ` Bart Schaefer 1 sibling, 0 replies; 19+ messages in thread From: Bart Schaefer @ 2017-06-07 22:18 UTC (permalink / raw) To: zsh workers On Jun 7, 12:53pm, Oliver Kiddle wrote: } } Should we perhaps change the flags field in struct param to be a 64-bit } value to make room for more flags? Can we portably use something like } uint64_t or is it not that simple? The main problem is that there is assumed to be a mapping from bit positions to indexes in the TYPESET_OPTSTR string. At some point we started adding PM_FLAGS at the opt of the range to mean things other than what could be assigned via "typeset -C" for some value of C, and now the stack has grown down to meet the heap, so to speak. So we'd have to renumber everything to move to a 64-bit representation, opening up that space in the middle again. This, by the way, is an example of why it's very difficult to build zsh modules outside the tree that can be re-used across zsh versions; there are lots of cases (wordcode being another example) where values that are built into a module at time T could be dangerously wrong for a shell compiled at T+N. ^ permalink raw reply [flat|nested] 19+ messages in thread
end of thread, other threads:[~2017-06-07 22:17 UTC | newest] Thread overview: 19+ messages (download: mbox.gz / follow: Atom feed) -- links below jump to the message on this page -- 2017-05-26 5:04 Valgrind testing, ideas Sebastian Gniazdowski 2017-05-26 5:50 ` Bart Schaefer 2017-05-26 7:57 ` Sebastian Gniazdowski 2017-05-26 10:56 ` Daniel Shahaf 2017-05-26 23:48 ` Bart Schaefer 2017-05-28 19:43 ` mikachu/badarrays (Re: Valgrind testing, ideas) Bart Schaefer 2017-05-29 13:21 ` Mikael Magnusson 2017-05-31 5:47 ` Bart Schaefer 2017-06-01 16:31 ` Sebastian Gniazdowski 2017-06-01 20:35 ` Bart Schaefer 2017-06-02 1:40 ` Sebastian Gniazdowski 2017-06-02 22:00 ` Bart Schaefer 2017-06-04 0:49 ` Bart Schaefer 2017-06-04 7:08 ` Sebastian Gniazdowski 2017-06-01 21:22 ` Mikael Magnusson 2017-06-01 21:38 ` Bart Schaefer 2017-06-07 10:53 ` PM_ flags (Re: mikachu/badarrays ...) Oliver Kiddle 2017-06-07 18:15 ` Mikael Magnusson 2017-06-07 22:18 ` Bart Schaefer
Code repositories for project(s) associated with this public inbox https://git.vuxu.org/mirror/zsh/ This is a public inbox, see mirroring instructions for how to clone and mirror all data and code used for this inbox; as well as URLs for NNTP newsgroup(s).