ethereum / solidity

Solidity, the Smart Contract Programming Language
https://soliditylang.org
GNU General Public License v3.0
23.37k stars 5.78k forks source link

Accept empty optimizer sequence even with Yul optimizer disabled #14621

Closed cameel closed 1 year ago

cameel commented 1 year ago

Original comment: https://github.com/ethereum/solidity/pull/14149#discussion_r1174026509.

Description

One weird consequence of #14149 is that now, counter-intuitively, this in Standard JSON optimizer settings:

yul: false,

executes more optimizer steps than this:

yul: true,
yulDetails: {optimizerSteps: ":"}

This is not obvious to users as can be seen in #14619 or #14470. From UI perspective it would be less confusing to allow setting optimizerSteps: ":" regardless of whether the optimizer is enabled or not.

Specification

ekpyron commented 1 year ago

In principle, the nicer way to do this in the longer term would be something like: Deprecate the current yul-related settings altogether (we won't allow disabling stack optimizations in the long-run making the optimizer steps the only remaining setting specific to the yul optimizer) and just have a single setting yul: "...sequenceOfSteps...".... maybe with a string alias for the default sequence... Then "enabling" the yul optimizer would be having a non-empty yul optimizer sequence and "disabling" the yul optimizer would be setting the empty sequence...

But that'd be a breaking change and we won't yet remove the stack optimization setting, so for the time being the plan suggested above should be the best solution.

cameel commented 1 year ago

Then "enabling" the yul optimizer would be having a non-empty yul optimizer sequence and "disabling" the yul optimizer would be setting the empty sequence...

But that would make it impossible to run without UnusedPruner, which is what #14619 was about, wouldn't it? I.e. if we do it that way and yul: "" still runs UnusedPruner then users can no longer have a completely empty sequence. And if it does not, then you can run into "Stack too deep".

I think we still need to distinguish these two cases. The problem is really that currently it's possible to have even less optimization than with the optimizer nominally disabled, which is weird.

ekpyron commented 1 year ago

I don't follow, why? ":" as a sequence would fully disable the yul optimizer (including the unused pruner). The sequence would just have two different default values (neither of them ":") depending on the general enabled setting of the optimizer.

cameel commented 1 year ago

It would not fully disable the optimizer but it would not run UnusedPruner because there's no u in it. That was enough in #14619.