Remove node_invoke(), comment_invoke(), etc

In fact we can simply make module_invoke() and module_invoke_all() support reference, and kill all those three beasts (node_invoke(), node_invoke_nodeapi(), comment_invoke_comment()).

Plus, it has the great side-benefit of removing the slow call to call_user_func_array(). Here is a patch.

PHP is trying to deprecate call-time references, for some reason... but Ubuntu sets allow_call_time_pass_reference = On by default. Back to the drawing board.

found another one...

/**
 * Invokes hook_user() in every module.
 *
 * We cannot use module_invoke() for this, because the arguments need to
 * be passed by reference.
 */
function user_module_invoke($type, &$array, &$user, $category = NULL) {

The reason I came across this is that I wanted to implement a devel type module for hooks - the devel module showing which themes are called is great.

But It's hard to do something similar for hooks as there isn't a clear hook controller.

@Damien: just run into this issue but think you're patch is almost on track. Shouldn't it be enough to remove the ampersands from the function calls? (sorry, can't test myself right now because my D7 is hosed...)

Morosely moving this to Drupal 8.

Not a patch... but I'm running a different function that does just this on a production system.

Call like so:

module_invoke_all_array('form_alter', Array('myform', &$form));

It works because PHP's deprecation of call-time pass-by-reference doesn't apply to /setting/ references into an /array/, which is what we'd be doing.

// Takes $args as array so we can use pass-by-reference...
function module_invoke_all_array ($hook, $args) {
  $return = Array();

  // We'll use eval(), so we need to turn $args into as a string.
  $params = '';

  $keys = array_keys($args);
  foreach ($keys as $key) {
    $params .= '$args['.$key.'],';
  }
  $params = substr($params, 0, -1);

  // Got params, now invoke everything.
  foreach (module_implements($hook) as $module) {
    $func = $module.'_'.$hook;
    $call = $func.'('.$params.');';
    eval('$result='.$call);

    if (isset($result) && is_array($result)) {
      $return = array_merge_recursive($return, $result);
    }
    else if (isset($result)) {
      $return[] = $result;
    }
  }

  return $return;
}

Comment #6 as a patch. Note these are all new functions; there's not reason to break API for this issue.

#679404: Warning: Parameter 1 to profile_load_profile() expected to be a reference, value given in module_invoke() could be a duplicate, not sure...

You can fix this by swapping func_get_args for a call to debug_backtrace (credit where it's due - mitko on http://www.php.net/manual/en/function.func-get-args.php#90095 )

func_get_args() loses the references, debug_backtrace() doesn't.

Swap:

$args = func_get_args();

For:

  $stack = debug_backtrace();
  $args = array();
  if(isset($stack[0]["args"])){
    for($i=0; $i < count($stack[0]["args"]); $i++){
      $args[$i] = &$stack[0]["args"][$i];
    }
  }  

Not sure what performance impact calling debug_backtrace() has though.

+1

Better title, proper component.

This is my solution for the problem of php5.3 warnings in the openlayers_geocoder module for the 6.20 version of Drupal core.

+ * Add code to get rid of PHP 5.3 warning
+ * This is a temporary solution until the problem can be resolved in core.
+ * Note that while this code allows arguments to be passed by reference,
+ * passing arguments by reference in call_user_func_array() is deprecated in PHP 5.3
+ */
+
+function ol_module_invoke($module, $hook, $args = array()) {
+ $function = $module . '_' . $hook;
+ if (module_hook($module, $hook)) {
+ return call_user_func_array($function, $args);
+ }
+}

These changes to module_invoke() and module_invoke_all() seem to be working for me (Drupal 6) so far:

function module_invoke() {
  $args = func_get_args();
  $module = $args[0];
  $hook = $args[1];
  unset($args[0], $args[1]);
  $function = $module . '_' . $hook;
  if (module_hook($module, $hook)) {
    if (strnatcmp(phpversion(),'5.2') <= 0)
      return call_user_func_array($function, $args);
    else {
      $ref_args = array();
      foreach ($args as $key => &$arg) {
        $ref_args[$key] = &$arg;
      }
      return call_user_func_array($function, $ref_args);
    }
  }
}

function module_invoke_all() {
  $args = func_get_args();
  $hook = $args[0];
  unset($args[0]);
  $return = array();
  $lOldPHP = (strnatcmp(phpversion(),'5.2') <= 0);
  foreach (module_implements($hook) as $module) {
    $function = $module .'_'. $hook;
    if ($lOldPHP)
      $result = call_user_func_array($function, $args);
    else {
      $ref_args = array();
      foreach ($args as $key => &$arg) {
        $ref_args[$key] = &$arg;
      }
      $result = call_user_func_array($function, $ref_args);
    }
    if (isset($result) && is_array($result)) {
      $return = array_merge_recursive($return, $result);
    }
    else if (isset($result)) {
      $return[] = $result;
    }
  }

  return $return;
}

I'm still something of a newbie, though, so if somebody sees a mistake here, please let me know!

Related to improvements to module_invoke_all(): #890660: add an alternative way of doing module_invoke_all() which sets a 'module' key in the returned info.

@Dave Goldman

I'm a rank amateur myself. My question is, why force all arguments to be passed by reference? If the ability to pass by reference at call time is removed in PHP 5.4, then it will be back to the drawing board. Pass by reference at call time is already deprecated in PHP 5.3., so chances are that it's not a long-term solution. It seems like an awful lot of processing.

I almost missed this. You're still using func_get_args (), which returns a copy of the argument and not a reference. This is the whole reason why you can't use module_invoke() to pass arguments by reference in the first place. Correct me if I'm wrong, but I don't see how this can maintain references that are passed into module-invoke().

@ccardea: Everything you're saying makes sense to me.

On the other hand, the code changes I made did solve the problems I was seeing on my PHP 5.3 machine.

I'll look into this a bit more and get back to you...

@ccardea:

(1) My question is, why force all arguments to be passed by reference? If the ability to pass by reference at call time is removed in PHP 5.4, then it will be back to the drawing board. Pass by reference at call time is already deprecated in PHP 5.3., so chances are that it's not a long-term solution.

Is my code doing a deprecated "call-time pass by reference", or is it passing an array by value, where each of that array's elements are references? Is the latter deprecated? (Based on Comment #6 by dickson.michael, I'm thinking maybe it's okay.)

(2) It seems like an awful lot of processing.

Probably true. But for a quick fix, anyhow, that's a reasonable trade-off to achieve a correct result. :)

(3) You're still using func_get_args (), which returns a copy of the argument and not a reference. This is the whole reason why you can't use module_invoke() to pass arguments by reference in the first place. Correct me if I'm wrong, but I don't see how this can maintain references that are passed into module-invoke().

Argh!! Quite right.

It still helps for module_invoke_all(), though, as at least all of the functions that it invokes are passed the same arguments by reference, even though the original caller's arguments won't be updated. This must be why these changes solved the Drupal problem I was originally having.

Assuming that we want Drupal 6 to work with PHP 5.3, where does this leave us? Apart from the above suggestion, by fmccormick, of using debug_backtrace(), must we do something like the following?

function module_invoke($module, $hook, &$arg0, &$arg1, &$arg2, &$arg3, &$arg4) {
  $function = $module . '_' . $hook;
  if (module_hook($module, $hook)) {
    if ($arg4 !== NULL)
      return $function($arg0, $arg1, $arg2, $arg3, $arg4);
    else if ($arg3 !== NULL)
      return $function($arg0, $arg1, $arg2, $arg3);
    else if ($arg2 !== NULL)
      return $function($arg0, $arg1, $arg2);
    else if ($arg1 !== NULL)
      return $function($arg0, $arg1);
    else if ($arg0 !== NULL)
      return $function($arg0);
    else
      return $function();
    }
  }
}

Using referenced variables in the array that is passed to call_user_func_array() is deprecated, but fortunately does not emit any warning messages. The PHP manual states that the ability to do this will most likely be removed in the next version of PHP.

In my solution, I set up the array outside of the call to module_invoke and just loaded the array with the function parameters before calling my module_invoke function. This allows you to place both referenced and non-referenced variables in the array, and avoids the processing inside the module_invoke() function. This works for now, but I'm really only focused on my one small problem. The thing is, if the ability to pass references at call time is eliminated in PHP 5.4, the module_invoke functions can stay the way they are now, but all the modules that use references in their function signatures will have to change the way they do business. They'll probably have to return something instead, maybe an object.

I think we should look at using the same approach that was taken with drupal_alter() in D7 - #593522: Upgrade drupal_alter(). Restrict the API to a limited number of arguments so we have something predictable to deal with. I need to re-read this issue, but have a feeling there are other duplicate issues around trying to do the same thing.

When PHP says that call-time-pass-by-reference is deprecated, I think all they mean is that you don't need the & in the function call. Using &$foo in the function signature is enough to cause the variable to be passed by reference. I can't see them eliminating something as fundamental as the ability to pass a reference to a function. The issue for module_invoke is simply how to replace the call to func_get_args.

+1 with @catch, let's cut it down to 3 arguments in total; if you need to pass more info, then convert the last into $context. I hope we gain some performance out of that.

Subscribe, I hate the way we have to work around this for the fullcalendar module :)

can i use module_invoke for og_user function

can i use module_invoke for og_user function

Leave the version number!

There is still a way to handle this in a generic (regarding the number of arguments) way:

Here is a simple test-script that works and is - as far as I have understood PHP's documentation - not deprecated:

function invoke($function_name, $args) {
	call_user_func_array($function_name, $args);
}

function hook(&$a, $b) {
	$a += 1;
	$b += 1;
}

$a = 0;
$b = 0;

invoke('hook', array(&$a, $b));
// $a = 1; $b = 0;

The deprecation of call-time pass-by-reference only concerns cases where the hook-function does not expect a reference.

The bad news is. func_get_args does not work, so we have to change invoke() to expect an array of values instead of a generic number of values.
Applied to module_invoke_all() this would look like this:

function module_invoke_all($hook, $args) {
	$return = array();
	
	foreach (module_implements($hook) as $module) {
		$function = $module . '_' . $hook;
		if (function_exists($function)) {
			$result = call_user_func_array($function, $args);
			if (isset($result) && is_array($result)) {
				$return = array_merge_recursive($return, $result);
			}
			elseif (isset($result)) {
				$return[] = $result;
			}
		}
	}

	return $return;
}

Note that I let the argument $args be passed as a copy so things like module_invoke_all('somehook', array($my, &$parameters)); will still work.

The interface change only concerns the calling side of the hooks. So it would be perfectly fine to add module_invoke_all2($hook, $args) to drupals interface and keep backwards compatibility.

#7: 353494-add-pass-by-reference-to-module-invoke.patch queued for re-testing.

My offering here, aimed at minimising processing and maximising compatibility across PHP versions, by avoiding use of call_user_func_array().

/**
 * Replacement for standard Drupal module_invoke to work-around problems with PHP 5.3
 * Instead of using call_user_func_array we call the module function directly.
 * This should avoid problems when functions need parameters to be passed be reference.
 * This should also be backward compatible with earlier PHP versions.
 * The main issue here is a limitation on the number of parameters. In practice, not likely to be a problem.
 */
function module_invoke() {
  $a = func_get_args();
  if (is_callable($func = sprintf('%s_%s', $a[0], $a[1]))) {
    if (count($a) > 10) {
      drupal_set_message(__FUNCTION__ . '() supports a maximum of 10 parameters', 'error');
    }
    return  $func($a[2], $a[3], $a[4], $a[5], $a[6], $a[7], $a[8], $a[9]);
  }
}

So, this handles a maximum of 8 parameters (not counting the first two which give module and hook name.) Obviously this number could be increased, but surely 8 are enough!?!? Ok, another potential problem is that a later PHP version might error when too many parameters are passed to a function.

Need to change $a = func_get_args(); to the following, or you'll get uninitialized index errors:

$a = array(NULL, NULL, NULL, NULL, NULL, NULL, NULL, NULL, NULL, NULL);
$a += func_get_args();

I wondered about something like that, but didn't get any errors under test (PHP 5.3.2-1ubuntu4.9). I'm pretty sure I've got PHP set up to report all errors, but I'll check.

So would this work? (and perhaps looks a bit more elegant?) ...

$a = array_fill(0, 10, NULL) + func_get_args();

This is actually a duplicate of #329012: Remove call_user_func_array() from ModuleHandler::invoke() + invokeAll(), but since it additionally targets those wonky _invoke() APIs, I'm only postponing it.

Over there, the (I think agreed on) strategy is to remove support for unlimited amount of arguments and make module_invoke[_all]() similar to drupal_alter().

If we are going to kill backward-compatibility. Why don't we do it the right way?

function module_invoke($function, $args) {
    return $function($args);
}

… with $args being an array of arguments. This way every hook has exactly 1 argument. This is IMHO the most effective way to deal with this issue within PHP's limitations.

This solution would mean:

• A huge performance boost.
• Still be able to handle an arbitrary number of arguments (enclosed in an array).
• Making things consistent with theme hooks.
• Breaking backwards-compatibility really hard.

Limiting the number of arguments without standardising them to a fixed number always has the bitter taste of being a workaround. Having a standardized/fixed number of arguments looks much more like design within PHP's limitations.

PS: It follows the KISS principle.

+1 for torotil #36 or #29
This seems to be the best way to handle this, good work torotil! Saved me from typing the same thing:)

This will need reroll after #111715: Convert node/content types into configuration

No more node_invoke() in core #2018375: Get rid of node_hook and node_invoke let's continue with comments

comment_invoke() is also bye bye.