Skip to content
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.

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

[7.2] Revert BC break caused by fixing bug #74035 #2893

Closed
wants to merge 1 commit into from

Conversation

Majkl578
Copy link
Contributor

@Majkl578 Majkl578 commented Nov 2, 2017

Reverts BC break in PHP 7.2 introduced in PR #2363, reverts commit 9ffc6ca
Fixes bug #74292
Relates to #74035 (which PR #2363 aimed to fix)
Unbreaks doctrine/common#822
This was already reverted in 7.0 and 7.1 back in february in eb1373e by @nikic, but kept in master as a valid fix (for 8.0).

NEWS don't seem to contain any mention thus no changes there.

This intentionally targets PHP-7.2 branch where BC break is introduced. It is fine to be kept in master (and though repeat this ceremony for 7.3 😆).

@Majkl578 Majkl578 changed the title Revert BC break caused by fixing bug #74035 [7.2] Revert BC break caused by fixing bug #74035 Nov 2, 2017
@krakjoe krakjoe added the Bug label Nov 3, 2017
ZEND_BEGIN_ARG_INFO_EX(arginfo_reflection_class_newInstance, 0, 0, 0)
ZEND_ARG_VARIADIC_INFO(0, args)
ZEND_BEGIN_ARG_INFO(arginfo_reflection_class_newInstance, 0)
ZEND_ARG_INFO(0, args)
Copy link
Contributor

Choose a reason for hiding this comment

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

This doesn't seem to be correct. On line 4759 below, we accept the args to newInstance as a variadic set, not as a single $args array.

@sgolemon
Copy link
Contributor

sgolemon commented Nov 6, 2017

After discussion with @nikic I'm merging this for inclusion in RC6.
I'm also merging it to master because I don't want this same discussion repeating for 7.3.
We either come up with a correct fix, or we don't.

@php-pulls
Copy link

Comment on behalf of pollita at php.net:

Merged.

@php-pulls php-pulls closed this Nov 6, 2017
@Majkl578 Majkl578 deleted the revert-74035-fix branch December 5, 2017 03:52
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants