Skip to content
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

Intuitive way to convert constructor to closure #12336

Closed
mindplay-dk opened this issue Oct 1, 2023 · 6 comments
Closed

Intuitive way to convert constructor to closure #12336

mindplay-dk opened this issue Oct 1, 2023 · 6 comments

Comments

@mindplay-dk
Copy link

Description

I wish one of these ways worked:

<?php

class Foo
{
    public function __construct(public string $lol) {}
}

$class = new ReflectionClass(Foo::class);

// method 1:
$ctor = $class->getConstructor()->getClosure(); // ReflectionMethod::getClosure(): Argument #1 ($object) cannot be null for non-static methods

$classname = Foo::class;

// method 2:

$ctor = new $classname(...); // Fatal error: Cannot create Closure for new expression

I'd expect both approaches to work - a constructor is conceptually just a static function that generates a new instance, although the reflection facility doesn't treat it or report it as such.

The second approach at least ought to work - for consistency with that feature.

Even new Foo(...) doesn't work - and judging by the error message, someone predicted what I'm trying to do, but it just isn't supported... yet? is this feature planned?

(I know there are ways to do it, using reflection, so you don't need to explain that - just wondering why this feature isn't available, as it would seem natural to expect this to work.)

@iluuu1994
Copy link
Member

A constructor is essentially just an instance method that is called after constructing a new object. I do agree that conceptually most people would intuitively understand new Foo(...) the way you have described it. Nonetheless, https://wiki.php.net/rfc/first_class_callable_syntax#object_creation explicitly chose not to implement this feature, and as such this would certainly require an RFC.

@mindplay-dk
Copy link
Author

The RFC explains what the expectation would be:

The general expectation is that new Foo(...) would be creating a new instance of Foo on each call, rather than creating a single instance of Foo and then repeatedly calling the constructor.

That seems pretty straight forward - it's exactly what I was expecting.

But then it goes on to say:

While certainly possible, this takes a step backwards from the straightforward semantics of the foo(...) syntax for ordinary calls.

I'm not sure why having this feature work consistently for constructor calls is considered a step backwards?

    foo(...)          is equivalent to         fn (...$args) =>     foo(...$args)
new Foo(...)          is equivalent to         fn (...$args) => new Foo(...$args)

This looks like a consistent and a logical expectation to me.

In either case, it's of course not as simple as merely producing a "trampoline function" like the RFC suggests - in either case, what actually needs to happen is you copy (or reference) the parameter list, type-hints and all, so things like reflection work.

If I was to write an RFC, that's all it would say: the paragraph from the existing RFC, plus this paragraph about making it work with reflection.

But perhaps we should hear from the RFC author before writing another RFC?

@damianwadley
Copy link
Member

a constructor is conceptually just a static function that generates a new instance,

I'm not sure why having this feature work consistently for constructor calls is considered a step backwards?

It seems you think that constructors work like factory methods. They don't. Not in the slightest. Really, it's only a small number of major languages that work that way*, while every other language treats them as a special mechanism. No, new is an operator which tells PHP to set up the object internally and then call a __construct (if present) on it, before then delivering that object back to the calling code.

Realize that constructors are not static methods, that they don't return anything, that new Foo (no parentheses) is perfectly valid syntax, and that PHP's parent::__construct idiom is actually rather bizarre compared to the rest of the programming world, and the "for consistency" argument starts to fall apart.

If you want closure-ified object construction then that's one thing, but saying "you can do this with methods so it should work with constructors too" is naive.

If I was to write an RFC, that's all it would say: the paragraph from the existing RFC, plus this paragraph about making it work with reflection.

Can I assume you mean that in jest? Because it would be a very effective way of making sure nobody would support the RFC.

Such a short RFC would suggest the author didn't put much time or effort into thinking about what they were proposing. For instance, whether they considered ("for consistency") whether this should be possible on cloning too.


* Rust's Foo::new() is one example, except "new" is actually just a convention for creating and naming a factory method (and in fact, ::default is actually more of an officially established concept than ::new is, though it serves the opposite purpose). Kotlin's mere Foo() is another, except, given that the language is an abstraction layer on top of another language/runtime, it's only the syntax that looks like a function call.

@mindplay-dk
Copy link
Author

@damianwadley you're explaining how it's implemented - the implementation details are besides the point.

new Foo() is conceptually (effectively, in practice, "as far as the programmer knows or cares") a static call to Foo::__construct in which $this is implicitly initialized with an empty instance according to property initializers.

what I would want out of this feature is essentially for new Foo(...) to be syntactic sugar for:

fn(int $a, string $b) => new Foo(int $a, string $b)

where the parameter list would be identical to the constructor of Foo.

if the implementation details aren't simple, the concept and the semantics certainly are both simple and intuitive.

Can I assume you mean that in jest? Because it would be a very effective way of making sure nobody would support the RFC.

I honestly don't know how to stamp it out with any greater verbosity - it's not a complex feature.

What else would you like an RFC to say?

@mindplay-dk
Copy link
Author

Okay, so I'd like to add two things.

First, my motivation for this proposal was for dependency injection - I would have liked to be able to convert a constructor to a closure for the purpose of obtaining a simple factory-function, which I could use in a DI container.

However, I started really thinking about how this would work in practice, and I realized, it's not actually going to work.

What I wanted for the DI container I've been tinkering with, was to access the argument types of the factory function - and while that would be possible, the performance implications (for most ordinary PHP projects) simply wouldn't be acceptable.

The simple realization is that, in order to reflect the constructor arguments, you would of course need to autoload the class - which, in the case of my DI container, which tries to validate the entire container ahead of time, that would, in practice, mean autoloading every type that's been bootstrapped. 😱

In other words, the performance implications of being able to promote a constructor to a closure are too unpredictable, and doesn't really jive with how PHP code generally relies on loading everything as late as possible.

I mean, it's not unthinkable that someone might find it useful to be able to convert a constructor to a closure anyway, but it's no use for the thing I had in mind.

I'm sorry if I came off short - before thinking through the implementation details, it seemed obvious to me that this would "just work", but I'm guessing unpredictable performance might be one reason why someone chose not to implement this in the first place.

You may consider this idea withdrawn, and feel free to close this issue if you don't think this feature is a good idea.

@iluuu1994
Copy link
Member

Thanks @mindplay-dk for your response.

@iluuu1994 iluuu1994 closed this as not planned Won't fix, can't repro, duplicate, stale Oct 23, 2023
thekid added a commit to xp-framework/compiler that referenced this issue Mar 2, 2025
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

3 participants