Skip to content

Conversation

devnexen
Copy link
Member

…ay element.

* as a result of that.
*/
str = zval_get_tmp_string(elem, &tmp_str);
if (!try_convert_to_string(elem)) {
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I believe this will implicitly modify the array. Can we skip the entire iteration when the parameter is unsed?

@nielsdos
Copy link
Member

nielsdos commented Nov 15, 2024

Honestly, I think throwing away the exception is pretty bad. What if the entry is actually used and the __toString() legitimately throws? I also think this isn't a bug in the first place:
If the key is used then we should throw, but if it isn't used then why is it passed in the first place?
If we were a statically typed language we'd have a typehint array<string> and the coercion would fail upfront.

@devnexen
Copy link
Member Author

devnexen commented Nov 15, 2024

I kind of agree with you (one of the reasons I keep it as draft), I was just trying to see what s possible but ultimately looking at the code so many times, it is mostly good as is..

@iluuu1994
Copy link
Member

I agree that exceptions should not be discarded. What I was suggesting is trying to skip the coercion to string in the first place. But as Niels has mentioned, IMO this is definitely not a bug, and might not even be a worthwhile feature.

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

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants