[HttpKernel][DI] Enable Kernel to implement CompilerPassInterface #24257
Conversation
fc44f38 to
2e1a0ea
Compare
| @@ -767,6 +768,9 @@ protected function getContainerBuilder() | |||
| $container = new ContainerBuilder(); | |||
| $container->getParameterBag()->add($this->getKernelParameters()); | |||
|
|
|||
| if ($this instanceof CompilerPassInterface) { | |||
| $container->setKernelPass($this); | |||
There was a problem hiding this comment.
Why do we need to set the kernel pass in the container builder (which is a bit weird to me) rather than simply adding it through $container->getCompilerPassConfig()->addPass(...) or $container->addCompilerPass(...)? Is processing the pass right after the extensions (in ExtensionCompilerPass) the best option? I think it probably is, but I'd like to get the full reasoning behind :)
There was a problem hiding this comment.
Good question :)
The reason is as you said that this pass should be run last after bundle passes, and before optimization passes.
There was a problem hiding this comment.
Actually, I now changed the implementation so that no new method is needed. Using priorities should be good enough and better in fact. WDYT?
There was a problem hiding this comment.
Much better to me 👍
It can be a small BC break for people already relying on priorities though. So should we mention this somewhere?
Also the CHANGELOG.md entry is missing :)
5779673 to
ad1f706
Compare
ad1f706 to
052a881
Compare
|
@ogizanagi comments addressed. |
052a881 to
aa986d7
Compare
aa986d7 to
7c9d2db
Compare
| @@ -4,6 +4,7 @@ CHANGELOG | |||
| 3.4.0 | |||
| ----- | |||
|
|
|||
| * moved the `ExtensionCompilerPass` to before-optimization passes with priority -1000 | |||
There was a problem hiding this comment.
may be worth adding this to the upgrade file too
7c9d2db to
b3b8e24
Compare
In the same spirit as #13761 that allowed DI exts to be also compiler passes, and as #23812 that allowed the kernel to listen to events, in our new bundle-less world, should we allow the kernel to register itself as a compiler pass? That would make some scenario possible (like having a
TestKernelthat turns some services public.)