Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Do-notation desugaring to monadic Bind #455

Closed
wants to merge 36 commits into from

Conversation

osavaryb
Copy link
Contributor

@osavaryb osavaryb commented Dec 9, 2019

Feature review.

"do { Expression}" allows for monadic bind "<-" to appear.
"x <- e1; e2" desugars to "Bind(e1, x => e2)"
"P x <- e1; e2" desugars to "match e1
P x => e2"

Both MonadicBindExpr and DoNotationExpr are ConcreteSyntaxExpression (i.e. they disappear after the resolving phase)

Olivier Savary Belanger added 17 commits December 2, 2019 14:51
…it makes sense to switch to a do-notation flag for expression instead
…on, but resolver throw error if they are find outside of do-notation. Still need to fill in the resolving of MonadicBindExpr
…n calc statement (otherwise BinaryExpr may captures the op
@robin-aws
Copy link
Member

AWS CodeBuild CI Report

  • CodeBuild project: Dafny
  • Commit ID: 8b6e83b
  • Result: FAILED
  • Build Logs (available for 30 days)

Powered by github-codebuild-logs, available on the AWS Serverless Application Repository

@robin-aws
Copy link
Member

AWS CodeBuild CI Report

  • CodeBuild project: Dafny
  • Commit ID: e219a5f
  • Result: FAILED
  • Build Logs (available for 30 days)

Powered by github-codebuild-logs, available on the AWS Serverless Application Repository

@robin-aws
Copy link
Member

AWS CodeBuild CI Report

  • CodeBuild project: Dafny
  • Commit ID: eb137ff
  • Result: FAILED
  • Build Logs (available for 30 days)

Powered by github-codebuild-logs, available on the AWS Serverless Application Repository

@osavaryb
Copy link
Contributor Author

Some of the test cases are failing at the moment due to "do" being used as a function/method name (some of these fails at the use of the function rather than at declaration). I could try to make it always fail at declaration, or try to fix the parser so that it doesn't get tripped by non-blocks' "do"

@RustanLeino
Copy link
Collaborator

Thanks for the initiative to add monadic binds to Dafny. I would like to see these, but think they should be more analogous to the :- support that was added to Dafny this year. In particular:

  • I don't see a need for a do block to go around these monadic binds.
  • The introduction of the variables in the LHS of the construction should use var, like let expressions.
  • Instead of a new <- operator for binding, use :-.
  • Resolve the Bind function as a member of the source expression's type.

So, an expression (which can occur anywhere an expression can occur, not just inside some do block)

var x :- E; Body

where x is a bound variable and E and Body are expressions, has the meaning

E.Bind(x => Body)

To distinguish this construct from the similar :- construct in Dafny today, I propose that the resolve look at the type of E to see:

  • Does it have a Bind member? If so, treat as monadic bind.
  • Does it have an IsFailure member? If so, treat as the error-result mechanism.

If neither or both applies, some error should be generated. If it cannot be determined from what's currently known about the type of E during this stage of type inference/checking, then I think an error should be produced.

The resolution of the types for the parameters passed to Bind, IsFailure, PropagateFailure, and Extract can be handled after desugaring according to the two bullets above. However, we can get better error messages if we don't just rely on the desugaring. I have found some of the current error-result error messages unclear, and I would rather that we do more checking before doing the syntactic rewrite.

I'm suggesting the monadic-bind rewrite use just =>, which gives a total arrow. If we ever have a need for a partial arrow (or even a read-effects arrow), then we'd have to invent more syntax.

Olivier Savary Belanger added 2 commits December 13, 2019 15:12
@robin-aws
Copy link
Member

AWS CodeBuild CI Report

  • CodeBuild project: Dafny
  • Commit ID: 38f3c99
  • Result: FAILED
  • Build Logs (available for 30 days)

Powered by github-codebuild-logs, available on the AWS Serverless Application Repository

@robin-aws
Copy link
Member

AWS CodeBuild CI Report

  • CodeBuild project: Dafny
  • Commit ID: 4de17f6
  • Result: SUCCEEDED
  • Build Logs (available for 30 days)

Powered by github-codebuild-logs, available on the AWS Serverless Application Repository

@robin-aws
Copy link
Member

Some thoughts on @RustanLeino's comments.

Rather than introduce another PartialBind method, could we make the signature of Bind always accept a partial function, and just expect that the precondition will often be requires true?

  • Split Bind up into several functions, akin to the IsFailure, PropagateFailure, and Extract functions that the "exception handling" :- looks for.

This would reduce the power of Monads quite a lot so I don't think it would be a great solution. I can't think of the right set of functions to support classic List Monad, for example, which applies the bound function to each element in the list.

  • Inline Bind at the usage site of :-, if the definition of Bind is statically known.

This strikes me as fragile, since refactoring in what should be a semantically-equivalent way would break verification very easily.

I'm also starting to think that we shouldn't try to retcon the existing :- syntax as monadic, as tempting as it is. It's a pretty fundamental problem that the block following a :- statement can already be more powerful than the traditional argument to Bind, since it can read and modify the heap. One possibility would be to represent such blocks as already implementing a monadic type, similar to the IO or State type classes from Haskell. For that to work you'd really need monad transformers, though, so that you could stack an additional monad like Option on top of a class method. We'd likely need some more syntax help for mixing monads and "methods" like that.

On a similar subject, will we end up missing a lot of the appeal of monads if we can't express the monad concept as a trait or something similar, rather than just a baked-in implicit interface? It means we can't have user code that implements monad transformers, for example.

@RustanLeino
Copy link
Collaborator

There are many good comments above and in the discussions linked from the above. Syntactic issues aside, I think we first need to find a good way to deal with the partial expressions that require preconditions above. Should we:

  • Allow users to provide partial functions as either named functions or as lambda expressions with explicit requires clauses? If so, then we can support both Bind and PartialBind or, as @robin-aws suggested, make Bind always be partial.
  • Allow users to provide partial functions and, as @samuelgruetter mentioned above, invent an improved syntax for explicitly giving them in the sugared monadic form?
  • Allow users to provide partial functions and, as @samuelgruetter dream of above, automatically infer these preconditions?
  • Disallow partial functions altogether?

I think we need to resolve this question to go on. Overall, the inclusion of these monadic constructs seems a bit premature at this time. Do we have any interesting use cases that someone would actually like to write? Then, we could let those inform the design. Alternatively, we could go for a restrictive design (I suggest the one in my first comment in the discussion above), so that we can play with it, and then hope to figure out where to go from there?

Opinions?

…n (which get forwarded to the desugared lambda-expression). Rerun coco, add a test case (from Rustan) in MonadicBind
@robin-aws
Copy link
Member

AWS CodeBuild CI Report

  • CodeBuild project: Dafny
  • Commit ID: ca5671e
  • Result: FAILED
  • Build Logs (available for 30 days)

Powered by github-codebuild-logs, available on the AWS Serverless Application Repository

@robin-aws
Copy link
Member

AWS CodeBuild CI Report

  • CodeBuild project: Dafny
  • Commit ID: d798a77
  • Result: SUCCEEDED
  • Build Logs (available for 30 days)

Powered by github-codebuild-logs, available on the AWS Serverless Application Repository

Copy link
Collaborator

@RustanLeino RustanLeino left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I mention my hesitation about the no-LHS version of :-, but maybe that can be resolved later.
All other comments pertain to code style. Please update the settings in your IDE accordingly.

Source/Dafny/Dafny.atg Outdated Show resolved Hide resolved
Source/Dafny/Dafny.atg Outdated Show resolved Hide resolved
Source/Dafny/Dafny.atg Outdated Show resolved Hide resolved
Source/Dafny/Dafny.atg Outdated Show resolved Hide resolved
Source/Dafny/Dafny.atg Outdated Show resolved Hide resolved
Source/Dafny/Resolver.cs Show resolved Hide resolved
Source/Dafny/Resolver.cs Outdated Show resolved Hide resolved
Test/monadic/MonadicBind.dfy Outdated Show resolved Hide resolved
Test/monadic/MonadicBind.dfy Show resolved Hide resolved
@@ -0,0 +1,77 @@
// RUN: %dafny /compile:0 "%s" > "%t"
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I would like to see more tests: some that don't use specifications, some that use reads.
Also, since these are expected to verify, there might as well be some compiling tests (so change to /compile:3 and add a Main).

@robin-aws
Copy link
Member

AWS CodeBuild CI Report

  • CodeBuild project: Dafny
  • Commit ID: 9839e71
  • Result: SUCCEEDED
  • Build Logs (available for 30 days)

Powered by github-codebuild-logs, available on the AWS Serverless Application Repository

@robin-aws
Copy link
Member

AWS CodeBuild CI Report

  • CodeBuild project: Dafny
  • Commit ID: 93662b6
  • Result: SUCCEEDED
  • Build Logs (available for 30 days)

Powered by github-codebuild-logs, available on the AWS Serverless Application Repository

@robin-aws
Copy link
Member

AWS CodeBuild CI Report

  • CodeBuild project: Dafny
  • Commit ID: b9d5af5
  • Result: FAILED
  • Build Logs (available for 30 days)

Powered by github-codebuild-logs, available on the AWS Serverless Application Repository

@robin-aws
Copy link
Member

AWS CodeBuild CI Report

  • CodeBuild project: Dafny
  • Commit ID: 23fe3f6
  • Result: SUCCEEDED
  • Build Logs (available for 30 days)

Powered by github-codebuild-logs, available on the AWS Serverless Application Repository

@robin-aws
Copy link
Member

AWS CodeBuild CI Report

  • CodeBuild project: Dafny
  • Commit ID: c7215ee
  • Result: SUCCEEDED
  • Build Logs (available for 30 days)

Powered by github-codebuild-logs, available on the AWS Serverless Application Repository

@robin-aws robin-aws self-requested a review February 19, 2020 19:55
@robin-aws
Copy link
Member

I still have reservations about edge cases in ambiguity between LetOrFail :- and Monadic :-. I'd like to have a closer look and see if I can break this before it's merged. :)


class Cell {
var data: int
}
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

What is class Cell used for?

case Cons(x, _) => Some(x)
}

function method G(list: List): Option {
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

You're using List and Option without giving a type argument. I'd also test it with List<A> and Option<A>.

var cdr :- Tail(list);
var cddr :- Tail(cdr);
Head(cddr)
}
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

All the examples involving generics go from a List<T> to List<T>, where T remains the same. I'd also test an example where the function you pass to Bind goes from List<T> to List<U>, where T and U are different, and also one where it goes from Foo<T> to Bar<U>, just to see how this interacts with type inference.

{
if this == None then None else f(get)
}
}
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

You're intending to implement quite a generic feature, so it would be nice to not only test it for the Option monad, but also for some other monad, maybe for the State monad? Or what other monads are people intending to use in Dafny?

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

+1 - this also needs to test the case where a type defines both Bind and IsFailure/PropagateFailure/Extract, since the syntax is ambiguous there

@RustanLeino RustanLeino added the status: implemented Candidate feature available for experimentation label Jun 26, 2020
@RustanLeino
Copy link
Collaborator

Upon further consideration, we will not include this feature in the language at this time. There are some issues to be resolved around how to write verifying code with the functions that are being generated.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
status: implemented Candidate feature available for experimentation
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants