[VarDumper] Fix global dump function return value for PHP7 #28497
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
nicolas-grekas
approved these changes
Sep 18, 2018
fdbeb64 to
0def211
Compare
|
Thank you @patrickcarlohickman. |
nicolas-grekas
added a commit
that referenced
this pull request
Sep 18, 2018
… (patrickcarlohickman) This PR was squashed before being merged into the 3.4 branch (closes #28497). Discussion ---------- [VarDumper] Fix global dump function return value for PHP7 Retarget of PR #28491. Reposting description below, with relevant updates. | Q | A | ------------- | --- | Branch? | 3.4 | Bug fix? | yes | New feature? | no | BC breaks? | no | Deprecations? | no | Tests pass? | yes | Fixed tickets | | License | MIT | Doc PR | In 3.4, the global `dump()` helper function in the VarDumper component was updated to return the arguments passed in to it. However, due to reusing the argument variable in the function, this introduces a bug in PHP7 in the return value of the function. The variable used in the `foreach` loop overwrites the value passed in by the first argument. In PHP5, this is okay. In PHP7, even though the argument is passed by value, the value returned by `func_get_args()` is affected by changes to the arguments inside the function. This is a change from PHP5. From the documentation for [`func_get_args()`](http://php.net/manual/en/function.func-get-args.php): > If the arguments are passed by reference, any changes to the arguments will be reflected in the values returned by this function. As of PHP 7 the current values will also be returned if the arguments are passed by value. This PR simply changes the name of the variable used in the `foreach` loop. It also adds a test file to test the return value of the global `dump()` function. This is my first contribution to Symfony, so please let me know if the issue should be resolved in a different manner, or if the test should be modified in any way. Thanks, Patrick Commits ------- 0def211 [VarDumper] Fix global dump function return value for PHP7
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.
Retarget of PR #28491. Reposting description below, with relevant updates.
In 3.4, the global
dump()helper function in the VarDumper component was updated to return the arguments passed in to it. However, due to reusing the argument variable in the function, this introduces a bug in PHP7 in the return value of the function.The variable used in the
foreachloop overwrites the value passed in by the first argument. In PHP5, this is okay. In PHP7, even though the argument is passed by value, the value returned byfunc_get_args()is affected by changes to the arguments inside the function. This is a change from PHP5.From the documentation for
func_get_args():This PR simply changes the name of the variable used in the
foreachloop. It also adds a test file to test the return value of the globaldump()function.This is my first contribution to Symfony, so please let me know if the issue should be resolved in a different manner, or if the test should be modified in any way.
Thanks,
Patrick