[solc] Unassigned immutables cryptic error

[bshastry]

contract C1 {
  bool immutable s1 = false;
  constructor() { (int8(127) * 3); }
  function f() external returns (bool) {
    if (s1)
        return true;
    else
        return false;
  }
}

$ solc --experimental-via-ir --optimize test.sol
...
Warning: Source file does not specify required compiler version!
--> test/libsolidity/semanticTests/testYulCompilerOptimisation.sol

Warning: Statement has no effect.
 --> test/libsolidity/semanticTests/testYulCompilerOptimisation.sol:3:19:
  |
3 |   constructor() { (int8(127) * 3); }
  |                   ^^^^^^^^^^^^^^^

Warning: Function state mutability can be restricted to view
 --> test/libsolidity/semanticTests/testYulCompilerOptimisation.sol:4:3:
  |
4 |   function f() external returns (bool) {
  |   ^ (Relevant source part starts here and spans across multiple lines).

Error: Some immutables were read from but never assigned, possibly because of optimization.

Legacy optimizer does not report and error. Legacy and Sol->Yul without optimization don't report either. Both legacy (with and without opt) and Sol->Yul (without opt) result in a runtime failure due to int8 overflow inside the constructor.

[hrkrshnn]

The constructor always reverts (checked arithmetic), and therefore the DeadCodeEliminator removes the code following the revert, which includes setimmutable.

Not sure what should be done in this case. Perhaps improve the error to say that it's likely that the constructor will always revert?

[bshastry]

The constructor always reverts (checked arithmetic), and therefore the DeadCodeEliminator removes the code following the revert, which includes setimmutable.

Yeah, came to that conclusion.

Not sure what should be done in this case. Perhaps improve the error to say that it's likely that the constructor will always revert?

Perhaps the error could say "constructor will always revert". At least that is easier to follow than the somewhat cryptic immutables are unassigned? Just curious if it's always cos of constructor revert though, because if it is not there may be cases where even the new error may not make sense :-)

[chriseth]

The same error will be generated for

contract C {
uint immutable x;
constructor() {
if (false)  { x = 2; }
}
}

[bshastry]

Wondering if this could be a warning instead of an error.

[bshastry]

Small update: This is not solely a Sol->Yul issue, also happens with the legacy compiler (even without optimization turned on)

contract C {
  int8 immutable s4 = 15;
  constructor () {
      for(;;) {}
  }
  function f2() external returns (int8) {
    return s4 / 2;
  }
}

$ solc test.sol
...
Error: Some immutables were read from but never assigned, possibly because of optimization.

[axic]

Hmm, shouldn't it be a requirement that an immutable must always be assigned in the constructor? As opposed to allow the "default value" rule?

[hrkrshnn]

Hmm, shouldn't it be a requirement that an immutable must always be assigned in the constructor

But how would that be enforced? One can always have a setimmutable inside a conditional branch that will never be reached.

The default value of zero for uninitialized immutable does seem consistent.

[axic]

Error: Some immutables were read from but never assigned, possibly because of optimization.

Then why this error? If zero-by-default is allowed, then we do not need this special error.

[chriseth]

You cannot assign an immutable in a conditional branch. We do not rely on the default value.

This error is about the optimizer removing the setimmutable and it is an additional low-level check because the high-level check does not catch all situations.

We can improve the error message.

It happens if you have an infinite loop or a require that always fails before you assign the immutable.

[cameel]

Is this a duplicate of #12864?

[bshastry]

Is this a duplicate of #12864?

I don't think so. Here the problem is the Yul optimiser removing unconditional setimmutable because it is safe to but then erroring on immutable reaf because the said setimmutable was removed.

There the problem is conditional initialization of immutable.

[ekpyron]

To be clear: the fix here is just to improve the error message in codegen. We should have two mechansisms to address situations where immutables are unassigned: one in the high-level language, which should account for the assignment to be syntactically missing - one in codegen, which checks the code after optimization.

The error message could just additionally to possible because of optimization say that the code path assigning the constructor is probably unreachable due to reverting or something like that.