Skip to content

Navigation Menu

Sign in
Appearance settings

Search code, repositories, users, issues, pull requests...

Provide feedback

We read every piece of feedback, and take your input very seriously.

Saved searches

Use saved searches to filter your results more quickly

Sign up
Appearance settings

RFC: Nested Classes #18207

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Closed
withinboredom wants to merge 26 commits into php:master from bottledcode:rfc/nested-classes
Closed

Conversation

Copy link
Member

@withinboredom withinboredom commented Mar 31, 2025

This is a complete rewrite of the previous PR: #18069

There are a couple things going on here and can be broken up into two parts:

  1. This makes namespaces into actual zend_class_entry types. This is used by nested classes for fast scope resolution and can be further used by a module implementation. These namespaces cannot be instantiated but are simply used for scope resolution only, as every class now has a "lexical scope" that is either a namespace or an outer class.
  2. Adds support for nested classes using \ as a scope operator.

These two work together to allow nested types to be resolved correctly at compile time:

namespace Foo\Bar;
class Inner {}
class Outer {
 class Inner {}
 class Middle {
 class Inner {}
 }
 public function foo(Inner $inner): void {} // resolves to Foo\Bar\Outer\Inner
 public function bar(\Foo\Bar\Inner $inner): void {} // FQN resolves to one in namespace
 public function baz(Middle\Inner $inner): void {} // resolves to \Foo\Outer\Middle\Inner
}

This implementation works through the use of three members on the zend_class_entry struct:

  • required_scope: for requiring that the class is used within a specific scope.
  • required_scope_absolute: when set to false, required_scope checks can use instanceof, otherwise the required scope must match exactly; thus false pertains to protected and true pertains to private.
  • lexical_scope: the class or namespace the class belongs to; used for visibility.

There are probably a number of things that can be improved in this PR, so feel free to suggest how this can be improved.

I'd like to improve how namespaces are managed, for example (this PR inspired #18189 and will likely benefit from what I learn there).

Other attempts

A different approach would involve using name mangling instead; however, this created issues: visibility and required scopes are subtly different from each other. This isn't as much an issue with things like properties and methods, because they are always the same rules. Nested types may be visible, but not be allowed to be used outside their scope:

class Outer {
 private class Middle {
 private class Inner {
 public function foo(): self {}
 }
 }
 private function foo(Middle\Inner $inner) {} // we can see Middle\Inner here
 public function bar(Middle\Inner $inner) {} // we cannot expose it outside the outer scope
}

Outer::foo() can "see" Outer\Middle\Inner, but the inner types cannot be exposed outside the outer class. These rules can possibly be implemented via name mangling, but some benchmarks showed that even a deeply recursive tree of pointer equality is often faster than even simple string operations.

krowinski reacted with thumbs up emoji mvorisek reacted with thumbs down emoji billisonline, zmitic, Panda-Program-Web, and JBlairy-Tikamoon reacted with heart emoji
The idea here is to create something that can be used for other things
essentially making this quite flexible.
required scope: when defined, the class can only be used in the
 required scope, or its descendents when absolute is false
lexical scope: the scope the class is defined in -- either a namespace
 or another class.
Now we modify the grammar to allow specifying visibility on a class.
This adds a new EG to support the addition of using namespaces as a
class's lexical scope
Now we can define nested classes inside other classes
Copy link

The idea is good, but since support would be added to indicate the visibility of the class, it would not be better to add such encapsulation within the Namespace?, something like what Java does with the packages

Copy link
Member Author

it would not be better to add such encapsulation within the Namespace?, something like what Java does with the packages

I'm not sure what you mean. If you mean, can this be used to create packages? Yes, it can; you need only a few changes to support that.

Copy link

for example

namespace A {
 public class PublicClass {
 public function __construct() {
 new PrivateClass; // works
 }
 }
 
 private class PrivateClass {}
}
namespace {
 $publicClass = new A\PublicClass();
 $privateClass = new A\PrivateClass(); // error
}

Copy link
Member Author

That is relatively simple to implement from here. This implementation leans on the fact that the parser thinks it is about to parse a property to expect public/private/protected, and the fact that properties reuse final/static/readonly/etc. It then has to translate the property modifiers into class modifiers during compilation... but there is currently only one slot left in ZEND_ACC for classes -- this is why I chose not to use a special ZEND_ACC slot for this feature. To implement packages, like you mention, we would need to take a ZEND_ACC slot, most likely; because this way of doing it won't work for top-level classes (IIRC).

Anyway, that's most likely an implementation detail to cross when we get there -- and the actual implementation may just use a ZEND_ACC slot, especially if packages and this were to pass. I intend to follow this up with packages/modules.

Copy link

fadrian06 commented Apr 21, 2025
edited
Loading

The only thing I want is to type PackageClass in the editor and LSP-intelephense doesn't burn me with 100 classes with the same name but different namespaces, some ones are internals from dependencies, if they were private, the autocomplete only shows me the public classes of those dependencies

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@kocsismate kocsismate Awaiting requested review from kocsismate kocsismate will be requested when the pull request is marked ready for review kocsismate is a code owner

@dstogov dstogov Awaiting requested review from dstogov dstogov will be requested when the pull request is marked ready for review dstogov is a code owner

@DanielEScherzer DanielEScherzer Awaiting requested review from DanielEScherzer DanielEScherzer will be requested when the pull request is marked ready for review DanielEScherzer is a code owner

Assignees
No one assigned
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

AltStyle によって変換されたページ (->オリジナル) /