Implement ?-> optional chaining operator (PPC0021)#24440
Conversation
a0f6f00 to
4540c92
Compare
|
The core team's policy on accepting LLM-generated code has not yet been determined, but (as an outside observer) I would recommend clearly marking in the commit message whether any code was generated by an LLM. |
0bf6530 to
af9a1e3
Compare
af9a1e3 to
22810d6
Compare
22810d6 to
b0e82b7
Compare
b0e82b7 to
2bdaf06
Compare
Thank you, I have added a note. |
|
|
||
| my $i = 0; | ||
| my $u = undef; | ||
| $u?->[$i++]; # $i is still 0 — subscript not evaluated |
There was a problem hiding this comment.
It feels surprising, but it's doing exactly what the PPC describes.
The PPC has this example:
# $y = ();
# if (defined $x) {
# my $tmp = $i++;
# if (defined $x->{$tmp}) {
# $y = $x->{$tmp}->[++$i]
# }
# }
$y = $x?->{$i++}?->[++$i];
Since nothing is supposed to happen on the right-hand side if the left-hand side is undef, then $i doesn't change if $x is undef, and it's incremented only once if $x->{$i++} is undef, and twice in the happy path.
There was a problem hiding this comment.
This example is longer, but still surprising.
I think my main problem is that ++ doesn't feel like a "behaviour" one can/would short circuit.
I guess without it the example would be crowded with explicit $i=$i+1 # add one to i type stuff
There was a problem hiding this comment.
Not all languages behave exactly like that, but most do, when it comes to optional chaining. And also the PPC :-)
There was a problem hiding this comment.
I think my main problem is that ++ doesn't feel like a "behaviour" one can/would short circuit.
foo() or $i++ would also shortcut the $i++, so it's not exactly a new type of behavior.
There was a problem hiding this comment.
And if ?-> was spelt minus greater than I'd be 100% in the metaphor
There was a problem hiding this comment.
Hate to quote PHP documentation, but they have a nice section on why they chose full short-circuiting and also a table for behaviour in other lanugages: https://wiki.php.net/rfc/nullsafe_operator
|
Unless I'm missing something, pp_optarrow() doesn't apply get magic to its LH argument; nor does it deal with overloading. |
| } | ||
|
|
||
| my $u = undef; | ||
| my $obj = Counter->new; |
There was a problem hiding this comment.
It'd be neat to see if ?-> works after an object is DESTROY'ed via undef $obj
|
|
||
| is($u?->[0], undef, 'array: undef lhs gives undef'); | ||
| is($aref?->[0], 10, 'array: first element'); | ||
| is($aref?->[2], 30, 'array: last element'); |
There was a problem hiding this comment.
Your previous test seems to care about the values being strings or not, but this one only uses numbers - is there a strategic reason for doing this for hash-refs but not array-refs?
Maybe you could include other types of thing in the hash/array in these tests?
| } | ||
|
|
||
| # --------------------------------------------------------------------------- | ||
| # postfix dereference: $ref?->@* $ref?->%* |
There was a problem hiding this comment.
Are there other kinds of postfix de-references?
There was a problem hiding this comment.
From the documentation:
$sref->$*; # same as ${ $sref }
$aref->@*; # same as @{ $aref }
$aref->$#*; # same as $#{ $aref }
$href->%*; # same as %{ $href }
$cref->&*; # same as &{ $cref }
$gref->**; # same as *{ $gref }
There was a problem hiding this comment.
Tag yourself, I'm blinded-by-the-light-chicken: ?->**
| # --------------------------------------------------------------------------- | ||
|
|
||
| { | ||
| my $u = undef; |
There was a problem hiding this comment.
Your tests all start with the $u = undef case.
Are there any other interesting ways you can get an undef? Maybe a tie, or a dbmopen if u nasty
| # stack layout. When the LHS was undef, the pop consumed a RHS value, leaving | ||
| # sibling lvalues in the list unassigned. | ||
| # --------------------------------------------------------------------------- | ||
|
|
There was a problem hiding this comment.
Can I STDOUT?->autoflush, a IO::Handle-style?
|
|
||
| # --------------------------------------------------------------------------- | ||
| # No autovivification | ||
| # --------------------------------------------------------------------------- |
There was a problem hiding this comment.
Maybe some coverage for what ?-> does to the aliases one finds in @_ in subs and $_ in for (@...) {}?
| is($obj?->value, 5, 'method: defined lhs with args'); | ||
| is($u?->inc(1), undef, 'method: undef lhs with args gives undef'); | ||
| is($obj?->value, 5, 'method: undef call did not mutate object'); | ||
| } |
There was a problem hiding this comment.
Can you also ?-> a method that Counter doesn't have?
How about a method that's AUTOLOAD'ed‡
Will $obj?->maybe_method shortcircuit the call to AUTOLOAD?
__
‡. in this case, it's me that's nasty
| # --------------------------------------------------------------------------- | ||
|
|
||
| { | ||
| my $u = undef; |
| ); | ||
|
|
||
| # --------------------------------------------------------------------------- | ||
| # map { } — was crashing in S_aassign_scan (peephole optimizer) because |
There was a problem hiding this comment.
This test doesn't look like it's doing anything to capture/guard against a crash, and since it's a "was" it makes me thing the crash is gone?
is that fix part of this PR?
| is($r[1], undef, 'map ?->hash: undef element gives undef'); | ||
| is($r[2], 3, 'map ?->hash: last element defined'); | ||
|
|
||
| my @a = ([10, 20], undef, [30, 40]); |
There was a problem hiding this comment.
Should ?-> give me an empty list in list context?
# @vals = map $_?->{foo}, grep defined $_, @aoh;
@vals = map $_?->{foo}, @aoh;... which seems to suggest it filters out the undef in the middle
(I assume it's a typo to use ?-> in the pre-?-> equivalent code)
There was a problem hiding this comment.
@garu is definitely capable of that, I've seen it many times!
|
This appears to be a large duplication of work. I believe @rabbiveesh is working on this idea. |
|
Ya, I am; this approach looks a lot less centralized than mine, tho I haven't had the chance to fully evaluate |
|
I'd also like to point out that lvalue optchains is an open issue in the PPC repo, and a serious design question to address |
| /* Coderef ?->() with pad-ferry strategy (non-padsv LHS): | ||
| * Pop the CV off the stack, stash it in the temporary pad slot | ||
| * owned by the placeholder OP_PADSV, then jump to inner pushmark. | ||
| * Execution proceeds: pushmark → [args...] → padsv (re-pushes CV | ||
| * at TOS) → ex-rv2cv (noop) → entersub. Works for any args. | ||
| * | ||
| * The ferry PADTMP slot is owned exclusively by the placeholder | ||
| * OP_PADSV child op; the logop has op_targ == 0. To find the slot | ||
| * we walk the tree-sibling chain from the pushmark (op_other). | ||
| * | ||
| * The tree structure depends on call type: | ||
| * | ||
| * Coderef call (?->(args)): | ||
| * ex-list children: [pushmark, args..., ex-rv2cv(nulled)] | ||
| * where ex-rv2cv's first child IS the placeholder padsv. | ||
| * The last sibling is OP_NULL (nulled rv2cv); placeholder | ||
| * is cUNOPx(last_sib)->op_first. | ||
| * | ||
| * Method call (?->method or ?->method(args)): | ||
| * entersub children: [pushmark, placeholder_padsv, method_named] | ||
| * The last sibling is OP_METHOD_NAMED (or similar); the | ||
| * placeholder is the second-to-last sibling. | ||
| * | ||
| * Using the tree-sibling chain (OpSIBLING) rather than the | ||
| * execution chain (op_next) avoids false matches on inner ex-rv2cv | ||
| * ops that appear in the execution chain when args are complex | ||
| * expressions (e.g. nested calls). */ |
There was a problem hiding this comment.
This may delay destruction of the object involved, compare:
tony@venus:.../git/perl6$ ./perl -Ilib -Mfeature=optional_chaining -E 'sub foo { my $x; bless sub { $x }, "Foo"; } foo()?->(); say 1; foo()?->(); say 2; sub Foo::DESTROY { print "DESTROYED\n"; }'
optional chaining is experimental at -e line 1.
optional chaining is experimental at -e line 1.
1
2
DESTROYED
DESTROYED
tony@venus:.../git/perl6$ ./perl -Ilib -Mfeature=optional_chaining -E 'sub foo { my $x; bless sub { $x }, "Foo"; } foo()->(); say 1; foo()->(); say 2; sub Foo::DESTROY { print "DESTROYED\n"; }'
DESTROYED
1
DESTROYED
2
This PR implements the
?->safe navigation / optional chaining operator asspecified by PPC0021.
NOTE: This implementation was developed with very substantial assistance from an LLM (Claude). The code, tests, and documentation were largely generated by the LLM — including solutions to problems I would not have easily arrived at myself.
My own role was primarily one of learning: working through the PPC spec, comparing with other languages' implementations, trying to understand the internals, running tests, and steering the LLM in the right direction when things went wrong. I also spent considerable time verifying that things actually worked and trying to understand why.
If the approach, style, or any detail does not meet the standards of the core team, I am happy to revise or withdraw.
What it does
?->behaves exactly like->except that when the left-hand side isundef, the entire expression short-circuits toundef(scalar context)or an empty list (list context), rather than throwing a runtime error.
Arguments, subscripts, and method names are not evaluated when the
LHS is undef.
Supported forms
$h?->{key}— hash element$a?->[idx]— array element$sub?->(),$sub?->(args)— coderef call$obj?->method,$obj?->method(args)— method call$obj?->&method,$obj?->&method(args)— lexical method call (my method, see perlclass)$ref?->@*,$ref?->%*— postfix dereference$h?->{k} = 42— lvalue (scalar and list assignment)$a?->{b}?->{c}Implementation notes
The feature is gated behind
use feature 'optional_chaining'and isexperimental (warning:
experimental::optional_chaining).The operator is implemented as a new
OP_OPTARROWLOGOP, structurallyanalogous to
OP_AND. The lexer recognises?->as anARROWtokenwith
ival=1; the grammar forks on that flag;Perl_newOPTARROWOP()builds the op tree;
pp_optarrow()executes it.Status
The tests should cover all supported forms, short-circuit semantics, chaining, lvalue use,
no-autovivification, the experimental warning, class methods, lexical
method invocation (
?->&), and structural invariants of the op tree.Known limitations (as per PPC0021 future scope):
Package?->newclass method calls not supportedRequest
I'm submitting this as a starting point for discussion and review rather
than a finished patch. I'd very much welcome feedback on:
pp_optarrow, particularly the pad-ferry strategyfor non-padsv coderef LHS
have missed
guidance on what level of detail is expected
Thank you for your time and consideration.