I'd rather replace constructor argument with new one.

This is how it is now, there is no constructor. I was about to add it, but wouldn't that enforce an unnecessary stronger binding between the actual current implementation and the way things works? Here, the only thing the code needs to know is that anaddPool method can be called. Looks a looser binding than a constructor argument to me. Given that this is not critical perf code to say the least, I think it should be kept asis.

The compiler pass is already implementation-aware, becauseaddPool is not part of any interface as I can see. It's more likePsr6CacheClearerPass to be honest.

And I think it's OK for compiler pass to be locked on specific implementation, DIC is the place for this knowledge.

So, what's your argument supporting the change? (you have mine :) )

Given you bother withisset vsarray_key_exists performance, I wonder why you didn't take into account this one. Same kind of micro-optimization for me. :)

And yet another mutable service looks painful, just for sake of clean code.

Given you bother with isset vs array_key_exists performance

I don't know how to react to this. Is it ironic? Or are you wondering why here performance is not and issue, and why it is in the container methods? I'll take the second option. A clearer is called every once in a while. On the container,get can be called thousands of times. 1000xsmall-optim == big optim. That's why it matters there.

I just wanted to say that I see no reason to prefer setter injection over constructor one. Same level of coupling for me, better performance of constructor injection, cleaner API, less side-effects, etc.

I see that the class has no constructor, but it's not an issue to add it.

Even if somebody added his own implementation, he'd look into this source code and find out that he needs to addaddPool method or adapt constructor signature, i.e. same kind of changes. How setter is looser coupling?

Even if somebody added his own implementation, he'd look into this source code and find out that he needs to add addPool

That's my point yes, and I find that having a convention that applies to a method is more open that one that applies to the constructor. That's why I said looser coupling.

Anyway, this is not directly related to this PR, which is just moving this line of code around, nothing new here.

If you really care, I propose you open a PR to change that.

Why this class is open for inheritance (not marked asfinal)?

here we are

I don't understand logic behind this code (array_reverse +break). Do you want to emulate overriding of the tag? Does it mean that a service cannot have 2 different clearers?

cannot have 2 different clearers

True: neither in 3.1 nor with this PR. Could be worth it, I'll give it a try.

But not in this PR, that's another topic :)