cadence icon indicating copy to clipboard operation
cadence copied to clipboard

Conditions (assert/pre/post) are eagerly evaluating messages

Open bjartek opened this issue 2 years ago • 3 comments

assert(true, message: "foo".concat("bar"))

The code above will never trigger, but it will still add computation since we have to evaluate the message.

bjartek avatar Jul 22 '22 15:07 bjartek

Thank you for pointing this out! This is correct behaviour, as, all function arguments are always eagerly evaluated, I see the problem here though.

Other languages solve this in various

  • In Kotlin, the message parameter is a function: https://kotlinlang.org/api/latest/jvm/stdlib/kotlin/assert. This means that the message is only evaluated when the message parameter function is called, not eagerly.
  • In Swift, the message parameter has an @autoclosure annotation: https://developer.apple.com/documentation/swift/assert(::file:line:). The autoclosure annotation automatically turns arguments into a function. This means that the message is only evaluated when the message parameter function is called, not eagerly.
  • In Scala, the message parameter is a call-by-name parameter (=>): https://www.scala-lang.org/api/2.13.6/scala/Predef$.html#assert(assertion:Boolean,message:=%3EAny):Unit Similar to Swift, this means that the message is only evaluated when needed, not eagerly.

Adding support for auto-closures or call-by-name seems like a lot of effort, for mostly this particular use-case.

Maybe we could adopt Kotlin's solution, i.e. change the message parameter to lazyMessage: ((): String), e.g. the example above would be:

assert(true, lazyMessage: fun (): String { return "foo".concat("bar") })

That's quite a lot more boiler-plate code though. We could maybe add sugar for function literals:

assert(true, lazyMessage: () => "foo".concat("bar"))

turbolent avatar Jul 22 '22 18:07 turbolent

On post conditions before has weird behavior too.

false || before(self.getBalance())>=0 calls getBalance, I know the reason but looks little strange

bluesign avatar Jul 22 '22 22:07 bluesign

Oh btw @autoclosure seems to be the elegant way to do this in my opinion. Backwards compatible. Basically rewriting ast to if !assert(expression) { panic(message) }

bluesign avatar Jul 24 '22 12:07 bluesign

Nice to have, not a stable Cadence blocker

j1010001 avatar Oct 19 '22 17:10 j1010001