Sdk: Require the first argument to assert() to be a bool

Created on 4 Aug 2017  路  14Comments  路  Source: dart-lang/sdk

The language currently allows the first argument to assert() to be either a bool, or a nullary function that returns a bool when called. In the latter case, it is implicitly invoked:

assert(() {
  return true;
});

That only adds the tiniest bit of syntactic sugar. Without this feature, you could always just call the function yourself:

assert(() {
  return true;
}());

In return for saving you a (), the type inference for assert() isn't as useful as it could be. It can't assume that the first argument should be a bool when performing downwards type inference.

There is also, of course, the implementation complexity and cognitive overhead of this obscure feature. It doesn't carry its weight, so the language team has decided to remove it for Dart 2.0.

In 2.0, the first argument to assert() must be an expression that evaluates to true or false. You can migrate your existing using of the removed feature by adding a () after your function.

We still need to figure out what tasks are required to ship this, but to get started:

  • [x] Update the assert() tests. (https://dart-review.googlesource.com/c/sdk/+/4181)
  • [x] Remove support for it from analyzer.
  • [x] Remove support for it from dart2js.
  • [x] Remove support for it from dartdevc. (https://dart-review.googlesource.com/c/sdk/+/4182)
  • [x] Remove support for it from the front-end.
  • [x] Remove support for it from the VM.
  • [x] Remove uses of it in the core libraries and packages we maintain.
  • [x] Remove uses of it inside google.
  • [x] Announce this publicly and explain the migration path.
  • [x] Update documentation that mentions this (if any).
  • [x] Remove it from the spec. (https://codereview.chromium.org/2974763002)
  • [x] Update downwards type inference in analyzer and front_end to assume the first argument to assert() is a bool.
area-language

Most helpful comment

For google, we're going to want it to be an error sooner, i.e. to prevent slide-back.

we can have it be an error in strong mode. I'm happy to send CL's to help fix internal code for the Dart SDK roll

All 14 comments

Remove support for it from analyzer.

What this probably means in practice is to start issuing a hint relatively soon indicating that this feature is obsolete (with a quick fix to add the parentheses). In 2.0 we can promote the hint to an error.

For google, we're going to want it to be an error sooner, i.e. to prevent slide-back.

For google, we're going to want it to be an error sooner, i.e. to prevent slide-back.

we can have it be an error in strong mode. I'm happy to send CL's to help fix internal code for the Dart SDK roll

Woohoo!

Last I heard, hints will also fail the internal build, so I don't think promoting it sooner will be necessary.

What this probably means in practice is to start issuing a hint relatively soon indicating that this feature is obsolete (with a quick fix to add the parentheses). In 2.0 we can promote the hint to an error.

@bwilkerson Let's get on that sooner rather than later. I'm guessing that we can go back to assuming the type of the assert argument is bool?

Right, @munificent ?

I'm guessing that we can go back to assuming the type of the assert argument is bool?

We aren't going back to assuming the type is a bool. It's allowed, effectively, bool | (bool Function()) for ages. But, yes, we are going forward to only allowing bool.

Support removed from analyzer in https://dart-review.googlesource.com/c/sdk/+/31963. All internal usage is fixed.

Also, I think some flutter testing this week revealed that closures-in-bool has been removed (was never written) in front-end.

Front end gives an error, as does the VM in preview-dart-2. The analyzer also gives an error now in strong mode. I believe that all of the code that we need to/plan to clean up is clean.

Is any work actually remaining here for Dart 2?

I guess that's just a question for dart2js, according to the checkboxes.

dart2js is derived from the CFE in this case, so no action needed on our end. I just marked the checkbox.

I think that covers it.

Was this page helpful?
0 / 5 - 0 ratings