Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

ternary reference assignment should be legal #18043

Open
divinity76 opened this issue Mar 13, 2025 · 6 comments
Open

ternary reference assignment should be legal #18043

divinity76 opened this issue Mar 13, 2025 · 6 comments

Comments

@divinity76
Copy link
Contributor

Description

I wanted to do

foreach((is_array($value) ? $value['images'] : $value->images) as &$image) {
// process $image
}

but that silently fail (!) because foreach will create an internal copy and iterate the copy (see `&$image);
So instead I tried:

$iter = is_array($value) ? (&$value['images']) : (&$value->images);
foreach($iter as &$image) {
// process $image
}

but that fail because ternary reference assignment is illegal (?)
So instead I have to do

if(is_array($value)){
    $iter = &$value['images'];
} else {
    $iter = &$value->images;
}
foreach($iter as &$image) {
// process $image
}

which works.
Now it feels like I'm fighting the language itself.

@iluuu1994
Copy link
Member

iluuu1994 commented Mar 13, 2025

Ternaries are not special in that regard. The same occurs when iterating by-ref over an array returned from a function by-value. The ref operation itself does not fail, it's just performed it on a temporary copy of the array, hence not being observable.

@divinity76
Copy link
Contributor Author

divinity76 commented Mar 13, 2025

I see, but

$iter = is_array($value) ? (&$value['images']) : (&$value->images);

fails with

Parse error: syntax error, unexpected token "&" in /in/2l3o7 on line 3

@iluuu1994
Copy link
Member

iluuu1994 commented Mar 13, 2025

Right, we don't have a syntax that does what you need, i.e. fetches a "var" in write context BP_VAR_W, transforms the value to a reference if it isn't already one, and then returns it. I would personally rather see less support for references rather than more, but my feeling aside, this is still a valid feature request. This would need an RFC though.

A warning for by-ref foreach when the array refers to a non-indirect, non-ref value might be possible too. Then again, it's not technically correct that it's fully side-effect free (https://3v4l.org/bnMCa#v8.4.5). Honestly, I would just leave things as is. Another warning also opens more possibility for error handler issues (#12805). But I'm open to hear what others have to say.

@bwoebi
Copy link
Member

bwoebi commented Mar 13, 2025

The primary issue is where references "decay". Currently references in ternaries decay before they're assigned. But they don't really have to.

It would be perfectly fine to have $a ? $b : $c return a reference (if the input is a reference). The only place where it actually needs to decay is when it's ultimately used for another operation.

I think that's rather a shortcoming of the language.

foreach(is_array($value) ? $value['images'] : $value->images as &$image) {
// process $image
}

should just work in my opinion. Like, if you iterate by reference, you explicitly opt in into wanting a reference to the iterated array.

I've been myself annoyed in the past by it and would have liked it to just work.

@iluuu1994
Copy link
Member

Reference decaying is not the only problem though. Nested expressions also need to gain the understanding that they are being fetched by reference, omitting warnings, creating offsets, etc.

@bwoebi
Copy link
Member

bwoebi commented Mar 13, 2025

@iluuu1994 Yeah, ?: and ?? would essentially need to become part of the (delayed) "variable" handling in compiler, which determines which fetch_type to use. But I think it's not as complicated as you try to describe it.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

3 participants