Giving "const asserts" a whirl

[sethladd]

We should be able to experiment with "const asserts". The Dart team would love our feedback.

For analyzer: dart-lang/sdk#27142 (comment)

tldr: "enableAssertInitializer is included in the language section of the analysis options file"

For VM: dart-lang/sdk#24841 (comment)

tldr: "use flag --assert-initializer."

Thanks!

[sethladd]

cc @Hixie @abarth

[sethladd]

cc @floitschG

[abarth]

This issue is in the "top customer requests" milestone. Which customer has requested this?

[sethladd]

Mixed semantics for the Milestone. We wanted to give the Dart team feedback "ASAP", so this was put into the "sooner than later" milestone.

To help make this milestone's semantics more clear, I've moved it out. But, if we can, let's try this soonish and send feedback over.

[abarth]

I think the analyzer support might have a bug, which is making me unsure of the evaluation:

The analyzer allows this code:

  const MultiChildRenderObjectWidget({ Key key, this.children })
    : assert(children != null), assert(!children.any((Widget child) => child == null)), super(key: key);

But I would have expected it to be an error because https://gist.github.com/lrhn/45bdae95e017ebbf7a84bfbe26dffcae says "the expression in the assert must be a potentially compile-time constant expression". Is this really supported? If so, yay! If not, it will be easier to evaluate after the bug is fixed.

[abarth]

abarth@983be4b is my experiment so far. The analyzer seems happy with all those changes, but I'm worried that not all the expressions in the asserts are potentially compile-time constant expression.

[Hixie]

Does it run? I mean, if they're not compile-time expressions you'll presumably find out pretty quick if you try to violate the constraint at compile time and the compiler allows it.

Ideally the analyzer itself would evaluate the constants to see if they're true.

[sethladd]

cc @bwilkerson for analyzer question (#5865 (comment))

[abarth]

Does it run?

I haven't hacked up the engine yet to turn on the flag.

[lrhn]

Any progress?

[abarth]

@lrhn I never got an answer to my question above. Would you expect this snippet of code to be allowed:

const MultiChildRenderObjectWidget({ Key key, this.children })
    : assert(children != null), assert(!children.any((Widget child) => child == null)), super(key: key);

The type of this.children is List<Widget>. Specifically, I'm surprised that I can call the any function inside an assert in a const constructor.

[floitschG]

The any call is definitely not a compile-time constant, and the VM should complain when running checked mode.

The current implementation of the analyzer doesn't evaluate the expressions, yet.

[abarth]

The current implementation of the analyzer doesn't evaluate the expressions, yet.

In that case, I'm not able to evaluate const asserts yet because the implementation isn't of sufficient quality to tell me whether a particular assert is allowed.

[lrhn]

On Thu, Oct 20, 2016 at 9:57 PM, Adam Barth [email protected]
wrote:

@lrhn https://github.com/lrhn I never got an answer to my question
above. Would you expect this snippet of code to be allowed:

const MultiChildRenderObjectWidget({ Key key, this.children }) : assert(children != null), assert(!children.any((Widget child) => child == null)), super(key: key);

The type of this.children is List. Specifically, I'm surprised
that I can call the any function inside an assert in a const constructor.

For the first assert, I'm not actually sure. The spec is ... indecisive.

A *potential constant expression *is one that is compile-time constant "if
all formal parameters of e’s immediately enclosing constant constructor
were treated as compile-time constants that were guaranteed to evaluate to
an integer, boolean or string value as required by their immediately
enclosing superexpression."
However children == null is only a compile-time constant expression if
children evaluates to a numeric, string, or boolean value or to null,
which I guess it doesn't.

Something can definitely slip through that spec hole: it's potentially
constant if it would be constant if children is int (which is true), but
children isn't int, so it's not actually constant, but that's not required
anywhere.

In any case, we should probably change the spec to only require one side of
c1 == c2 to be null to allow the expression as compile-time constant, so
the expression is both potentially constant and actually constant.

Until then, you can use:

...: assert(!identical(children, null)), ...

which is a compile-time constant expression independent of children since
identical doesn't require specific types for the operands like == does.

The second assert is right out. It's not a potentially constant expression.
(The VM has some holes in its constant constructor evaluation that allows
non-constant computations to leak through, but that's a bug (
dart-lang/sdk#392) and there are probably more
cases like it).

/L

Lasse R.H. Nielsen - [email protected]
'Faith without judgement merely degrades the spirit divine'
Google Denmark ApS - Frederiksborggade 20B, 1 sal - 1360 København K

• Denmark - CVR nr. 28 86 69 84

[abarth]

Please let me know when there's some program I can run over my code to tell me whether my const asserts are allowed. Until then, I'm not able to evaluate whether they're useful because I don't know what is allowed.

[sethladd]

I checked into the Dart SDK's issues. Sounds like there's a few things we can try?

From dart-lang/sdk#27142

"As of https://codereview.chromium.org/2282233002/ the analyzer now accepts the new syntax if the flag enableAssertInitializer is included in the language section of the analysis options file."

From dart-lang/sdk#24841 (comment)

"I have landed an experimental implementation of assert in initializer lists, including const initializer lists, for the VM. It is guarded behind a flag --assert-initializer."

Are those sufficient to try this feature?

@abarth by "are allowed" above, did you mean "don't throw exceptions when evaluated" ?

[sethladd]

Chatted offline with @abarth . We're going to follow up on the Dart SDK's issue re: the analyzer's implementation of this feature. dart-lang/sdk#27142

[sethladd]

Update for those following along. The analyzer should now check that "the expressions in an assert in the initializer list of a const constructor are all potentially const expressions".

We are waiting for a Dart SDK roll (#6840), in order to test this new feature again. Stand by...

[Hixie]

Will the analyzer actually check the assertions though? Because that's what would really be useful...

[bwilkerson]

No. At the moment it will only check that the expressions are potentially constant expressions.

[sethladd]

@Hixie the VM/compiler should check them at compile time, if I understand correctly. We need to pass a flag to the VM/compiler to enable this, though.

@bwilkerson : is it possible for the analyzer to evaluate const assets at static analysis time?

[pq]

Landed Dart SDK 1.21.0-dev.6.0 w/ 3edf44f . I think you should be good to start experimenting?

[sethladd]

Thanks @pq ! Ball's in our court now.

[sethladd]

OK, did some testing.

The VM appears to evaluate asserts in const constructor initialization lists, at compile time, IF you use --checked --assert-initializer: (you have to use both of the flags)

~/tmp $ cat const_asserts.dart 
class Foo {

  final List children;

  const Foo({ this.children })
    : assert(children != null); //, assert(!children.any((child) => child == null));

}

main() {
  new Foo(children: null);  // no compile-time failure

  const Foo(children: const []);  // no compile-time failure

  const Foo(children: null);  // compile-time failure
}
~/tmp $ dart --checked --assert-initializer const_asserts.dart 
Unhandled exception:
Unhandled exception:
'file:///Users/sethladd/tmp/const_asserts.dart': Failed assertion: line 6 pos 14: 'children != null': is not true.
#0      _AssertionError._doThrowNew (dart:core-patch/errors_patch.dart:33)
#1      _AssertionError._throwNew (dart:core-patch/errors_patch.dart:29)
#2      Foo.Foo (file:///Users/sethladd/tmp/const_asserts.dart:6:14)
#3      _startIsolate.<anonymous closure> (dart:isolate-patch/isolate_patch.dart:261)
#4      _RawReceivePortImpl._handleMessage (dart:isolate-patch/isolate_patch.dart:148)
'file:///Users/sethladd/tmp/const_asserts.dart': error: line 15 pos 9: error while evaluating const constructor
  const Foo(children: null);  // failure
        ^

#0      _startIsolate.<anonymous closure> (dart:isolate-patch/isolate_patch.dart:261)
#1      _RawReceivePortImpl._handleMessage (dart:isolate-patch/isolate_patch.dart:148)

Neat!

The formatter doesn't work, but maybe I'm doing it wrong:

~/tmp $ dartfmt --assert-initializer const_asserts.dart 
Could not find an option named "assert-initializer".

Usage: dartfmt [-n|-w] [files or directories...]

-h, --help                   Shows usage information.
    --version                Shows version information.
-l, --line-length            Wrap lines longer than this.
                             (defaults to "80")

-i, --indent                 Spaces of leading indentation.
                             (defaults to "0")

    --preserve               Selection to preserve, formatted as "start:length".
-n, --dry-run                Show which files would be modified but make no changes.
    --set-exit-if-changed    Return exit code 1 if there are any formatting changes.
-w, --overwrite              Overwrite input files with formatted output.
-m, --machine                Produce machine-readable JSON output.
    --profile                Display profile times after running.
    --follow-links           Follow links to files and directories.
                             If unset, links will be ignored.

-t, --transform              Unused flag for compability with the old formatter.
~/tmp $ dartfmt --version
0.2.13

Tracking at: dart-lang/dart_style#522

The analyzer seems to work, if you turn on the feature in your .analysis_options file:

analyzer:
  language:
    enableAssertInitializer: true

and

~/tmp $ dartanalyzer const_asserts.dart 
Analyzing [const_asserts.dart]...
No issues found

And the analyzer tells the developer if they use a non-const expression in the asserts:

~/tmp $ dartanalyzer const_asserts.dart 
Analyzing [const_asserts.dart]...
[error] Initializer expressions in constant constructors must be constants. (/Users/sethladd/tmp/const_asserts.dart, line 6, col 41)
1 error found.

[munificent]

this new language feature.

Wait wait wait. Just to be clear, this is not a new language feature. It is a proposed language feature experiment. We haven't committed to shipping it as part of the language as far as I know.

[sethladd]

I've updated my comment in #5865 (comment) to reflect that it's experimental and proposed. Thanks for the clarification.

[munificent]

OK, thanks. Just wanted to make sure.

[github-actions]

This thread has been automatically locked since there has not been any recent activity after it was closed. If you are still experiencing a similar issue, please open a new bug, including the output of flutter doctor -v and a minimal reproduction of the issue.