dart-lang / language

Design of the Dart language
Other
2.65k stars 202 forks source link

Proposal: guards #416

Open yjbanov opened 5 years ago

yjbanov commented 5 years ago

This proposes a language feature that allows guarding parts of programs based on conditions such that the analyzer and the compiler can statically verify that those parts are accessed correctly.

This aims to provide a solution for #415.

Declaration

New keywords are introduced in Dart: guard and requires.

The guard keyword is used as a top-level or static declaration of a const or a runtime guard, or as an instance guard of a class. Guards are initialized from boolean expressions:

const guard debugMode(const_bool_expression);
guard inPaintPhase(false);

class Foo {
  static const guard isDebugMode(const_boolean_expression);
  static guard isInPaintPhase(false);
  guard enabled = true;
}

A guard may be in two states:

const guards are evaluated eagerly before dead-code elimination. Variable guards can change their state dynamically.

Usage

To change the state of a runtime guard, call its set method, e.g. inPaintPhase.set(true).

Guards are used to modify functions, methods, getters, setters, and typedefs (not sure if it's worth guarding variable access) using a requires clause:

typedef PaintingContextCallback = void Function(PaintingContext context, Offset offset)
    requires isInPaintPhase;

class ContainerRenderObjectMixin {
  void debugValidateChild(RenderObject child) requires isDebugMode {
    // validation logic
  }
}

abstract class RenderObject {
  void paint(PaintingContext context, Offset offset) requires isInPaintPhase;
}

Members with requires clauses are referred to as "guarded". Guarded members may only be accessed if it can be statically proven that the guard is on. Such proofs are provided by promoting if blocks, requires clauses, and data flow analysis. In the following example, the if block is promoted to "isDebugMode is on" and therefore it is safe to call debugValidateChild:

if (isDebugMode && child != null) {
  debugValidateChild(child);
}

The following example shows how the body of debugValidateChild is promoted allowing it to call debugValidateIsEmpty safely without extra checks.

void debugValidateIsEmpty(RenderObject object) requires isDebugMode {
  assert(object._children.isEmpty);
}

void debugValidateChild(RenderObject child) requires isDebugMode {
  debugValidateIsEmpty(child);
}

The following example shows how the remainder of a function block is promoted using data flow analysis:

void drawFrame() {
  build();
  layout();
  isInPaintPhase.set(true);  // code following this line is promoted
  paint();  // safe to call without checking due to previous line
  isInPaintPhase.set(false);
}

Runtime checks

Because runtime guards may change state dynamically we need to ensure guards are not disabled in the middle of a guarded block. This is done by locking the guard when a guarded block is entered and releasing it when the outermost guarded block exits. While locked, a guard's state may not change.

Dynamic dispatch

requires clause is considered part of the method's name. Dynamic dispatch may never reach a guarded method.

Overrides

requires clauses use the same rules as parameters. Namely, method overrides are allowed to relax requirements, but never tighten them. For example:

class Button {
  guard enabled(true);

  void click() requires enabled {
  }
}

class AlwaysClickableButton extends Button {
  @override
  void click() {  // ok to not require enabled in the override
  }
}

It is legal to call click() on AlwaysClickableButton without the enabled guard. However, it is not legal to call it on Button even when the runtime type is AlwaysClickableButton.

Asynchrony

Guards ignore asynchrony. A microtask scheduled from within a guarded function is not automatically guarded. The body of the microtask must check the guard independently. This applies to await, timers, I/O, etc. In particular, await demotes a previously promoted block back to unguarded. Examples:

void click() requires enabled {
  foo(); // guarded
  await bar();  // guarded
  baz();  // unguarded
}

void foo() required isInPaintPhase {
  bar();  // guarded
  scheduleMicrotask(() {
    baz();  // unguarded
  });
  qux(); // guarded
}

Future extensions

This proposal limits guards to bool only. In the future, we might want to support enum.

rrousselGit commented 5 years ago

~What about blocks with more than one instruction for runtime guards?~

Because runtime guards may change state dynamically we need to ensure guards are not disabled in the middle of a guarded block. This is done by locking the guard when a guarded block is entered and releasing it when the outermost guarded block exits. While locked, a guard's state may not change.

var button = Button();

if (button.enabled) {
  button.enabled.set(false);
  button.click(child); // that's not good
}
ds84182 commented 5 years ago

Absolutely brilliant! I love it!

Have you thought about the ability to use enums as guards? It would allow for mutual exclusion of guard types, and if extended to arbitrary const values (in the far future), it would allow for some interesting amounts of expressibility (a library defined implementation of Java's throws construct? ;).

This would make asserting states in a state machine a bit easier. Say you have two stages/phases: layout and paint. With enums, you could describe a method that is only available during layout, or a method that is only available during paint. If you know you're in the middle of layout, you definitely aren't in the middle of paint, for example.

yjbanov commented 5 years ago

@ds84182 In the "Future extensions" section I propose exactly that, although I didn't explain the motivation behind adding enum support. You are right, state machines are a good motivator here. We have lots in Flutter, such as AnimatedCrossFade, AnimatedSize, BuildOwner, PersistedSurface (even though they don't call themselves state machines).

@jonahwilliams pointed out that this enters the dependent type territory. So there may be lots of opportunities, although we should be careful about not over-complicating the language.

lrhn commented 5 years ago

This description seems to include mutex support (single write, multiple read).

You can set a guard variable to false to disable readers, and successfully read a true result to disable writing until the guarded block exits. Multiple readers can check that the value is true, and it cannot be set to false until all of them are done. Since such a guard can guard an async operation, the read-lock can persist arbitrarily long.

This suggests some of the implementation complexities around the suggested feature. If we make the guard simply a boolean variable, then the syntax would be:

static guard foo = false;

if (foo) {  // get read lock if possible.

}  // Release read lock. This cannot fail.

foo = false;  // throws if any read lock.
foo = true;  // always possible

(unless we have negative guards, requires !foo, and promote on if (!foo), then that will block setting to true as well).

There is no synchronization. There is no way to block until the guard becomes available. There is no way to check whether assigning false will work except trying and catching the error. That might be an issue.

The example:

  isInPaintPhase.set(true);  // code following this line is promoted
  paint();  // safe to call without checking due to previous line
  isInPaintPhase.set(false);

suffers from this problem. If paint() reaches any asynchronous code, then some other code might enter an if (isInPaintPhase) block during its execution and prevent the isInPaintPhase.set(false);. Then this code will throw at run-time, the very thing the guard was introduced to prevent. I'm not sure throwing is really the right thing to do when you try to set the value to false. Maybe setting it to false must be an async operation, which completes when all current readers are done. That is, an (async) blocking operation.

Using data-flow based promotion and mutex is clever, but perhaps too clever. How about introducing an actual guarded block syntax:

if guard foo {
  // read lock on foo.
}

Then you won't get into discussion about how far the scope of the test is.

I doubt putting the requirements on function types is a good idea. That will complicate function types (I'm not sure what the subtype relation would be, but I guess it's possible that int Function(int) is a subtype of int Function(int) requires foo since the former can be used everywhere the latter can. It would mean that you can't add requirements on a method in a subclass. And you'll be able to require more than one guard: requires foo, bar, baz because that allows you to implement multiple function types with each their own requirement.

If we use enums (constant values) for guards, then I guess it becomes requires foo == Foo.one, bar == Bar.alpha. It's obviously an error to end up with requires foo == Foo.one, foo == Foo.two (or maybe it's allowed, it's just unsatisfiable, at least unless identical(Foo.one, Foo.two)).

I guess one use-case is when the guard is constant. Then the compiled can remove any guarded code not enabled by the constant value. Since the function invocations are guarded themselves, we can definitely promise that they are not called when the guard can never become true (or false, if the guard is requires !foo).

yjbanov commented 5 years ago

@lrhn

Since such a guard can guard an async operation, the read-lock can persist arbitrarily long.

I don't think guards should support async operations out-of-the-box. I think it's OK to leave that to the developer. IOW, if a guarded function creates a microtask, the microtask is executed on its own. If the microtask callback requires a certain guard, it would have to check for it separately. This also means that await demotes the following statements. That is OK.

There is no synchronization. There is no way to block until the guard becomes available. There is no way to check whether assigning false will work except trying and catching the error. That might be an issue.

I think these would be interesting potential future extensions to this feature, but I think these can be left out for the MVP.

How about introducing an actual guarded block syntax

I was thinking that guards should be usable inside boolean expressions so that they compose with normal if, while, and switch statements, with a goal to keep the language simple and to capitalize on concepts that users are already familiar with. Note that in the debugValidateChild example a guard is used in tandem with a normal boolean expression (isDebugMode is a guard, while child != null is plain old boolean expression):

if (isDebugMode && child != null) {
  debugValidateChild(child);
}

I doubt putting the requirements on function types is a good idea. That will complicate function types.

It definitely requires some new subtyping rules, but I think this proposal would lose a lot of value if this was left out. Yes, tree-shaking using constant guards is one huge use-case for this. Another big use-case is elimination of reentrant locking of runtime guards (i.e. a guarded function does not need to re-lock when calling another guarded function).

mdebbar commented 5 years ago

If we are going to mark functions with requires foo, we might as well introduce mutates foo too (but it would be automatically inferred. The mutates foo annotation could eliminate the need to throw at runtime if the guard was set to false:

bar() {
  // bar() is inferred to be `mutates foo`.
  foo.set(false);
}

if (foo) {
  bar(); // Static error: Can't mutate `foo` inside a block guarded by `foo`.
}

In the same way, requires foo can also be inferred.

Both mutates foo and requires foo propagate upwards to all caller functions:

bar() {
  // if baz() requires foo, then bar() should also require foo.
  baz();
}

bar() {
  // If baz() mutates foo, then bar() also mutates foo.
  baz();
}
yjbanov commented 5 years ago

@mdebbar I think mutates foo won't work because it's too "contagious". Guarded functions should be able to freely call arbitrary function, otherwise they are not very useful. However, if we want to rely on mutates to signal that a function changes the state of a guard that information has to propagate upwards through the entire call stack. This would eliminate the ability for a guarded function to call forEach, for example, and that's too restrictive.

The other issue is that in Dart, by design, type inference does not leak types into the function/method signature (whole world analysis is a different story). This simplifies code analysis (you don't need to look at function bodies), but also makes subtyping saner. For example, what if in your example, bar was an @override? How would type inference communicate mutates foo up the type hierarchy? Or would you disallow mutating guards in method overrides?

An alternative is to make statically inferred guard mutation best effort. However, that makes it very brittle. Seemingly benign code changes would break inference. That would make the system too brittle to rely on.

yjbanov commented 5 years ago

Update: added "Asynchrony" section in the issue description.

munificent commented 5 years ago

Very interesting! This looks like typestate. Is that deliberate?

yjbanov commented 5 years ago

@munificent

This looks like typestate. Is that deliberate?

No, first time I hear about typestate. Does look similar.

Sunbreak commented 3 years ago

After migerated to NNBD, we've got a lot boilerplate code to guard usage of nullable field in method

class dart {
  int? _index;

  void foo() {
    if (_index == null) return;

    final int index = _index;
    print('index %index');
  }
}

Could we get Swift-like guard

private var index: Int?;

func foo() {
  guard let index = self.index else { return }

  print("index \(index)")
}

Or Koltin-like Elvis operator ?:

private var index: Int? = null;

fun foo() {
  val index = index ?: return

  println("index $index")
}
lrhn commented 3 years ago

If we allow "statement expressions" (#1211, expressions which can contain general statements, including control flow) then this would just be:

  var index = self.index ?? {{return;}};  // 
alsoLut commented 1 year ago

Typescript has a similar feature called type guard

https://www.typescriptlang.org/docs/handbook/2/narrowing.html#using-type-predicates