In some ways it was actually compiling (resolving the parameters), as this is also done in a compiler pass. So we can also just go withcompile() +isCompiled() on both sides. And deprecate the concept of freezing, which breaks;

Still allowing to compile a already frozen container builder.

-1 for renaming compile to freeze. This method is not only about freezing. It is about performing all the compilation steps of the container building (everything done in compiler passes for instance).

@stof different approach. WDYT?

I don't understand the problem you are trying to fix here. Can you elaborate? Deprecating something should be taken lightly, so we need to have a very good reason to rename something.

It's confusing to mix&matchfreeze/is-frozen andcompile/is-compiled.isFrozen is currently enabled due compilation (ie. we practically havecompile/is-frozen). Imo, the parameter bag is frozen; the container compiled.

This exposes a minor code smell inContainerBuilder where it's additionally tracking state (ContainerBuilder::$compiled) where this is actually the same as checking if it's frozen (ie. compiled). I guess the difference was unclear back then, and you get "better be safe then sorry" changes.

I tend to be sympathetic to deprecating the "frozen" vocabulary on containers. It's been looking strange to me also and the difference is unclear with the "compile" concept.isCompiled looks better.
That may enhance DX, that would be the argument supporting this change.

Updated thePhpDumper as well.. it makes sense, but can be unpredictable when you have
new Container<Builder>(new FrozenParameterBag())

What do we expect when dumping this uncompiled (ie. we dont call$container->compile())? Currently the behavior is the same when doningnew Container<Builder>(new ParamaterBag())

@ro0NL : As we do not rely anymore on the$this->parameterBag instanceof FrozenParameterBag, I'd say it doesn't matter anymore... But...you'll probably have to check if the parameterBag is frozen in theContainerBuilder::merge() method before trying to add parameters.

@ogizanagi merging is only allowed before compiling. If you passed aFrozenParameterBag at that point i tend to let it crash (from the paramereter bag side that is).

Not sure what's best for DX here.. crashing or ignoring silently...

@nicolas-grekas any thoughts?

Other then that.. this is ready.

Oh and btw... just to be sure; the YAML+XML dumper really depend on a frozen bag right? Due escaping.

Rebase needed to take#19704 into account.

@ro0NL as far as i can see isCompiled will return false for dumped container whether or not container was compiled beforehand

I really don't know if this has a chance to be accepted by the core team, but let's say yes, I think we should go one step further and try deprecating dumping non-compiled containers.
WDYT?

@allflameisCompiled depends oncompile(). The PHP dumper doesnt compile your container, it''s more or less designed to cover both cases, ie.

• dump a compiled container(builder)
• compile a dumped container(builder)

@ro0NL I'm aware of that fact. Let me explain using your own statements:
SincePhpDumper has nothing to do with the compiled status, it shouldn't change it, right?
What I'm trying to say is that if you take your code, compile the container, you will getisCompiled resolving to true (correct), but if you dump it to the file and then create the container from the dumped file, you will getisCompiled resolving to false (incorrect), which is "strange" since you were dumping compiled container.
@nicolas-grekas tbh I'm not using the whole Symfony ecosystem so I cant think of implication this change can have (potential BC issues), but what I'm pretty sure of is that if you don't compile your container the "original" and "dumped" version won't necessarily be the same.

@allflame sorry, you're right#19704 applies toisCompiled as well, i should fix that 👍

About BC, i truly consider this a design flaw andthinkhope the impact is minimal.

Rebased. Still all good :)

I just realized i made a mistake with the latest deprecation:ed5b1d8#diff-f7b23d463cba27ac5e4cb677f2be7623R76

Im talkinguncompiled container here, looking at the class itshould have beennot-frozen container. I really dont mind this tiny semantic. Then again, if we dont do the PR it might be worth updating that instead... sorry :)

👍 with suggested changes

Updated, rebased & waiting for travis.

Cant wait for 4.0 guys 👍

@ro0NL Looks like tests fail.

Hm.. looks fixture related, maybe something added in the meanwhile. Ill have a look tonight.

Note to self;

• Symfony\Component\DependencyInjection\Tests\ContainerTest::testIsFrozen
• Symfony\Component\DependencyInjection\Tests\Dumper\PhpDumperTest::testClosureProxy

ping@ro0NL

Yes.. ill check it tonight. Focussed on#19668 first.. but that's more or less dealt with now.

All good@nicolas-grekas

LGMT@ro0NL Can you rebase one last time? Thanks.

@fabpot all good.

Thank you@ro0NL.