WordPress.org

Make WordPress Core

Opened 2 years ago

Closed 2 years ago

Last modified 2 years ago

#20163 closed defect (bug) (fixed)

Call-time pass-by-reference has been removed in PHP (Fatal error in 5.4) or deprecated (Warning in 5.3)

Reported by: emiluzelac Owned by: koopersmith
Milestone: 3.4 Priority: normal
Severity: normal Version: 3.4
Component: Warnings/Notices Keywords: has-patch
Focuses: Cc:

Description

After upgrading to 3.4-alpha-20095 this message shows up at the top behind the admin bar:

Warning: Call-time pass-by-reference has been deprecated in /wp-includes/class-wp-customize-setting.php on line 451

WP_DEBUG is on and from plugins I have just standard Theme Review stuff in.

Attachments (3)

Screen Shot 2012-03-04 at 9.50.48 PM.png (17.4 KB) - added by emiluzelac 2 years ago.
20163.patch (49.1 KB) - added by kurtpayne 2 years ago.
Removing call time pass by reference
class-wp-customize-setting_pass-by-reference.patch (135 bytes) - added by HerbMillerJW 2 years ago.
Corrects the one appearance of calling a function by reference in class-wp-customize-setting.php

Download all attachments as: .zip

Change History (23)

comment:1 SergeyBiryukov2 years ago

  • Milestone changed from Awaiting Review to 3.4

comment:2 ocean902 years ago

  • Owner set to koopersmith
  • Status changed from new to reviewing

comment:3 knutsp2 years ago

  • Cc knut@… added
  • Severity changed from normal to major

I get this as a fatal error in PHP 5.4 on Windows: "Call-time pass-by-reference has been removed in ...". All theme related pages are blank, except Menues and Widgets.

comment:4 follow-up: emiluzelac2 years ago

(for some other ticket) it would be nice to append warning/error messages like this within UI instead of <body> to avoid UI breakage. It would look better as well.

comment:5 follow-up: knutsp2 years ago

  • Component changed from Warnings/Notices to Bundled Theme
  • Severity changed from major to critical
  • Summary changed from Warning: Call-time pass-by-reference has been deprecated to Call-time pass-by-reference has been removed in PHP (Fatal error in 5.4) or deprecated (Warning in 5.3)

As it seems Twenty Twelve will not be ready for 3.4, the version property may be set to "Future Release", and fixing it it is not so urgent, even probably very simple to do. But as PHP 5.4 is already released, and will be a more common platform during 2012, this bug is now either "critical" or "blocker", for this future component (Bundled Theme), at least.

I look forward to start testing Twenty Twelve and give constructive feedback!

comment:6 ocean902 years ago

  • Component changed from Bundled Theme to Warnings/Notices
  • Keywords needs-patch added
  • Severity changed from critical to normal

The Customizer has nothing to do with Twenty Twelve.

comment:7 follow-up: knutsp2 years ago

Thanks, ocean90. I was under the impression that the theme itself caused this, because I installed and activated it along with the latest build. I also had trouble reverting to Twenty Eleven, but I just restored everything from backup (test site).

But this is now not just about warnings, but fatal errors in latest core. I now can't change theme or edit theme options in 3.4-alpha-20111. If this is not critical, I don't know what's not.

comment:8 in reply to: ↑ 7 emiluzelac2 years ago

Yes @ocean90 stands correct this is not Theme related and in my report there's no fatal errors. It's warning, not fatal error.

Replying to knutsp:

Thanks, ocean90. I was under the impression that the theme itself caused this, because I installed and activated it along with the latest build. I also had trouble reverting to Twenty Eleven, but I just restored everything from backup (test site).

But this is now not just about warnings, but fatal errors in latest core. I now can't change theme or edit theme options in 3.4-alpha-20111. If this is not critical, I don't know what's not.

comment:9 knutsp2 years ago

It is a fatal error when running on PHP 5.4, released March 1, 2012. Removed legacy features: allow_call_time_pass_reference

comment:10 in reply to: ↑ 5 ocean902 years ago

Replying to knutsp:

and fixing it it is not so urgent, even probably very simple to do.

Why not providing a patch?

kurtpayne2 years ago

Removing call time pass by reference

comment:11 follow-up: kurtpayne2 years ago

  • Cc kpayne@… added
  • Keywords has-patch added; needs-patch removed

20163.patch Should be a very good start on removing call time pass by reference. I've given the admin and front end a light testing (hitting most pages and functions) and ensured that there are no php syntax errors in any php files. This was a mostly scan, verified with some automated grepping.

If you look on your own, be careful of array(&$var); as that's perfectly valid. Changing a piece of code like this will break things.

do_action_ref_array( 'wp_default_styles', array(&$this) );
Version 0, edited 2 years ago by kurtpayne (next)

comment:12 ocean902 years ago

kurtpayne, your patch belongs more too #16767.

HerbMillerJW2 years ago

Corrects the one appearance of calling a function by reference in class-wp-customize-setting.php

comment:13 HerbMillerJW2 years ago

My apologies for the duplicate. Didn't realize kurtpayne's already did this.

comment:14 follow-up: duck_2 years ago

In [20136]:

Remove call-time pass by reference as it causes a compile time fatal error in PHP 5.4. See #20163.

comment:15 in reply to: ↑ 14 duck_2 years ago

Replying to duck_:

In [20136]:

Remove call-time pass by reference as it causes a compile time fatal error in PHP 5.4. See #20163.

Left open in case we want to add the pass by reference to the method declaration instead.

The other usage of multidimensional() wasn't trying to pass a reference so I left it for now.

comment:16 in reply to: ↑ 11 duck_2 years ago

Replying to kurtpayne:

20163.patch Should be a very good start on removing call time pass by reference.

Those aren't cases of pass by reference -- you seem to mention this in the second half of the comment. See #16767 as ocean90 pointed out.

comment:17 kurtpayne2 years ago

duck_ and ocean90, thanks for the code review. I took a second look based on your feedback and you're right, 20163.patch was too aggressive and doesn't belong in this ticket. I don't think any further patches are necessary for this specific problem.

For posterity's sake, I'm documenting my findings below. I hope it helps somebody!

<?php
function test($s) {echo $s;}
function test2($x) {test(array_pop($x));}
$x = "Hello World\n";
test($x);                                 // Works
test2(array(&$x));                        // Works
call_user_func('test', $x);               // Works
call_user_func_array('test', array($x));  // Works
call_user_func_array('test', array(&$x)); // Works
test(&$x); // <---------- Deprecated, doesn't work
call_user_func('test', &$x); // <------ Deprecated, see note below

This note is on the PHP documentation page for call_user_func_array

Referenced variables in param_arr are passed to the function by reference, regardless of whether the function expects the respective parameter to be passed by reference. This form of call-time pass by reference does not emit a deprecation notice, but it is nonetheless deprecated, and will most likely be removed in the next version of PHP ...

This note is on the PHP documentation page for call_user_func

Note that the parameters for call_user_func() are not passed by reference.

I scanned trunk with the following command (if someone wants to replicate my work). I went through the results and did not find any code that required changes.

find . -type f -name "*.php" -exec egrep -Hri '[&][$]' {} \; | egrep -v "function\s*[&]?[a-zA-Z0-9_]+\s*\(" | egrep -v "[*]\s*@(param|return)"| egrep -v "=[>]?\s*[&][$]" | grep -v "\s*as\s*[&][$]"

+1 for fixed.

comment:18 ryan2 years ago

  • Resolution set to fixed
  • Status changed from reviewing to closed

comment:19 in reply to: ↑ 4 ; follow-up: SergeyBiryukov2 years ago

Replying to emiluzelac:

(for some other ticket) it would be nice to append warning/error messages like this within UI instead of <body> to avoid UI breakage. It would look better as well.

The consensus in #19196 was to keep them out of UI for now.

comment:20 in reply to: ↑ 19 emiluzelac2 years ago

Replying to SergeyBiryukov:

Replying to emiluzelac:

(for some other ticket) it would be nice to append warning/error messages like this within UI instead of <body> to avoid UI breakage. It would look better as well.

The consensus in #19196 was to keep them out of UI for now.

Thanks for additional info :)

Note: See TracTickets for help on using tickets.