Skip to content

#61877: Make SimpleXMLElement respect PHP's array rule #5309

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.

Sign up for GitHub

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

Jump to bottom
Closed
wants to merge 1 commit into from
Closed

#61877: Make SimpleXMLElement respect PHP's array rule #5309

wants to merge 1 commit into from

Conversation

carusogabriel
Copy link
Contributor

From the manual:

Additionally the following key casts will occur:

  • Floats are also cast to integers, which means that the fractional part will be truncated. E.g. the key 8.7 will actually be stored under 8.

@carusogabriel
Fix #61877: Make SimpleXMLElement respect PHP's array rule
03ce786 
From the [manual](https://www.php.net/types.array):

> Additionally the following key casts will occur:
> * Floats are also cast to integers, which means that the fractional part will be truncated. E.g. the key 8.7 will actually be stored under 8.
@cmb69
Copy link
Member

cmb69 commented Mar 27, 2020

Thanks for moving this forward! I'm not sure, though, whether this qualifies as bug, since the usual array semantics don't apply to SimpleXMLElement anyway. E.g. using a numeric string as array key casts the string to int, but that would totally break SXE, since non-integer keys are supposed to get/set attribute values (see https://3v4l.org/kET56). Treating floats as integers may make sense, but due to the BC break we shouldn't do that for PHP 7.

@carusogabriel
Copy link
Contributor Author

since the usual array semantics don't apply to SimpleXMLElement anyway

If this is the case then yes, it isn't a bug but more of a feature.

Treating floats as integers may make sense, but due to the BC break we shouldn't do that for PHP 7.

In that case, I'll rebase against master. Thanks for the review 👍

@carusogabriel carusogabriel changed the base branch from PHP-7.3 to master March 28, 2020 14:11
@carusogabriel carusogabriel added Feature and removed Bug labels Mar 28, 2020
@carusogabriel carusogabriel requested a review from cmb69 March 28, 2020 14:12
@carusogabriel carusogabriel changed the title Fix #61877: Make SimpleXMLElement respect PHP's array rule #61877: Make SimpleXMLElement respect PHP's array rule Mar 28, 2020
cmb69
cmb69 reviewed Mar 28, 2020
View reviewed changes
Copy link
Member

@cmb69 cmb69 left a comment

Choose a reason for hiding this comment

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

The implementation looks fine, but I think this change should be discussed on the internals@ mailing list.

@cmb69
Copy link
Member

cmb69 commented May 6, 2021

What's the status here? Has there been any discussion on internals?

@carusogabriel carusogabriel deleted the bugfix/cast-issues-simplexml branch August 16, 2021 11:52
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@cmb69 cmb69 cmb69 left review comments

Assignees
No one assigned
Labels
Feature
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
2 participants
@carusogabriel @cmb69