Should asserts in const expressions always be evaluated?

[leafpetersen]

A question was raised as to whether asserts in const expressions should always be evaluated, or only when asserts are otherwise enabled. Note that the notion of "asserts are otherwise enabled" is not necessarily well-defined at the point that consts are evaluated if we allow turning assertions on and off dynamically. In discussion, it seems useful to allow normal assertions to be:

• stripped out/not stripped out during compilation
• if not stripped out, turned on/turned off at runtime

Since consts are evaluated at compiled time, the only way we could turn const asserts on/off would be via the compile time control. That implies that we would only not evaluate const asserts if the compile time flag was off.

If we allow the compile time flag to turn off const assertions, then in table form the behavior looks like this:

	runtime flag on	runtime flag off
compile flag on	const + normal	const only
compile flag off	none	none

If we always evaluate const asserts, then the behavior looks like this:

	runtime flag on	runtime flag off
compile flag on	const + normal	const only
compile flag off	const only	const only

I don't see a lot of value in not evaluating const asserts. I also suspect that the analyzer always evaluates them: @stereotype441 is that correct?

So my inclination would be to go with the second behavior, where const asserts are always evaluated. The implementations are going with this behavior for now, since it is simpler to implement. If we decide to require the ability to turn on/off const asserts they can add this later as a non-breaking change.

cc @lrhn @munificent @eernstg @mraleph @a-siva

[munificent]

I don't see a lot of value in not evaluating const asserts.

+100. The only real value I see in not evaluating an assertion is performance. Const asserts have zero performance cost, so there's no reason to not always evaluate them.

[lrhn]

Agree, it brings real value to evaluate assertions, and the main reason not to is to avoid production overhead.

However, it means that:

class C {
  const C(int x) : assert(x != 0);
}

behaves differently between compile-time and run-time.
A const C(0) is a compile-time error and new C(0) is perhaps no error at all.
We have to make sure that we don't introduce run-time errors that shouldn't be there.

[eernstg]

Agreed, same reasons as @munificent and @lrhn! ;-)

I don't see a problem in new C(0) not being an error when assertions are turned off: This occurs when the person who runs the program has chosen to omit these checks, and we don't disable assertion checking because the program should be able to violate the assertions, just for performance.

[leafpetersen]

Okay, are we happy with accepting this behavior? If so, we should update the spec appropriately.

[rrousselGit]

There are some situations where it "removes overhead".

Some constructors cannot be declared as const, because they have an assert that calls a function:

class Foo {
  Foo(this.bar): assert(bar.trim().length > 0);

  final String bar;
}

When the assert is removed, this makes it possible to declare the constructor as const Foo(this.bar).
Which can then be used for performance optimization.

I don't think it truly matters, but worth noting.

[lrhn]

@rrousselGit
Not sure I understand the example. The Foo constructor with a non-constant assert is not allowed as constant, whether assertions are enabled or not.

There should be no difference between the allowed programs before and after this change, but the assertions that are part of a constant evaluation (read: asserts in const constructor initializer lists of constructors invoked as const) will be checked at compile-time even when run-time assertions are disabled.

[lrhn]

Seems like the CFE/Fasta has implemented the "constant asserts are always evaluated", but we haven't added it to the Null Safety specification

I'm not sure exactly how it's implemented by the CFE (no tests), and we haven't specified it formally yet, so just to say what I expect it to do:

Execution of an assert statement s of the form assert(e1, e2)
proceeds as follows:

• If the assert is evaluated as part of compile-time constant evaluation, then the expression e₁
  (which must be a potentially constant expression) is evaluated as a compile-time constant to a value c.
  It is a compile-time error if this evaluation would throw or if c is not the value true.
  Otherwise execution of s completes normally.
• If the assert is not evaluated as part of compile-time constant evaluation, and run-time assertions are enabled,
  then e₁ is evaluated to a value v.
  A dynamic error occurs if v is not an instance of the type bool.
  If v is false, then e₂ is evaluated to a value m, and then an AssertionError is thrown
  which contains m.
  Otherwise (when v is true) execution of s completes normally.
• Otherwise (when not evaluated as a constant and run-time assertions are disabled) execution of s completes normally.

(An assert of the form assert(e) is equivalent to assert(e, null), so we don't need to write that out).

I think this is the right choice. It avoids the front-end/Fasta compilers needing to be told whether assertions are enabled when compiling constants. Otherwise we'd need to let pkg/fasta/compiler accept the --enable-asserts flag too in order to be able to test constant assertion compile-time errors.

[a14n]

Regarding the above example it would be nice to have 2 kinds of assert in compile-time constant evaluation to allow:

class Foo {
  const Foo(this.bar): assert(bar.trim().length > 0); // not possible today
  final String bar;
}

If the assert is evaluated as part of compile-time constant evaluation e1 would be evaluated only if e1 is a potentially constant expression. If e1 is not a potentially constant expression the assert would be evaluated at runtime when the const value is instantiated.

There are a lot of immutable classes that should/could be const but they are not because of asserts in intializer list.

[eernstg]

Decided at language meeting Nov 11: We always evaluate const asserts.

[eernstg]

Changes to pkg/compiler, pkg/dev_compiler, pkg/front_end, pkg/vm were landed as dart-lang/sdk@e82ca2f.

@scheglov, a couple of experiments seem to confirm that the analyzer always evaluates assertions that occur in the initializer list of a const constructor k when we encounter a constant object expression that invokes k. Is it correct that this work has been completed? If that is true then I believe we can close this issue.

[scheglov]

@eernstg Yes, the analyzer always evaluates the condition of AssertInitializer as a part of evaluating a const constructor invocation. It does not matter whether run-time assertions enabled in the Dart VM that executes the analyzer.

[eernstg]

Thx!