dart-lang / language

Design of the Dart language
Other
2.66k stars 204 forks source link

Add parallel keyword identifier which is opposite to await #979

Open Rockvole opened 4 years ago

Rockvole commented 4 years ago

I find the use of the await keyword to be the opposite of what I expect the language to provide. Having a keyword which is mandatory when calling async functions in a parallel manner would make it clearer how the code differs at first glance.

Examples of the parallel keyword identifier : acont apara

I would even suggest that await is the optional keyword since it indicates that this async function call will perform as expected from a non async function call.

Points :

  1. A mandatory parallel keyword would show at a glance that the function being called is async without needing to find the function definition.
  2. Would make conversion from other languages easier. Most of the gotchas I found when converting from Java were due to forgetting that a function is now async in dart (often forced due to external libraries being used).
  3. await keyword is redundant since it shows that an async function is being called and identifies it as functioning the same as the non-async function calls?
Rockvole commented 4 years ago

Here is an example of how this makes life harder for developers.

My example code :

import 'package:print_lib/print_lib.dart' show Pl;
Pl.printA();
Pl.printB();
Pl.printC();
Pl.printD();

This outputs :

ABCD

Now, if the external library is changed so that printC() becomes an async and has a pause at the start, now my code silently completely changes to :

ABD *long pause* C

If there was a mandatory parallel keyword, then I would immediately see that the library had changed and I need to decide how I want my code to behave - do I want synchronous or asynchronous, e.g. :

await Pl.printC(); 

to have everything continue as before.

That's a trivial example, but what if we have a bunch of routines which are sending network requests to different locations and then writing entries in a database with timestamps? Entries are expected to be in a particular order. In test everything works fine because the tests return in the correct order on the local network, then in production sometimes something goes wrong because of network contention, or maybe only in some countries. Its easy to see how you could spend days debugging problems like that because the compiler is not helping you spot that code has changed.

A method could be changed to async in a bunch of scenarios :

  1. A library call was changed to async.
  2. Someone on another team converted a call to async
  3. You refactor your code and decide async is more efficient, but forget to update all the procedure calls.
  4. A library upstream was converted to async and you had to change your own procedures to async and you forgot to add await where you called your own procedure calls.
  5. You are converting hundreds of lines of code from a language which doesn't have async/await and you miss the await keywords all over the place leading to a lot of wasted time debugging.

Mostly, this is just about saving time for developers by making changes more clearly noticeable.

bwilkerson commented 4 years ago

You might find the lint rule unawaited_futures to be helpful. It will flag any use of a function or method that returns a Future that does not have an await, which helps catch situations like the ones you're describing.

Rockvole commented 4 years ago

@bwilkerson - I see I can get the functionality I want :

  1. Add lint rule in analysis_options.yaml

    linter: rules:

    • unawaited_futures
  2. Add to the top of my 200 files :

    import 'package:pedantic/pedantic.dart';

  3. Get rid of all the unwanted lint rules by wrapping my 44 unawaited futures with :

    unawaited()

It does seem quite involved though, and doesn't feel like the language is helping me to easily discover bugs. I suppose I am just being pedantic ;P

bwilkerson commented 4 years ago

I should have been more clear. I'm not proposing that we don't need/want language support (I haven't actually formed an opinion about that one way or the other). What I was trying to do was to point you to a work-around that might save you some time in the absence of such a feature. If you don't believe that it's worth the effort then feel free to not use it.

Also, if there are valid cases that the lint shouldn't be flagging, please let us know (preferably in the linter package's repository) so that we can investigate the feasibility of improving the lint.

  1. Add to the top of my 200 files :
  2. Get rid of all the unwanted lint rules by wrapping my 44 unawaited futures with :

As long as we're being pedantic :-), if there are only 44 unawaited futures (and they're all valid, rather than errors that should be fixed), then they can't appear in more than 44 separate files. That means that you need to add the import to a maximum of 44 libraries (the import is only needed in order to bring unawaited into the name space).

Rockvole commented 4 years ago

No, the lint feature is useful and I will keep it turned on. It would have saved me a great deal of time on my Java code -> Dart code conversion project if I had known about it. I have wasted more time on this project finding bugs due to missing awaits than anything else.

To me, that feels like something that should be baked into the language.

lrhn commented 4 years ago

The keyword should obviously be apart!. 😁

But seriously, there is a lot of history in the language design around async/await.

We originally defined futures before having async and await. You had to call .then to handle the result of the future, and futures were just values like any other. Then we added async/await which allowed you to write more normal-looking code, but which was fundamentally just syntactic sugar for calling then anyway. Futures are still just values like any other. And you can ignore values (and sometimes you do want to ignore futures). The syntax was heavily inspired by similar features in other languages.

If we had designed async/await into the language from scratch, with the experience we have now, we might have done it differently. Doing something differently now will be a breaking change to a lot of code. It's not completely impossible, though, given language versioning. It just have to be worth the cost, both doing the change and retraining users to understand and use the new behavior.

One option could be to change async functions so that any expression which has a type implementing Future<T> or being FutureOr<T> is automatically awaited, and you have to make an effort to not do so - say by prefixing/suffixing it with something. Let's say apart for now.

That is:

Future<int> foo(int n) async {
  if (n < 2) return 1;
  return foo(n - 1) + foo(n - 2);
}

would automatically be treated as the current:

Future<int> foo(int n) async {
  if (n < 2) return 1;
  return await foo(n - 1) + await foo(n - 2);
}

In order to not implicitly await, you'd have to write something like:

Future<int> foo(int n) async {
  if (n < 2) return 1;
  var values = Future.wait([apart foo(n - 1), apart foo(n - 2)]);
  return values[0] + values[1];
}

And if you want to ignore a future, you'd have to avoid it having type Future<Something>. We'd probably just add:

extension FutureExtension<T> on Future<T> {
  void ignore() {}
}

so you can just do asyncOperation().ignore();.

It's not even a completely bad idea, and migration is simple: Any place the code already has an await that would also be added implicitly, it's removed. Any place there is an await and the type isn't a Future/FutureOr type, it's a top type or Object, and we append as FutureOr<Object?> and remove the await. Any place the code would introduce an await that isn't there now, and the value is used, we insert an apart. If the value is not used, we append an .ignore(). That should be entirely automatable.

OK, the apart name doesn't work. ☹️

It would be a major change to the language. Not something we'd do without serious considerations for the alternatives and costs.

Rockvole commented 4 years ago

I don't think the apart keyword is as clear.

In computing terms, the keywords indicate to me : apart - short for partition or a piece of something. Also, the part keyword is already used in imports. aconc - short for concurrent apara - short for parallel

Perhaps putting in a warning saying the apara keyword should be used and leave that for a couple of years. Or (later) add a flag to the compiler to throw an error when apara is not used and have that be a default flag used by IDEs so that command line compilation still works on servers.

Making unawaited_futures a default on new projects would help quite a bit, although I find unawaited() to be unaesthetic.

Rockvole commented 4 years ago

I thought of an alternative parallel keyword. It may not accurately reflect whats going on but it feels like a matched pair with await acont - short for continue.

jamesderlin commented 4 years ago

One option could be to change async functions so that any expression which has a type implementing Future<T> or being FutureOr<T> is automatically awaited,

Currently await allows readers to see where execution might yield. Knowing that is important because yielding means that any state that the function already checked might no longer be valid by the time execution resumes. It's already hard catching such mistakes with explicit await keywords, but without them it'd become drastically worse since asynchronous code would be visually indistinguishable from synchronous code.