Subtle signature params interactions with defaulting expression code

[Paul "LeoNerd" Evans]

(CC'ed to Dave M in particular, because there's a lot of overlap with
some signatures unit tests I believe you wrote)

After a bit of a Christmas-time pause, I am now back and working on
finishing my `faster-signatures` branch, the first of the 4 steps to
hopefully get us to named parameters in signatures, plus overall
performance benefits to all signatured code.

My branch now attempts to unconditionally apply the OP_MULTIPARAM
rewrite to any signatured sub, and in doing so manages to break a few of
the t/perf/opcount.t tests (which is entirely to be expected), and in
addition breaks a few of the tests in t/op/signatures.t. Aside from
these, all of core perl passes just fine - though that *may* just be
because we don't use signatures very much even within core. Perhaps
against CPAN there'd be additional issues, but hopefully nothing too
major I can't work out.

The core tests that break here are the entire range of t150 to t162 (I
won't link to exact line numbers as they may move in future):

https://github.com/Perl/perl5/blob/blead/t/op/signatures.t

The scenarios that fail are all to do with what happens if code in a
defaulting expression attempts to modify some other state somewhere,
and fall into a few different categories.

* Modifications of @_

Scenarios t150, t151 and the range t156 to t159 all attempt some
variation of modifying @_ during processing arguments, in order to
see that later parameter variables that would continue to read from
@_ indeed see that modification.

* Modifications of (lexical) parameter variables by captured closures

Scenarios t152 to t155 all perform a weird trick, whereby a named
sub with a signature defines within it another named sub that
captures one of those lexical variables defined in the signature of
the outer one. That inner function modifies the captured lexical
variable in some way. This inner function is called as a defaulting
expression by an earlier parameter of its outer one *before* the
parameter that the inner one captured is processed.

sub outer ( $x = inner(), $y ) {
sub inner { $y = ... }
# test logic here
}

((yes; it's a nontrivial scenario to imagine, you'd best go read
the code in the real unit test file to be clear ;) ))

* Reuse of values in captured parameter variables from lexical
closures

Scenarios t160 and t161 perform an even weirder trick. Here, an
outer signatured function is defined, which contains inside it
another named function which lexically captures one of the
parameter variables. The inner function modifies that variable,
then calls the outer function by passing in elements of that
parameter variable, in order to test reüse of values into that
variable by its own signature handling.

((again, this is really weird; go read the code ;) ))

Currently in my branch, all three of these categories of tests are
failing. In each case, I really don't feel that a lot of effort should
be put into trying to fix it.

The entire point of the OP_MULTIPARAM change is to get to the no-snails
situation where we're no longer populating @_ at all, so any tests for
the interaction of @_ with signatures are necessarily going to become
broken. I vote we just get rid of those tests in that first category.

The tests in the second and third categories are *already* doing
something weird that ought to provoke a warning anyway - namely, having
an outer (named) function that declares a lexical variable that's then
captured by an inner named function. It's analogous to:

$ perl -Mv5.36 -e 'sub outer { my $var; sub inner { undef $var; } }'
Variable "$var" will not stay shared at -e line 1.

The behaviour seen in any of the unit tests in these second two
categories isn't repeatable in general in a real program, because of
that "not stay shared" problem. In more detail: the problem is that the
inner function only captures the $var from outer at CvDEPTH==1; i.e.
the first call. If any deeper recursion happens, then it won't see
those different variables. So quite apart from the fact that I've
changed behaviour, I'm not entirely sure that tests are asserting on
behaviour we'd want to claim is stable anyway.

In more detail on why they fail: tests in the second category (t152 to
t155) fail because of the changed order of argument processing. I
accidentally foreshadowed this in one of my previous emails:

https://www.nntp.perl.org/group/perl.perl5.porters/2024/11/msg269155.html

There is one small detail about this rewrite that occurs to me is
*technically* a user-visible change, but the details and
circumstances around are so obscure it makes me feel like it's one
we don't need to care about.

it goes on...

The test, as written, uses the side-effect of a function invoked in the
defaulting expression of the first parameter variable to cause a
side-effect of populating the second parameter variable (which is a
slurpy array or hash). The test then asserts that this variable gets
emptied again because of parameter processing. That's because in the
currently-existing signature handling code, each parameter variable is
processed individually from start to end. In my new OP_MULTIPARAM code,
all of the assignments from argument values happen first, and only then
afterwards are defaulting expressions run. This means that in the new
code, the side-effect on that variable persists, and the asserting code
does not see it as empty, as it had been expecting. This causes the
test to now fail.

Tests in the third category (t160 and t161) not only rely on this same
inverted lexical sharing with inner defined named subs, but also are
testing refcounts-on-the-stack behaviour. The tests currently fail for
me now but they would pass with a refcounted stack. I could insert some
code to artificially bump the refcount of those stack values if RC
stack is not defined, but this code would only be useful for working
around this *really really odd* inverted nesting case, and are just
pointless slowdown in the 99.9{many more nines}% of all other cases.

At this point, I don't have a good feel for what is the best course of
action on the latter two test categories. Those tests ought not
outright break (currently some of them throw Perl internals errors and
I need to fix that), but I think the exact semantics that are currently
being asserted on are quite fragile and not reflective of any
guarantees we'd actually want to make for real-world code.

How does anyone else feel on these?

[Dave Mitchell]

Post by Paul "LeoNerd" Evans
The core tests that break here are the entire range of t150 to t162 (I
https://github.com/Perl/perl5/blob/blead/t/op/signatures.t
The scenarios that fail are all to do with what happens if code in a
defaulting expression attempts to modify some other state somewhere,
and fall into a few different categories.

TL;DR: you can't assume that fresh lexicals are actually fresh.

In more detail: in a simple statement like:

my $x = 1;

a reasonable assumption might be that $x has either just been freshly
allocated with newSV(), or had been reset to undef on a previous scope
exit. Thus if you chose to optimise the above with a hypothetical combined
"declare and set to integer" op, that op would "know" that the lexical is
already cleaned up, and so can skip doing all the SvTHINKFIRST(sv) etc
stuff. Except it can't. For example:

foo();
my $x = 1;
sub foo { $x = "abc"; }

or:

goto B;
A:
my $x = 1;
goto C;
B:
tie $x, ....;
goto A;
C:

Now for signature parameters, things are a little more restrictive: in
particular, I think we've locked down goto enough in default expressions
that it isn't an issue, but closures still are.

There are two issues to consider: first, whether optimisations are safe
and aren't going to SEGV because of faulty assumptions, and second,
whether the behaviour of perl in those circumstances is sane.

I think your point that these examples are contrived and in real life
would elicit a "won't stay shared" warning", leans towards it being ok
to change the behaviour of such code: as long as the behaviour has some
sort of logical consistency and/or is documented, e.g. "under these
conditions, it is undefined behaviour whether $x will get set to X or
Y".

But however we change the behaviour, it still mustn't crash the
interpreter, or produce bizarre behaviour. So for the first example above,
one *might* expect the behaviour to be documented to leave $x as either 1
or "abc", but never undef or "1abc". Even if it's not sensible for the
user to have done so, the parameter-setting code must still cope with the
possibility of $x being a tied variable whose FETCH returns an LVALUE
overloaded object, or whatever.

So there should still be tests to exercise the limits of any optimised
parameter-setting code, but I would be open to the possibility of changing
their semantics.

But if working on perl bug reports over the years has taught me anything,
it's that if it is physically possible to do combine a whole bunch of
obscure perl edge cases in a bizarre way, some code on CPAN *will* have
done so. And that CPAN module will be in the top 100 most-used.

As for the @_ stuff: I didn't look closely, but I presume that the
semantics might change once @_ becomes the callers args rather than our
args, but those semantics should be consistent with @_ being the caller's
args.

PS the thread which led to me adding those tests was this:

http://nntp.perl.org/group/perl.perl5.porters/238060

where sprout pointed out that I can't assume vars aren't magical etc.

[Paul "LeoNerd" Evans]

On Sat, 18 Jan 2025 13:43:07 +0000
Dave Mitchell <***@iabyn.com> wrote:

...

Post by Dave Mitchell
There are two issues to consider: first, whether optimisations are
safe and aren't going to SEGV because of faulty assumptions, and
second, whether the behaviour of perl in those circumstances is sane.
I think your point that these examples are contrived and in real life
would elicit a "won't stay shared" warning", leans towards it being ok
to change the behaviour of such code: as long as the behaviour has
some sort of logical consistency and/or is documented, e.g. "under
these conditions, it is undefined behaviour whether $x will get set
to X or Y".
But however we change the behaviour, it still mustn't crash the
interpreter, or produce bizarre behaviour. So for the first example
above, one *might* expect the behaviour to be documented to leave $x
as either 1 or "abc", but never undef or "1abc". Even if it's not
sensible for the user to have done so, the parameter-setting code
must still cope with the possibility of $x being a tied variable
whose FETCH returns an LVALUE overloaded object, or whatever.
So there should still be tests to exercise the limits of any optimised
parameter-setting code, but I would be open to the possibility of
changing their semantics.

So, of the various categories mentioned earlier, I've fixed the
implementation so that the "reused values" tests t160 and t161 don't
upset the interpreter, so that category is now entirely resolved.

Of the other lexical capture cases, I've now changed the expected
result of the tests (t152 .. t155), to match the current observed
behaviour. I think the new behaviour is equally defensible as "does
something sensible", and I can adjust various docs and so on to match.
Outlined below is the difference, in some wording that might do for a
perldelta. Really, most of this wording is my defence of why I think
this is a valid change in behaviour to be making ;)


=head2 Altered Evaluation Order of Signature Parameter Processing

In previous versions of Perl, each signature parameter variable was
processed entirely, including assigning a default value from a
defaulting expression, before moving on to the next. In the current
version, first all parameters receive values from the arguments passed
by the caller, before any defaulting expressions are evaluated. These
expressions are still evaluated in left-to-right order, it's just
that they are no longer interleaved with assignments from caller
argument values, but instead happen now in two distinct stages.

In all reasonable cases (and 99% of even unreasonable ones), programs
should not notice any difference in this order. However, in certain
very-contrived examples it is possible to notice this change in
behaviour. These situations all involve creating a lexical capture of
a signature parameter variable by a nested named subroutine defined
inside the subroutine that has the signature.

Consider the following example, which defines two named functions;
the outer one using two named parameters, and the inner one captures
a signature variable of it.

sub outer ( $x = inner(), @rest ) {
sub inner { @rest = (123); }

say "The value of rest is <@rest>";
}

outer(); # no argument values are passed

In previous versions of Perl, first the C<$x> parameter is handled
entirely. As the caller did not pass a value for it, the defaulting
expression must be evaluated, which involves calling the C<inner>
subroutine. That subroutine has a side-effect of assigning the value
of 123 to the C<@rest> array. However, this value gets thrown away
when parameter processing moves on to the second parameter. The
caller did not pass a value for this one either, so the C<@rest>
variable gets cleared. This results in the following output:

The value of rest is <>

In the current version, all of the assignment into parameter
variables from the caller's arguments happens first. This leaves both
the C<$x> and C<@rest> variables empty. Following this, the defaulting
expression is evaluated, which has the side-effect of assigning a
value into C<@rest>, after which no further code will modify it. This
value persists until the C<say> statement, resulting in the following
output instead:

The value of rest is <123>

It should again be stressed that the circumstances required to
observe this behaviour are very obscure, involving these two nested
functions with a lexical variable capture. Because the inner function
is named, and captures a lexical variable of the outer function, it
would in any case provoke a warning at compile-time:

Variable "@rest" will not stay shared at ...

In addition, any use of this kind of arrangement in a real program
would be very fragile, as any recursion of the outer function would
cause different variables to be allocated that are not shared by the
inner function (which is what the warning is informing about). Thus
not only is it a rare and unusual code structure to begin with, but
additionally such a structure would cause fragile and broken
behaviour even without this change to signature parameter handling.

Furthermore, this is only observable with a final slurpy array or
hash parameter. Any subsequent scalar parameters after C<$x> would
have to also have a defaulting expression on them, so if evaluating
an expression before this had some side-effect on a later parameter
variable, that effect would be overwritten by the later defaulting
expression on that later parameter anyway.

Post by Dave Mitchell
But if working on perl bug reports over the years has taught me
anything, it's that if it is physically possible to do combine a
whole bunch of obscure perl edge cases in a bizarre way, some code on
CPAN *will* have done so. And that CPAN module will be in the top 100
most-used.

I wonder if this first step is already at a point where I should throw
it via a smoke-me and get some tests against CPAN for it.

Post by Dave Mitchell
caller's args.

Yeah something we can look more at when I get to the no-snails part of
the change.

[Dave Mitchell]

Post by Paul "LeoNerd" Evans
So, of the various categories mentioned earlier, I've fixed the
implementation so that the "reused values" tests t160 and t161 don't
upset the interpreter, so that category is now entirely resolved.
Of the other lexical capture cases, I've now changed the expected
result of the tests (t152 .. t155), to match the current observed
behaviour.

Sounds plausible,