How would you feel about merging this change to 6.1 instead of 4.4?

I hesitated. I think I'd be fine with it. Calling@jderusse for his opinion about that.

(PR updated to account for PHP 7.4, which has the same issue)

Is it possible to trigger a deprecation or reporting this in theerror_log?

How would you feel about merging this change to 6.1 instead of 4.4?

Actually, this PR fixes a bug insymfony/di: 4.4 => the generated preload script could trigger errors in PHP when class has typed props. It could be a symfony 6.1 packageor in whatever packages.

I'm not sure why we would only fix it in symfony/di 6.1.

I'm fine with bumping the php requirement as in#45377.

It could be a good idea to do both: Increase php requirement in 6.1 to increase maintainability and skip preloading in older php versions so people are not affected by this common problem, e.g. if it happens in other libraries.

But I would suggest to skip preloading in the recipehttps://github.com/symfony/recipes/blob/master/symfony/framework-bundle/5.4/config/preload.php instead. This way, people can still use preloading in 7.4/8.0 if they are not affected by optional dependency types by just removing the condition.
So this would solve

The drawback of this approach is that many apps that don't use typed properties would benefit from the perf boost provided by preloading on PHP 7.4/8.0.

I rebased this PR for 6.1: the more I think about it, the more I see merging this on 4.4 as something that will break the perf of perfectly fine infras, while fixing an issue that is yet to be reported by userland. I think the topic for branches < 6.1 should be dealt with separately, and should be based on real-world feedback.

I would suggest to skip preloading in the recipehttps://github.com/symfony/recipes/blob/master/symfony/framework-bundle/5.4/config/preload.php instead

that's a nice idea, but here also I wouldn't take a preemptive approach. If the issue is super rare on < 6.1 the correct approach could very well be to tell to disable preloading on their own. Let's wait for the issue to be reported in real world before doing anything.

seescheb/2fa#121 for real-world reference

Thanks for the link@jderusse that's interesting. I still think we should not break the perf of existing infras lightly, but of course this topic remains open.

Closing as I don't think we should break the perf of perfectly fine infras lightly and#45377 is going to solve this for 6.1+.