Zsh Mailing List Archive
Messages sorted by: Reverse Date, Date, Thread, Author

Re: PATCH: Simplify resolve_nameref and setscope



I think this should either be

((pm = (Param)paramtab->getnode(paramtab, asg->value.scalar))

That's almost what I do in the second patch in my message PATCHes: fix reference checks in bin_typeset (it uses realparamtab instead of paramtab). It is however built on top of the first patch in that message. When I apply these two patches at bde2b183f and switch to paramtab, all tests pass. If I apply only the second patch and switch to paramtab: all tests pass. Among the commits that came after bde2b183f, none changed any C file. Could the error be caused by one of your local changes?

Or do you get the error after fixing builtin.c:3132 and applying my patch to simplify resolve_nameref and setscope? In that case it could be because that patch is built on top of the two patches above plus the patch from my message [BUG & tentative PATCH] Invalid call to upscope in createparam and the three patches from my message [PATCHes] Fix issues in setscope. If you didn't include some of these patches, it could explain the error that you observe. When I apply all these patches and switch to paramtab, all tests pass.

I can try to exclude some of these patches but I think that at least some of them are vital.

Extra questions
- Should builtin.c:3132 do the same gimmick as here (and a few other places in builtin.c)?
- I used realparamtab because the line previously called resolve_nameref, which uses realparamtab here and here. Should resolve_nameref use paramtab, too?

I'm actually surprised that "typeset" could possibly apply to something else than the real parameter table but obviously the code in builtin thinks differently.

- It uses a constant "false" which appears nowhere else in our sources.

I can replace "false" and "true" with "0" and "1". In case I should use/do something else, let me know.

- we have been following the practice of always declaring variables at
the start of a code block, even though it's not strictly necessary
with modern compilers.

I will adapt my code to conform.

Philippe


On Mon, Jun 9, 2025 at 12:37 AM Bart Schaefer <schaefer@xxxxxxxxxxxxxxxx> wrote:
On Sun, Jun 8, 2025 at 11:12 AM Bart Schaefer <schaefer@xxxxxxxxxxxxxxxx> wrote:
>
> There are some mechanical problems with this patch:
> - It omits a change on line 3132 of builtin.c (arguments to resolve_nameref).

((pm = (Param)paramtab->getnode(realparamtab, asg->value.scalar))

I think this should either be

((pm = (Param)paramtab->getnode(paramtab, asg->value.scalar))

or just a call to getparamnode() [which it can't be because that's
static to params.c]

In any case if I fix builtin.c:3132 and run "make check", I get

+ params.c:1076: BUG: local parameter is not unset
 inner: foo: attempt to assign array value to non-array
Test ./K01nameref.ztst failed: error output differs from expected as
shown above for:
 (
  inner() { local -n var="${1:?}"; var=(alpha beta gamma); }
  outer() { local foo=outer; inner foo; typeset -p foo; }
  foo=3 ; { outer foo } always { typeset -p foo }
 )
Was testing: up-reference part 11, assignment to enclosing scope, type mismatch


Messages sorted by: Reverse Date, Date, Thread, Author