Fix bad Pure attributes
#1730
Open
Fix bad Pure attributes
#1730
Conversation
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
These functions all trigger autoloading which may have the effect of definining one or more new symbols. Note this does not apply to `function_exists()` as there is no function autoloading.
Neither of these functions depend on any external state.
Both functions have the side effect of ending the current output buffer. Whilst the latter also writes to the output stream.
As this function takes a callable it should not be marked as pure, this appears to be the convention based on all the other array user intersect/diff functions.
isfedorov
reviewed
Apr 11, 2025
| @@ -609,7 +605,7 @@ function get_class_vars(string $class): array {} | |||
| * for the specified <i>object</i> in scope. If a property have | |||
| * not been assigned a value, it will be returned with a null value. | |||
| */ | |||
| #[Pure(true)] | |||
There was a problem hiding this comment.
It looks like this change is also incorrect since function actually depends on global state. The function depends on the structure of the class from which the object was instantiated. If the class definition changes (e.g., due to autoloading), the result can change. Also if string property names or values are affected by locale settings (e.g., setlocale()), it might affect the output. Please correct me if I'm wrong
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
This PR is based on my interpretation of the documentation on the attribute that
Pure(true)means a function:This contradicts with the interpretation in #1724 by @zonuexe which was accepted:
The reasoning for each change is documented in the commit messages.
As such it reverts some of the changes made in that PR. If my interpretation is wrong then reject this, but please improve the documentation.