[3.4] - Bug Fix - Fix global dump function return value for PHP7 #28491
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
99a2a98 to
46c5688
Compare
|
Nice catch. I'd suggest to name the new var just |
|
@nicolas-grekas You're absolutely correct. Silly oversight on my part. I'll update and retarget tonight. I'll change the var to |
|
Well, GitHub did not like what I did and closed this PR. I have created a new PR (#28497), targeted at 3.4, with the mentioned variable change. I apologize for the extra PR. 😒 |
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.
In 3.4, the global
dump()helper function in the VarDumper component was updated to return the arguments passed in to it. However, in 4.0, the way the variables are dumped was updated, and it introduced a bug in PHP7 in the return value of the function.With the update made in 4.0, 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