Re: [BUG & tentative PATCH] Invalid call to upscope in createparam

[Bart Schaefer <schaefer@xxxxxxxxxxxxxxxx>]

On Sat, May 24, 2025 at 3:30 PM Philippe Altherr
<philippe.altherr@xxxxxxxxx> wrote:
>
> While I was looking into implementing the solution for hidden references, I found this call to upscope, which had to be wrong because it applies "base", a property of the referent, to the reference.

That particular call to upscope() is in fact looking for a property of
the reference.  The second parameter to upscope() says where to start
searching.  We're being asked to create a new non-reference parameter
that has the same name as an existing reference.  That means deleting
the reference and making the new assignment.  But if there's yet
another reference at a surrounding scope with the same name, then if
we delete + assign we'll instead assign through the outer reference,
which may be bad.  Instead the parameter at local scope has to be
re-created.

The first part of the test you changed isn't testing that behavior,
rather it's confirming than when the  local reference is a placeholder
hiding a global reference with a valid referent, that doesn't induce
an infinite loop.

The second part of the test whose expected result your patch changed:
  () { typeset -n foo; foo=zz; local zz; foo=zz; print $bar $zz }
should in fact assign local zz=zz via the foo reference, because foo
is not an "upper" reference there.  Why did you think otherwise?

> I don't fully understand how the stop parameter works, so better double-check this.

The stop parameter conveys three pieces of information:  (1) the name
of the reference that began the resolution (stop.name); (2) the
referent name where resolve_nameref should stop searching through a
reference chain (stop.value.scalar); and (3) whether the referent is
itself expected to be another named reference and/or a local in some
scope (stop->flags).  In several tries at this I can't find an example
in the context of your patch where the values in "stop" matter;
"&stop" just must be non-NULL.

I've added a bunch of comments to explain the above and also made the
failed assignment in the first case an explicit warning rather than
just setting $? = 1.  Patch in a separate message.