Use unset() instead of array_shift() to remove arguments from func_get_args()

array_shift is bad in performance

Excellent, thanks!

d'oh, sorry.

Can you enlighten us, too, please? :) Why did #2 fail?

@sun,

the failed tests are hardcode the args key. eg.

    ->condition('uid', $form_state['build_info']['args'][0]->uid)
    ->condition('aid', $form_state['build_info']['args'][1])

array_shift reorder the index key from ZERO and unset just remove an element from array only.

#7: drupal-1217840-7.patch queued for re-testing.

I'm OK with the changes in this patch that only effect subsequent code, which is doing stuff like implode().

In other words, I'd like to see the changes to form.inc reverted. (but fine with adding a comment as in #7)

There's lots of code that's doing that outside of core, and that's why I disagree with it. It's a perfectly fine way for accessing the arguments, and I do not see why we want to break such code. Contrary to that, the change to unset() is a pure micro-optimization only.

I've to admit that there's no difference between unset() + array_values(), but at the same time, array_shift() is just one amount of confusion.

Also, did we check the behavior of PHP references with array_values()?

Interesting how this impacts on performance ;)

Committed/pushed to 8.x, thanks!

Moving 7.x for backport.

Backported #25 to D7.

There were only a couple of things to backport. module_invoke_all() in D7 already uses unset() and the changes to Views aren't relevant.

+++ b/includes/bootstrap.inc
@@ -3425,7 +3425,8 @@ function &drupal_register_shutdown_function($callback = NULL) {
-    array_shift($args);
+    // Remove $callback from the arguments.
+    unset($args[0]);

+++ b/includes/form.inc
@@ -124,8 +124,8 @@ function drupal_get_form($form_id) {
-  array_shift($args);
-  $form_state['build_info']['args'] = $args;
+  unset($args[0]);
+  $form_state['build_info']['args'] = array_values($args);

why do we have to do array_values in the second case, and not in the first?

I went back and re-read some comments... maybe because

array_shift reorder the index key from ZERO and unset just remove an element from array only.

Does array_values reorder from zero then?

http://us2.php.net/manual/en/function.array-values.php
"array_values() returns all the values from the array and indexes the array numerically."

tim said in #16

there is no reason for an implementation of hook_form_alter() to blindly trust the array keys in $form_state['build_info']['args'], and we can document that and fix it.

and then added some array_values() in #18.

I would have thought we would have had to do unset and then array values all the time? ...
and why not do it right after the unset? like:
+ unset($args[0]);
+ // Put reason here...
+ $args=array_values($args);
$form_state['build_info']['args'] = $args;

OTOH, this is a backport... so maybe we should just do it the same as was done for drupal 8. (but I am curious)

EDIT: Sorry, pasted in the wrong text.

Just looks like most of @YesCT's comments have to do with D8. Do we want to re-open this or just accept the decisions made in D8. If we want to accept them, we should be able to just back-port them to D7, right?