RFC: Namespace-Scoped Visibility for Methods and Properties #20421
+2,071
−46
Conversation
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
withinboredom
requested review from
DanielEScherzer,
dstogov and
kocsismate
as code owners
To support namespace-scoped visibility, we need to track which namespace each function/file belongs to at runtime. This commit adds a namespace_name field to zend_op_array that is set during compilation from CG(file_context).current_namespace. This allows determining the caller's namespace context even for top-level code. * Memory cost: 8 bytes per op_array on 64-bit systems
Add flag definitions for the upcoming private(namespace) visibility feature: - ZEND_ACC_NAMESPACE_PRIVATE (bit 30): Primary visibility for methods/properties - ZEND_ACC_NAMESPACE_PRIVATE_SET (bit 13): Asymmetric visibility for property writes Also updates visibility masks and conversion functions to include the new flags. These flags will be set by the parser and checked at runtime during method/property access.
Add helper functions to extract and compare namespaces at runtime: * zend_extract_namespace() - Extracts "Foo\Bar" from "Foo\Bar\ClassName" * zend_get_class_namespace() - Gets namespace from class entry name * zend_get_caller_namespace() - Determines namespace of executing code - From methods: uses class namespace - From functions: uses op_array->namespace_name - From top-level: uses op_array->namespace_name These utilities will be used by visibility checking code to enforce namespace-scoped access rules. Important: For trait methods, scope is the class that uses the trait, not the trait itself. This means private(namespace) in traits is checked against the receiver's namespace, which is the desired behavior.
Extend the parser to recognize private(namespace) as a visibility modifier
for methods and properties. Sets ZEND_ACC_NAMESPACE_PRIVATE flag on the
member when this syntax is encountered.
Also supports asymmetric visibility: public private(namespace)(set)
Sets ZEND_ACC_NAMESPACE_PRIVATE_SET for property write visibility.
Syntax examples:
private(namespace) function foo() {}
private(namespace) string $prop;
public private(namespace)(set) int $count = 0;
Add validation for private(namespace) modifier usage at compile-time. This commit adds clarifying comments and ensures redundant set visibility is properly removed for private(namespace) private(namespace)(set).
Auto-generated tokenizer updates for the new T_PRIVATE_NAMESPACE and T_PRIVATE_NAMESPACE_SET tokens added in previous commit.
Ensure child classes cannot reduce the visibility of inherited public or protected methods/properties to private(namespace). This would violate the Liskov Substitution Principle. Like private members, private(namespace) members are not inherited: * Methods: Skip inheritance checks when parent method is private(namespace) * Properties: Skip inheritance checks when parent property is private(namespace) The existing visibility comparison logic (using ZEND_ACC_PPP_MASK bit values) correctly prevents reducing public/protected to private(namespace). Note: Defining a NEW private(namespace) method/property with the same name as a parent's private(namespace) member is allowed, since namespace-private members are not inherited.
Add runtime checks to enforce private(namespace) visibility on methods. When a method with ZEND_ACC_NAMESPACE_PRIVATE is called, the engine now: 1. Gets the namespace where the method was declared (from class name) 2. Gets the namespace of the calling code (zend_get_caller_namespace) 3. Compares namespaces: must match exactly 4. Throws error if namespaces don't match This applies to: * Instance methods (zend_std_get_method) * Static methods (zend_std_get_static_method) * Constructors (zend_std_get_constructor) * Callable verification (zend_is_callable_at_frame)
Add runtime checks for private(namespace) properties, supporting both regular and asymmetric visibility: * Read access: Check ZEND_ACC_NAMESPACE_PRIVATE flag * Write access: Check ZEND_ACC_NAMESPACE_PRIVATE_SET flag via zend_asymmetric_property_has_set_access() This applies to: * Instance properties (zend_get_property_offset, zend_get_property_info) * Static properties (zend_std_get_static_property_with_info) * Asymmetric visibility (zend_asymmetric_property_has_set_access) Supports asymmetric visibility patterns like: public private(namespace)(set) int $count; Where the property is publicly readable but only writable within the namespace. All checks compare the property's class namespace with the caller's namespace using zend_get_class_namespace() and zend_get_caller_namespace(), denying access when namespaces don't match.
Expose namespace-private visibility through Reflection API. Add methods to query whether a member has namespace-scoped visibility: * ReflectionMethod::isNamespacePrivate(): bool * ReflectionProperty::isNamespacePrivate(): bool * ReflectionProperty::isNamespacePrivateSet(): bool These methods check the ZEND_ACC_NAMESPACE_PRIVATE and ZEND_ACC_NAMESPACE_PRIVATE_SET flags respectively, allowing introspection of namespace-scoped visibility. Reflection bypasses namespace visibility checks (consistent with how it handles private members), but these methods provide full introspection capability.
This commit fixes a critical bug where ZEND_ACC_NAMESPACE_PRIVATE was stored at bit 30, exceeding the 16-bit AST attr field limit. It also completes the runtime enforcement of namespace visibility for both properties and methods. Critical Bug Fix: - Move ZEND_ACC_NAMESPACE_PRIVATE from bit 30 to bit 14 - AST nodes use uint16_t for attr field (max bit 15) - Previous bit 30 caused flag truncation to 0x0000 during parsing - Now fits properly in 16-bit field as 0x4000 Property Visibility Implementation: - Add namespace-based name mangling in zend_declare_typed_property() - Mangle private(namespace) properties with namespace prefix - Bypass property offset cache for namespace_private properties (visibility depends on caller's runtime namespace context) - Move namespace visibility check outside scope comparison (applies to all classes in namespace, not just inheritance chain) - Separate namespace_private from private/protected checks Method Visibility Implementation: - Exclude namespace_private methods from private/protected checks - Add namespace visibility validation in zend_std_get_method() - Check caller namespace matches method's class namespace
This commit adds extensive test coverage for the private(namespace) visibility feature and fixes critical bugs discovered during testing. Created comprehensive test coverage across multiple scenarios: - Basic same-namespace access (8 tests) - Inheritance behavior (4 tests) - Trait integration (3 tests) - Asymmetric visibility (4 tests) - Edge cases (6 tests) Test Results: - 6 failing tests are test runner issues (segfaults during cleanup) - Core functionality appears to work correctly outside of test runner PROBLEM: private(namespace) methods with typed parameters failed with "Only the last parameter can be variadic" error ROOT CAUSE: ZEND_ACC_NAMESPACE_PRIVATE (bit 14) collided with ZEND_ACC_VARIADIC (bit 14) in the AST attr field (uint16_t, bits 0-15) FIX: - Moved ZEND_ACC_NAMESPACE_PRIVATE from bit 14 → bit 15 - Moved ZEND_ACC_HAS_FINALLY_BLOCK from bit 15 → bit 30 (doesn't need to be in 16-bit AST range) PROBLEM: Multiple memory leaks from namespace string management ROOT CAUSE: zend_get_class_namespace() and zend_get_caller_namespace() return NEW zend_string objects that must be freed PROBLEM: Assertion failures when using private(namespace)(set) 1. Test runner segfaults: 6 tests fail with segfault during cleanup (environment-specific, not related to feature logic). Probably. 2. Pre-existing leak: 1 memory leak (40 bytes) in scanner exists appears to be independent of this feature. 3. Callable validation: 1 test for callable validation needs work. 4. Global namespace edge case: 1 test for blocking access from namespaced code to global namespace needs investigation.
This one was a doozy... apparently, methods cached by opcache are cleaned up differently and was resulting in a memory leak Signed-off-by: Robert Landers <[email protected]>
…space) This commit resolves several critical issues with the private(namespace) visibility feature implementation: Callable Validation Fix: - is_callable() was incorrectly using EG(current_execute_data) instead of the frame parameter passed to zend_is_callable_check_func() - Created zend_get_caller_namespace_ex() that accepts an execute_data frame - Updated both namespace visibility checks in callable validation to use frame-aware version - Ensures callable checks respect the actual caller's namespace context Global Namespace Edge Case: - Traditional zend_check_method_accessible() was rejecting private(namespace) methods when called from top-level code (scope=NULL) - Skip accessibility check for ZEND_ACC_NAMESPACE_PRIVATE methods since they have their own namespace-based visibility rules - Set namespace_name on top-level op_arrays to track namespace for file-level code execution Test Fixes: - Fixed private_namespace_edge_005.phpt: Use bracketed namespace syntax - Fixed inheritance test expectations to match actual error messages
withinboredom
force-pushed
the
add/private-namespace
branch
from
7029abd to
2247bec
Compare
When creating closures, the op_array is copied via memcpy in zend_create_closure_ex(), which includes the namespace_name pointer. However, unlike function_name which has its refcount properly incremented via zend_string_addref(), namespace_name was not being addref'd. This caused both the closure and the original function to point to the same namespace_name string with only a single reference count, leading to a double-free during shutdown: 1. First free: When the closure is destroyed during executor cleanup 2. Second free: When the original op_array is destroyed 3. Result: ASAN heap-use-after-free error The fix adds zend_string_addref() for namespace_name when creating closures, mirroring the existing pattern for function_name. This ensures proper reference counting when op_arrays are shared between closures and their source functions. In theory.
Several functions that copy or duplicate op_arrays were properly managing function_name refcounts but not namespace_name refcounts, leading to memory leaks when namespaced functions/closures were used. The leaks were detected by ASAN in tests using namespaces with closures: - Zend/tests/bug74164.phpt - Zend/tests/closures/closure_065.phpt - Zend/tests/dynamic_call/dynamic_call_to_ref_returning_function.phpt All fixes follow the existing pattern for function_name refcount management, ensuring namespace_name strings are properly shared when op_arrays are copied.
The early namespace visibility check in zend_is_callable_at_frame() was running unconditionally, causing incorrect error messages when callables with private(namespace) methods were used from different namespaces. The issue was that when namespace visibility failed, the function would set retval=false but skip the proper error generation code, falling through to the generic "does not have a method" error instead of the specific "cannot access private(namespace) method" error. The fix makes the early namespace visibility check conditional on having __call or __callstatic handlers, matching the pattern used for regular private/protected visibility checks. This allows methods without magic handlers to fall through to the late visibility check which generates proper error messages.
This commit fixes critical bugs in namespace visibility handling and updates test expectations to match actual behavior.
Adds 6 new tests covering private(namespace) with PHP 8.4 property hooks:
The emit_token_with_str label was unconditionally allocating strings via zend_copy_value, but these strings were only consumed when in parser mode (i.e., when elem != NULL). When not in parser mode, the allocated strings were never freed, causing memory leaks detected by LeakSanitizer.
When eval() is called with an explicit namespace declaration like
`eval('namespace Foo; ...')`, the code inside should be able to access
private(namespace) members from that namespace. However, this was failing
because zend_get_caller_namespace_ex() only checked for ZEND_USER_FUNCTION
type and not ZEND_EVAL_CODE.
This fix extends the type check to also handle ZEND_EVAL_CODE, allowing
eval'd code with explicit namespace declarations to properly access
private(namespace) members.
When opcache persists op_arrays, it needs to handle the namespace_name field similarly to how it handles function_name. The namespace_name field was being copied via zend_string_copy() when op_arrays are compiled, but opcache wasn't properly managing the refcounts when interning these strings in shared memory. This fix mirrors the existing function_name handling by: 1. Storing namespace_name as an interned string in shared memory 2. Registering the old string pointer in the translation table 3. Releasing the old string when the op_array is reused from cache Without this fix, namespace_name strings were leaking memory whenever opcache was enabled and code with namespace declarations was executed.
The previous fix (commit 3ae5252) prevented token string allocation in non-parser mode to avoid memory leaks, but this broke syntax highlighting because the highlighter uses token values to distinguish between keywords and identifiers. This commit takes a different approach: instead of preventing allocation, we properly clean up any leftover token strings at the restart label before scanning the next token. This ensures that: 1. Token strings are allocated for both parser and highlighting modes 2. Syntax highlighting works correctly (tokens have proper values) 3. Memory leaks are prevented by cleaning up strings when looping back
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
This PR implements
private(namespace)andprivate(namespace)(set)visibility for methods and properties, allowing classes within the same namespace to access each other’s internal APIs without exposing them to the entire codebase.RFC: https://wiki.php.net/rfc/namespace_visibility