ext/pcre: Refactor preg_replace_callback(_array)() to not pass a useless FCI

[Girgias]

And other drive-by refactorings.

This fixes an unclean shutdown with trampolines for preg_replace_callback_array(), I just don't know what causes it atm.

[nielsdos]

Most of this PR is fine (and does more welcome cleanup than just the FCI stuff).

What I'm just not entirely convinced by is if in this particular case getting rid of FCI passing is a good idea. If we don't pass it, the engine makes an FCI itself and also needs to do a bit extra work when dealing with trampolines:

php-src/Zend/zend_API.h

Lines 847 to 851 in 47f1cae

	if (UNEXPECTED(func->common.fn_flags & ZEND_ACC_CALL_VIA_TRAMPOLINE)) {
	func = (zend_function*) emalloc(sizeof(zend_function));
	memcpy(func, fcc->function_handler, sizeof(zend_function));
	zend_string_addref(func->op_array.function_name);
	}

But we already have an FCI anyway (that we don't store), so I'm not sure how much improvement this actually is.

[nielsdos]

I'm not entirely sure this is right. This error path calls into pcre_handle_exec_error(count) where count represents an error code normally. However, in this call this is a real count and not an error code.
I was first thinking that you should probably set PCRE_G(error_code) yourself. However, perhaps not even that is necessary because we're throwing a userland exception anyway (which is not a pcre error itself).

[Girgias]

Right, I wasn't exactly sure myself here.

[Girgias]

Most of this PR is fine (and does more welcome cleanup than just the FCI stuff).

What I'm just not entirely convinced by is if in this particular case getting rid of FCI passing is a good idea. If we don't pass it, the engine makes an FCI itself and also needs to do a bit extra work when dealing with trampolines:

php-src/Zend/zend_API.h

Lines 847 to 851 in 47f1cae

	if (UNEXPECTED(func->common.fn_flags & ZEND_ACC_CALL_VIA_TRAMPOLINE)) {
	func = (zend_function*) emalloc(sizeof(zend_function));
	memcpy(func, fcc->function_handler, sizeof(zend_function));
	zend_string_addref(func->op_array.function_name);
	}

But we already have an FCI anyway (that we don't store), so I'm not sure how much improvement this actually is.

Long term, I am trying to see if I can get rid of the function_name zval in the FCI struct and only rely on the FCC for the actual function, trampolines are slightly annoying in this regard. I don't know enough about hardware to know if reducing argument count is helpful or not, but most of the FCI initialization is done just prior to calling it anyway here.