Add macros to simplify handling of internal structs by alcaeus · Pull Request #1965 · mongodb/mongo-php-driver

alcaeus · 2026-03-27T15:15:50Z

Following the changes made in #1959, this PR introduces additional macros to handle declarations of the intern variables used throughout and working with $this and object initialisation.

@paulinevos and @GromNaN, I'll let you review and decide whether you find the code more legible with the macros or if you prefer the current style.

Copilot

Pull request overview

This PR introduces a small set of new helper macros to standardize how internal intern structs are fetched/initialized, and refactors many classes to use those macros for $this handling and object initialization.

Changes:

Added PHONGO_INTERN_FROM_*, PHONGO_INTERN_OBJECT_ALLOC, and PHONGO_INTERN_INIT_EX macros to centralize internal struct access/allocation.
Refactored many BSON/Driver/Monitoring classes to use the new macros instead of per-type Z_*_OBJ_P helpers and manual object_init_ex + fetch patterns.
Simplified several object creation/debug/free paths by replacing repeated boilerplate allocations/fetches.

Reviewed changes

Copilot reviewed 50 out of 50 changed files in this pull request and generated 1 comment.

Show a summary per file

File	Description
src/phongo_classes.h	Adds macros for fetching/allocating/init’ing `intern` structs.
src/phongo_bson_encode.c	Replaces direct `Z_*_OBJ_P` intern fetches with macros during BSON encoding.
src/phongo_bson.c	Uses `PHONGO_INTERN_INIT_EX` for typemap BSON document/array initialization.
src/MongoDB/WriteResult.c	Refactors `$this`/object/free/debug intern fetch & allocation to macros.
src/MongoDB/WriteError.c	Refactors `$this`/object/free/debug intern fetch & init to macros (contains a type mismatch bug).
src/MongoDB/WriteConcernError.c	Refactors `$this`/object/free/debug intern fetch & init to macros.
src/MongoDB/WriteConcern.c	Refactors `$this`/object/free/debug intern fetch & init to macros.
src/MongoDB/TopologyDescription.c	Refactors `$this`/object/free intern fetch & init to macros.
src/MongoDB/Session.c	Refactors `$this`/object/free/debug intern fetch & allocation to macros.
src/MongoDB/ServerDescription.c	Refactors `$this`/object/free intern fetch & init to macros.
src/MongoDB/ServerApi.c	Refactors `$this`/object/free intern fetch & init to macros.
src/MongoDB/Server.c	Refactors `$this`/object/free intern fetch & allocation to macros.
src/MongoDB/ReadPreference.c	Refactors `$this`/object/free intern fetch & init to macros.
src/MongoDB/ReadConcern.c	Refactors `$this`/object/free intern fetch & init to macros.
src/MongoDB/Query.c	Refactors object/free/debug intern fetch & allocation to macros.
src/MongoDB/Monitoring/TopologyOpeningEvent.c	Refactors `$this`/object/free/debug intern fetch & allocation to macros.
src/MongoDB/Monitoring/TopologyClosedEvent.c	Refactors `$this`/object/free/debug intern fetch & allocation to macros.
src/MongoDB/Monitoring/TopologyChangedEvent.c	Refactors `$this`/object/free/debug intern fetch & allocation to macros.
src/MongoDB/Monitoring/ServerOpeningEvent.c	Refactors `$this`/object/free/debug intern fetch & allocation to macros.
src/MongoDB/Monitoring/ServerHeartbeatSucceededEvent.c	Refactors `$this`/object/free/debug intern fetch & allocation to macros.
src/MongoDB/Monitoring/ServerHeartbeatStartedEvent.c	Refactors `$this`/object/free/debug intern fetch & allocation to macros.
src/MongoDB/Monitoring/ServerHeartbeatFailedEvent.c	Refactors `$this`/object/free/debug intern fetch & allocation to macros.
src/MongoDB/Monitoring/ServerClosedEvent.c	Refactors `$this`/object/free/debug intern fetch & allocation to macros.
src/MongoDB/Monitoring/ServerChangedEvent.c	Refactors `$this`/object/free/debug intern fetch & allocation to macros.
src/MongoDB/Monitoring/CommandSucceededEvent.c	Refactors `$this`/object/free/debug intern fetch & allocation to macros.
src/MongoDB/Monitoring/CommandStartedEvent.c	Refactors `$this`/object/free/debug intern fetch & allocation to macros.
src/MongoDB/Monitoring/CommandFailedEvent.c	Refactors `$this`/object/free/debug intern fetch & allocation to macros.
src/MongoDB/Manager.c	Refactors `$this`/object/free intern fetch & allocation to macros.
src/MongoDB/Cursor.c	Refactors `$this`/object/free/debug intern fetch & allocation to macros.
src/MongoDB/Command.c	Refactors `$this`/object/free/debug intern fetch & allocation to macros.
src/MongoDB/ClientEncryption.c	Refactors `$this`/object/free intern fetch & allocation to macros (contains a missing intern fetch bug in debug info).
src/MongoDB/BulkWriteCommandResult.c	Refactors `$this`/object/free/debug intern fetch & init to macros.
src/MongoDB/BulkWriteCommand.c	Refactors `$this`/object/free/debug intern fetch & allocation to macros.
src/MongoDB/BulkWrite.c	Refactors `$this`/object/free/debug intern fetch & allocation to macros.
src/BSON/Undefined.c	Refactors object/free intern allocation/fetch to macros.
src/BSON/UTCDateTime.c	Refactors `$this`/object/free/debug intern fetch & init to macros.
src/BSON/Timestamp.c	Refactors `$this`/object/free/debug intern fetch & init to macros.
src/BSON/Symbol.c	Refactors `$this`/object/free/debug intern fetch & init to macros.
src/BSON/Regex.c	Refactors `$this`/object/free/debug intern fetch & init to macros.
src/BSON/PackedArray.c	Refactors `$this`/object/free/debug intern fetch & init to macros.
src/BSON/ObjectId.c	Refactors `$this`/object/free/debug intern fetch & init to macros.
src/BSON/MinKey.c	Refactors object/free intern allocation/fetch to macros.
src/BSON/MaxKey.c	Refactors object/free intern allocation/fetch to macros.
src/BSON/Javascript.c	Refactors `$this`/object/free/debug intern fetch & init to macros.
src/BSON/Iterator.c	Refactors `$this`/object/free/debug intern fetch & init to macros (including iterator handlers).
src/BSON/Int64.c	Refactors `$this`/object/free/debug intern fetch & init to macros.
src/BSON/Document.c	Refactors `$this`/object/free/debug intern fetch & init to macros.
src/BSON/Decimal128.c	Refactors `$this`/object/free/debug intern fetch & init to macros.
src/BSON/DBPointer.c	Refactors `$this`/object/free/debug intern fetch & init to macros.
src/BSON/Binary.c	Refactors `$this`/object/free/debug intern fetch & init to macros, including vector helpers.

Comments suppressed due to low confidence (1)

src/MongoDB/WriteError.c:1

This fetches a phongo_writeconcernerror_t* for a WriteError object. That is a type mismatch and can lead to invalid memory access in the destructor. Use PHONGO_INTERN_FROM_Z_OBJ(writeerror, object); here so intern has the correct struct type.

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

Copilot · 2026-03-27T15:20:11Z

+	zval retval = ZVAL_STATIC_INIT;
+	*is_temp    = 1;

 	array_init(&retval);



phongo_clientencryption_get_debug_info() no longer fetches intern (the previous intern = Z_OBJ_CLIENTENCRYPTION(object); was removed), but the function almost certainly still needs intern to populate debug info. This will either fail to compile (if intern is referenced later) or produce incomplete debug output. Add PHONGO_INTERN_FROM_Z_OBJ(clientencryption, object); near the top of this function and use intern as before.

alcaeus · 2026-04-02T10:36:02Z

@GromNaN @paulinevos do you feel like these new macros increase readability, or should we keep the original code?

GromNaN · 2026-04-07T12:34:55Z

+	do {                                                                   \
+		intern = zend_object_alloc(sizeof(phongo_##name##_t), class_type); \
+		zend_object_std_init(&intern->std, class_type);                    \
+		object_properties_init(&intern->std, class_type);                  \
+	} while (0)


Why do you need this while loop?

This pattern is a standard C idiom for writing multi-statement macros safely. It lets us wrap multiple statements in a macro that behaves like a single statement. This is important in certain contexts, most importantly when used in an if/else without braces (which we fortunately don't use):

if (condition) MY_MACRO(x); // expands safely; no dangling-else or missing-brace bugs else other();

Without the do/while, an if-less { ... } block would break the else, and a bare statement list would only execute the first line under the if. The while (0) is optimized away by the compiler, so there's no runtime cost — it's purely a syntactic wrapper.

And the variable declaration is moved up to the begining of the function by the compiler, so it's not an issue.

GromNaN

It seems to me that this makes things clearer and more consistent.

paulinevos

I like, just one small naming suggestion

alcaeus requested a review from a team as a code owner March 27, 2026 15:15

alcaeus requested review from Copilot and paulinevos and removed request for a team March 27, 2026 15:15

Copilot started reviewing on behalf of alcaeus March 27, 2026 15:16 View session

Copilot AI reviewed Mar 27, 2026

View reviewed changes

alcaeus added 4 commits April 1, 2026 13:16

Add macro to declare and extract internal struct from $this

e3afdd2

Add macros to declare intern from zend_object and zval

c542f76

Add macro to simplify object allocations

8e06a4b

Add macro to simplify calls to object_init_ex

a3f05bf

alcaeus force-pushed the simplify-intern-declarations branch from 417862e to a3f05bf Compare April 1, 2026 11:58

GromNaN reviewed Apr 7, 2026

View reviewed changes

GromNaN approved these changes Apr 7, 2026

View reviewed changes

paulinevos approved these changes Apr 8, 2026

View reviewed changes

Comment thread src/BSON/Binary.c

alcaeus merged commit 3073fe4 into mongodb:v2.x Apr 8, 2026
43 checks passed

alcaeus deleted the simplify-intern-declarations branch April 8, 2026 11:39

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Add macros to simplify handling of internal structs#1965

Add macros to simplify handling of internal structs#1965
alcaeus merged 4 commits intomongodb:v2.xfrom
alcaeus:simplify-intern-declarations

alcaeus commented Mar 27, 2026

Uh oh!

Copilot AI left a comment

Uh oh!

Copilot AI Mar 27, 2026

Uh oh!

alcaeus commented Apr 2, 2026

Uh oh!

GromNaN Apr 7, 2026

Uh oh!

alcaeus Apr 7, 2026

Uh oh!

GromNaN Apr 7, 2026

Uh oh!

GromNaN left a comment

Uh oh!

paulinevos left a comment

Uh oh!

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

4 participants

Conversation

alcaeus commented Mar 27, 2026

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Pull request overview

Reviewed changes

Uh oh!

Copilot AI Mar 27, 2026

Choose a reason for hiding this comment

Uh oh!

alcaeus commented Apr 2, 2026

Uh oh!

GromNaN Apr 7, 2026

Choose a reason for hiding this comment

Uh oh!

alcaeus Apr 7, 2026

Choose a reason for hiding this comment

Uh oh!

GromNaN Apr 7, 2026

Choose a reason for hiding this comment

Uh oh!

GromNaN left a comment

Choose a reason for hiding this comment

Uh oh!

paulinevos left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

4 participants