[E_STRICT] menu.php at line 218

[arena]

PHP [E_STRICT] 2048 : Only variables should be passed by reference in ........wordpress\wp-admin\includes\menu.php at line 218

[nacin]

[18110] should probably be reverted.

Also, [18995].

[ryan]

In [18997]:

Avoid pass by ref warning. Props arena. see #18975

[ryan]

In [18998]:

Avoid 'Only variables should be passed by reference' warnings. Reverts [18110]. see #18975

[arena]

I re open this ticket because

• the code such as in post.php

$dir = array_shift($keys = array_keys($dirs));

still gives a E_STRICT PHP error
PHP [E_STRICT] 2048 : Only variables should be passed by reference in D:\webserver\htdocs\wordpress\wp-admin\includes\menu.php at line 218

• same for following code

$last_menu_key = array_pop( $last_menu_key = array_keys( $menu ) );

PHP [E_STRICT] 2048 : Only variables should be passed by reference in D:\webserver\htdocs\wordpress\wp-includes\post.php at line 4036

[arena]

another one :

[E_STRICT] 2048 : Only variables should be passed by reference in wordpress\wp-includes\general-template.php at line 1624

[SergeyBiryukov]

Apparently this doesn't work as expected:

$last_menu_key = array_pop( $last_menu_key = array_keys( $menu ) );

Here's what I see on Posts screen in 3.3-beta2-19067 on PHP 5.3.3 with E_ALL | E_STRICT:

Strict Standards: Only variables should be passed by reference in wp-includes/pomo/mo.php on line 198
Strict Standards: Only variables should be passed by reference in wp-includes/pomo/mo.php on line 198
Strict Standards: Declaration of Walker_Category_Checklist::start_lvl() should be compatible with that of Walker::start_lvl() in wp-admin/includes/template.php on line 52
Strict Standards: Declaration of Walker_Category_Checklist::end_lvl() should be compatible with that of Walker::end_lvl() in wp-admin/includes/template.php on line 52
Strict Standards: Declaration of Walker_Category_Checklist::start_el() should be compatible with that of Walker::start_el() in wp-admin/includes/template.php on line 52
Strict Standards: Declaration of Walker_Category_Checklist::end_el() should be compatible with that of Walker::end_el() in wp-admin/includes/template.php on line 52
Strict Standards: Only variables should be passed by reference in wp-admin/includes/menu.php on line 218
Strict Standards: Non-static method WP_MatchesMapRegex::apply() should not be called statically, assuming $this from incompatible context in wp-includes/class-wp.php on line 222

18975.2.patch​ fixes menu.php and mo.php. Doesn't touch the rest for now.

[ryan]

In [19072]:

Avoid 'Only variables should be passed by reference' warnings. Props SergeyBiryukov. see #18975

[ryan]

That fixes all strict warning I see except for:

PHP Strict Standards:  Redefining already defined constructor for class WP_Widget in /.../trunk/wp-includes/widgets.php on line 93

We'll have to keep that PHP4 constructor around for a time.

[SergeyBiryukov]

Perhaps [18998] should also be altered (see 18975.3.patch​).

[ryan]

In [19074]:

Avoid 'Only variables should be passed by reference' warnings. Props SergeyBiryukov. see #18975

[scribu]

It's more efficient to use reset() and end() instead of array_shift() and array_pop(), since we don't use $var_by_ref afterwards.

[scribu]

And there's also key(), if needed.

[scribu]

I know it's not exactly related to this ticket, but as long as we're here...

[scribu]

Also, this line is rather silly:

array_map('wp_setup_nav_menu_item', array( get_post( $var_by_ref ) ) )

It can just be:

array( wp_setup_nav_menu_item( get_post( $var_by_ref ) ) )

[scribu]

Or, we can just use list( ) = and avoid $var_by_ref altogether. Patch incoming.

[nacin]

IIRC, reset() and end() don't work as you might expect, depending on the order items were added to the array. ryan seems to recall the same thing but we could both be off. end(), key(), and reset() all require references like array_shift() and array_pop(), so they don't eliminate the requirement for the variable. I'd love to see a patch that changes our understanding here.

[scribu]

reset() and end() don't work as you might expect, depending on the order items were added to the array.

Yes, in the sense that order matters, even for associative arrays:

array( 'a' => 1, 'b' => 2 ) vs. array( 'b' => 2, 'a' => 1 )

but array_shift() and array_pop() follow the same rule.

end(), key(), and reset() all require references like array_shift() and array_pop(), so they don't eliminate the requirement for the variable.

They don't eliminate the requirement for the variable, but they avoid all the work that array_shift() and array_pop() do, which can be significant, especially in the case of wp-db.php.

[scribu]

Actually, I might be exaggerating how much extra work array_pop() and array_shift() do. Let me run some tests.

[scribu]

Yep, the difference in performance between array_shift() and array_pop() vs. reset() and end() is unnoticeable, even for arrays with thousands of items.

So 18975.4.patch​ is just a single conversion to key(), since I think it makes the code easier to understand.

[ryan]

In [19094]:

Avoid E_STRICT notices. see #18975

[ryan]

That's 18975.diff sans walkers. Those can wait.

[ryan]

In [19095]:

Use key(). Props scribu. fixes #18975

[hakre]

Replying to ryan:

That fixes all strict warning I see except for:

PHP Strict Standards:  Redefining already defined constructor for class WP_Widget in /.../trunk/wp-includes/widgets.php on line 93

We'll have to keep that PHP4 constructor around for a time.

The strict error can be cured while keeping the PHP 4 constructor: #20801 (and even giving deprecated notices)

[jdgrimes]

Just found another one of these notices today:

PHP Strict Standards:  Only variables should be passed by reference in /wp-admin/widgets.php on line 185

It only shows itself when using widget accessibility mode. Attaching a patch.

[SergeyBiryukov]

Replying to jdgrimes:

Just found another one of these notices today

This ticket was closed on a completed milestone. Moved to #25225.