[php-src] Issue #20645: New Zend fake scope guard API proposal
[email protected] (alexandre-daubois)
| Newsgroups | php.bugs |
|---|---|
| Message-ID | <SgH1kjbVeRONgCgxSl2B5k3UB2VJmYRX05KUeGaCKmE@main.internal.php.net> |
Issue: https://github.com/php/php-src/issues/20645
Author: alexandre-daubois
### Description
In a recent PR (#20373), we needed to use executor global `fake_scope`. During the review, [the following comment](https://github.com/php/php-src/pull/20373#pullrequestreview-3497642325) has been posted:
> We should probably introduce some nicer api for that fake_scope as it has not been used outside zend and reflection. It should not be directly accessed filters code IMHO but new API is more master material...
I think this makes sense. Here is how we currently use `fake_scope` in the code:
```c
const zend_class_entry *old_scope = EG(fake_scope);
EG(fake_scope) = ce;
// ...
EG(fake_scope) = old_scope;
```
We need to access internal Zend globals directly and the save/restore is manual and error prone.
High-level wrappers exist (`zend_read_property_ex`, `zend_update_property_ex`) that already handle `fake_scope` internally, but they're not flexible enough for complex cases. Introducing a new API like this one would help:
```c
typedef struct _zend_fake_scope_guard zend_fake_scope_guard;
ZEND_API zend_fake_scope_guard *zend_fake_scope_guard_enter(
const zend_class_entry *scope
);
ZEND_API void zend_fake_scope_guard_exit(zend_fake_scope_guard *guard);
```
This is the kind of diff we could expect on current call sites:
```diff
diff --git a/ext/reflection/php_reflection.c b/ext/reflection/php_reflection.c
index d6e55c982b4..3931f648e2b 100644
--- a/ext/reflection/php_reflection.c
+++ b/ext/reflection/php_reflection.c
@@ -5932,11 +5932,10 @@ ZEND_METHOD(ReflectionProperty, getValue)
}
}
- const zend_class_entry *old_scope = EG(fake_scope);
- EG(fake_scope) = intern->ce;
+ zend_fake_scope_guard *guard = zend_fake_scope_guard_enter(intern->ce);
member_p = Z_OBJ_P(object)->handlers->read_property(Z_OBJ_P(object),
ref->unmangled_name, BP_VAR_R, ref->cache_slot, &rv);
- EG(fake_scope) = old_scope;
+ zend_fake_scope_guard_exit(guard);
if (member_p != &rv) {
RETURN_COPY_DEREF(member_p);
```
The intent is explicit, we don't directly access executor globals and it has the same flexibility as manual save/restore.
Given that we're talking about an internal Zend API, I'm not 100% sure this should follow the same process as classical RFCs and/or be announced in the internal mailing list. If yes, please let me know and I'll take care of it.