nowarp / misti

TON Static Analyzer
https://nowarp.io/tools/misti
Apache License 2.0
24 stars 1 forks source link

Double initialization of storage variables #97

Closed anton-trunov closed 2 months ago

anton-trunov commented 2 months ago

Summary

Double initialization of storage variables can be either a programmer's mistake or just a waste of gas.

Context

https://github.com/tact-lang/tact/issues/713

The analysis should try to take care of legitimate cases where a variable is eventually initialized after a series of mutations. Although, I don't this is going to be a widespread case.

Examples

contract Test {
    a: Int = 0;

    init(x: Int) { self.a = x }
}

Use instead:

contract Test {
    a: Int;

    init(x: Int) { self.a = x }
}
jubnzv commented 2 months ago

@anton-trunov I was thinking why don't we want to generate a compilation error here. I see the only valid case for this: if the developer is trying to implement some kind of conditional initialization:

contract Test {
    a: Int = 0;
    init(x: Int) {
         if (some_condition()) { // non-evaluable in compile-time
               self.a = x;
         } // otherwise the default value `0` is used
    }
}

Anyway, it looks like a code smell even in this case and should be reported. The explicit initialization in init would be easier to understand.

anton-trunov commented 2 months ago

@jubnzv Yeah, you can have almost arbitrary code in init() it's basically executed once before the very first receiver gets called, but other than that it's like any other piece of Tact code (that just needs to define the values of all storage vars).

I agree about the code smell argument, but I think the compiler is not in the position do disallow it.

anton-trunov commented 2 months ago

For instance, the user might want to use some kind of iterative process to initialize their storage vars