Zend: add new zend_object_init{_with_constructor}() API by Girgias · Pull Request #24135 · php/php-src

Girgias · 2026-10-05T14:28:44Z

Commits should be reviewed individually.

The motivation is that creating an object shouldn't require providing a zval backing storage, as often one only cares about the zend_object*. The common case where we do have a backing storage is within userland functions where return_value is passed, this is IMHO suboptimal as it leaks an implementation detail that is usually hidden by the use of the RETURN/RETVAL macros, moreover it can make determining what the return value of userland function is confusing if return_value is initialized and a plain return statement is used to return early.

The commits follow the rough idea:

Minor preparatory refactorings
Introduction of new API
Partial migration to the new API

Obviously not all cases have been migrated, sometimes it makes more sense to use the existing object_init_ex API, or moving to the new API is desirable but various other refactorings need to be made at an extension level for it to make sense.

Some future refactoring I am thinking of doing with the new APIs are:

reflection
intl
SPL

TimWolla · 2026-10-12T08:40:38Z

 	zval_ptr_dtor(&errors_array);

-	zend_throw_exception_object(&ex);
+	zend_throw_exception_internal(ex);


Seeing this change, I wonder if the instanceof check should become an assertion in zend_throw_exception_internal.

I'm guessing you mean in zend_throw_exception_object()? And it cannot as it is used by the ZEND_THROW opcode.

No. I meant zend_throw_exception_internal(). Given that more users switched to zend_throw_exception_internal() directly, the instanceof check here in zend_throw_exception_object() could also become an assertion in zend_throw_exception_internal(), since the type system can only guarantee it's an object, but not that it's a throwable.

TimWolla · 2026-10-12T08:45:12Z

-			zval_ptr_dtor(arg);
-			ZVAL_UNDEF(arg);
-			return FAILURE;
+			zend_object_store_ctor_failed(obj);


Is this change fixing a pre-existing bug?

I don't think it does, as that branch is tested via the ext/zend_test/tests/zend_object_init_with_constructor.phpt and the destructor doesn't seem to be called already. So I'm slightly confused. But this whole logic will go away when I get back round to remove the annoying get_constructor object handler.

I agree this looks like a sus behaviour change

This should be testable with a non-final class that throws with a destructor. I don't think we have a class already that can show this difference

Isn't that case already covered by the ThrowingUser class in ext/zend_test/tests/zend_object_init_with_constructor.phpt?

No, you're looking for a class that throws in get_constructor without implicitly already calling zend_object_store_ctor_failed, and then subclass that with a destructor

TimWolla

Didn't check all extensions, I trust you with those. Some additional smaller nits, overall LGTM.

…nternal()

In preparation for a new object initializer API

By using the new zend_object_init_with_constructor() API.

Did not migrate object_init_ex() usage in create_driver_specific_pdo_object() as this needs some larger refactoring

TimWolla

Still couldn't be bothered to check all extensions. Changes LGTM now. Further comments are optional, either way no further review round necessary for them.

TimWolla · 2026-10-14T20:36:36Z

 #endif /* HAVE_DTRACE */

 	if (exception != NULL) {
+#ifdef ZEND_DEBUG


Explicit ZEND_DEBUG check should not be necessary for ZEND_ASSERT() to be elided including its calls (but perhaps check the assembly to verify).

That's false. That requires the compiler to know the function is pure, which it doesn't at this point. The same ifdef pattern is used in the VM for a similar check.

This should also be #if, not #ifdef, because the macro is always defined: it's just either 0 or 1

TimWolla · 2026-10-14T20:37:52Z


 	/* Extract the $message property. */
-	z = zend_read_property_ex(zend_ce_nodiscard, Z_OBJ_P(&obj), ZSTR_KNOWN(ZEND_STR_MESSAGE), false, NULL);
+	zval *z = zend_read_property_ex(zend_ce_nodiscard, obj, ZSTR_KNOWN(ZEND_STR_MESSAGE), false, NULL);


Could also declare that separately for consistency with Deprecated.

ndossche · 2026-10-14T20:56:27Z

-		add_property_resource(&object, "context", context->res);
+		zval tmp;
+		ZVAL_RES(&tmp, context->res);
+		zend_update_property(ce, object, ZEND_STRL("context"), &tmp);


This causes a refcount leak because add_property_resource dtors the input, so you need to remove the GC_ADDREF on line 262

Also this new code has a behaviour change: it will behave differently wrt private/protected properties due to the fake scope.

ndossche · 2026-10-14T20:58:23Z

-			zval_ptr_dtor(arg);
-			ZVAL_UNDEF(arg);
-			return FAILURE;
+			zend_object_store_ctor_failed(obj);


I agree this looks like a sus behaviour change

ndossche · 2026-10-14T21:02:44Z

-			zval_ptr_dtor(arg);
-			ZVAL_UNDEF(arg);
-			return FAILURE;
+			zend_object_store_ctor_failed(obj);


This should be testable with a non-final class that throws with a destructor. I don't think we have a class already that can show this difference

ndossche · 2026-10-14T21:04:45Z

 	ftp->use_ssl = false;
 #endif

-	object_init_ex(return_value, php_ftp_ce);


I disagree that the new code is better, the old code was more compact and return_value is open-coded plenty of times in the code base.

ndossche · 2026-10-14T21:05:40Z

-		object_init_ex(&zv, old_obj->ce);
-		GC_ADD_FLAGS(Z_OBJ(zv), IS_OBJ_DESTRUCTOR_CALLED);
-		return Z_OBJ(zv);
+		zend_object *obj = zend_object_init(old_obj->ce);


This on the other hand is a code change where the new coed looks better, same with the code above

github-actions Bot added Category: Engine Extension: ftp Extension: hash Extension: libxml Extension: mysqli Extension: pdo (core) Extension: phar Extension: reflection Extension: shmop Extension: sockets Extension: spl ABI break labels Oct 5, 2026

Girgias force-pushed the 2026-09-zend_object_init_api branch 2 times, most recently from 5df1993 to c5a38d1 Compare October 6, 2026 22:25

Girgias marked this pull request as ready for review October 11, 2026 21:58

Girgias requested review from DanielEScherzer, LamentXU123, SakiTakamachi, arnaud-lb, bukka, devnexen, dstogov and kamil-tekiela as code owners October 11, 2026 21:58

Girgias mentioned this pull request Oct 11, 2026

reflection: various minor refactorings (2026-10 n1) #24039

Open

Girgias requested a review from TimWolla October 11, 2026 22:00

TimWolla requested changes Oct 12, 2026

View reviewed changes

Girgias force-pushed the 2026-09-zend_object_init_api branch from c5a38d1 to ada1544 Compare October 13, 2026 13:59

Girgias requested a review from TimWolla October 13, 2026 17:30

TimWolla reviewed Oct 13, 2026

View reviewed changes

Comment thread Zend/zend_API.h Outdated

Comment thread Zend/zend_execute.c Outdated

Comment thread Zend/zend_execute.c Outdated

Comment thread Zend/zend_closures.c Outdated

Girgias force-pushed the 2026-09-zend_object_init_api branch from ada1544 to 425e7ae Compare October 14, 2026 18:00

Girgias requested a review from TimWolla October 14, 2026 18:00

Girgias added 17 commits October 14, 2026 21:30

Zend: assert that the object is a Throwable in zend_throw_exception_i…

93db860

…nternal()

Zend: refactor _object_and_properties_init() to return zend_object*

7238c45

In preparation for a new object initializer API

Zend: add const qualifier to object API

c51ba83

Zend: add new zend_object_init() API

04b5b01

Zend: migrate to new zend_object_init() API

62b1370

Zend: add zend_object_init_with_constructor() API

e776a13

Zend: refactor zend_get_attribute_object() to return zend_object*

452f637

By using the new zend_object_init_with_constructor() API.

ftp: migrate to new zend_object_init() API

6ad8d6b

hash: migrate to new zend_object_init() API

716e019

pdo: migrate to new zend_object_init() API

98bf16f

Did not migrate object_init_ex() usage in create_driver_specific_pdo_object() as this needs some larger refactoring

socket: migrate to new zend_object_init() API

7f4ef66

shmop: migrate to new zend_object_init() API

0e40afd

mysqli: migrate to new zend_object_init() API

d6f44c8

streams: migrate to new zend_object_init() API

02ead33

libxml: migrate to new zend_object_init() API

2bdea0f

phar: migrate to zend_object_init_with_constructor() API

afd805f

spl: migrate to zend_object_init_with_constructor() API

203213a

Girgias force-pushed the 2026-09-zend_object_init_api branch from 425e7ae to 203213a Compare October 14, 2026 20:34

TimWolla approved these changes Oct 14, 2026

View reviewed changes

ndossche requested changes Oct 14, 2026

View reviewed changes

ndossche reviewed Oct 14, 2026

View reviewed changes

Conversation

Girgias commented Oct 5, 2026

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

TimWolla left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

TimWolla left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants