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

Add extended sequencing operators. #1474

Open
wants to merge 1 commit into
base: trunk
from

Conversation

Projects
None yet
3 participants
@paurkedal

paurkedal commented Nov 12, 2017

This PR adds a new class of low precedence operators suitable as statement separators. E.g. a rewriter may set up the following aliases:

  • e1 -; e2 as e1; e2
  • e1 >|; e2 as e1 >|= fun () -> e2
  • e1 >>; e2 as e1 >>= fun () -> e2

which could be used in an Lwt context:

open Lwt.Infix

let rec test l n =
  Lwt_io.printlf "Entering level %d." l >>;
  begin
    if l > 5 || !n > 20 then
      Lwt_io.printl "Sleeping." >>;
      Lwt_unix.sleep (Random.float 1.0)
    else
      n := !n + 1 -;
      test (l + 1) n <&> test (l + 1) n >|;
      n := !n - 1
  end >>;
  Lwt_io.printlf "Leaving level %d." l

I might have gone slightly overboard with the extended_semicolon.ml test, so that provides a more elaborate example.

Syntax

The operators are composed of a semicolon prefixed by one or more operator characters, except for a few constraints due to conflicts with existing syntax. The period is excluded from the operator characters here, since both .. and . may occur before ;; and at least the latter before ;. The >; operator also had to be excluded, since > works as a delimiter in type expressions, and may thus precede both ; and ;;. I'm not sure if this covers everything, so we should give this point some though before merging (with a "-" bullet in the change log).

The precedence level is just above if, as opposed to ;, which is just below. This level was chosen to prevent a prefixed let expression from affecting the grouping of statements under then or else, as discussed to length in #715 and related threads. I believe the -; operator suggested above provides a more well-behaved alternative to ; for those who are okay with the heavier notation.

Related

@lpw25

This comment has been minimized.

Show comment
Hide comment
@lpw25

lpw25 Nov 12, 2017

Contributor

I'm not a fan of using ordinary operators for extensions. You could use #508 instead, or at least explicitly reserve some operators for use with extensions (IIRC this is how ## works). The choice of precedence level seems a bad idea, much more likely to cause issues than just using the precedence level of ;.

Contributor

lpw25 commented Nov 12, 2017

I'm not a fan of using ordinary operators for extensions. You could use #508 instead, or at least explicitly reserve some operators for use with extensions (IIRC this is how ## works). The choice of precedence level seems a bad idea, much more likely to cause issues than just using the precedence level of ;.

@paurkedal

This comment has been minimized.

Show comment
Hide comment
@paurkedal

paurkedal Nov 12, 2017

I'm not a fan of using ordinary operators for extensions. You could use #508 instead, or at least explicitly reserve some operators for use with extensions (IIRC this is how ## works). The choice of precedence level seems a bad idea, much more likely to cause issues than just using the precedence level of ;.

It may be a good idea to reserve it for extensions; I added a check in env.ml, but took it out for no good reason. It's a matter of taste, but I think ;%ext makes the code look messy, so one of the motivations for this PR is to provide an alternative which looks more like a regular operator.

I disagree about the precedence level, since the level of ; causes inconsistent grouping when mixing conditions with let, and providing an opt-out from those issues is the second motivation for this PR.

Added: For those who haven't followed the "safe-syntax" discussion, the main issue as as follows. Consider the code

if ... then
   ...
else
   stmt1 ();
stmt2 ()

This works as suggested by the indentation, but if we insert a let ... in before stmt1 (), then stmt2 () becomes part of the else clause, since let effectively acts as a start delimiter with a missing end delimiter, though it's not terminated by ;. The issue also occurs for then then clause.

paurkedal commented Nov 12, 2017

I'm not a fan of using ordinary operators for extensions. You could use #508 instead, or at least explicitly reserve some operators for use with extensions (IIRC this is how ## works). The choice of precedence level seems a bad idea, much more likely to cause issues than just using the precedence level of ;.

It may be a good idea to reserve it for extensions; I added a check in env.ml, but took it out for no good reason. It's a matter of taste, but I think ;%ext makes the code look messy, so one of the motivations for this PR is to provide an alternative which looks more like a regular operator.

I disagree about the precedence level, since the level of ; causes inconsistent grouping when mixing conditions with let, and providing an opt-out from those issues is the second motivation for this PR.

Added: For those who haven't followed the "safe-syntax" discussion, the main issue as as follows. Consider the code

if ... then
   ...
else
   stmt1 ();
stmt2 ()

This works as suggested by the indentation, but if we insert a let ... in before stmt1 (), then stmt2 () becomes part of the else clause, since let effectively acts as a start delimiter with a missing end delimiter, though it's not terminated by ;. The issue also occurs for then then clause.

@lpw25

This comment has been minimized.

Show comment
Hide comment
@lpw25

lpw25 Nov 12, 2017

Contributor

I don't disagree that it would be a better precedence level for ;, however it's obviously too late to change that. Having bind/map operators that look and behave like ; but with a different precedence is going to cause confusion and subtle bugs. Same goes for having both -; and ; which differ only in precedence -- if you miss a - by accident you are going to get a bug that only shows up at run-time.

Contributor

lpw25 commented Nov 12, 2017

I don't disagree that it would be a better precedence level for ;, however it's obviously too late to change that. Having bind/map operators that look and behave like ; but with a different precedence is going to cause confusion and subtle bugs. Same goes for having both -; and ; which differ only in precedence -- if you miss a - by accident you are going to get a bug that only shows up at run-time.

@paurkedal

This comment has been minimized.

Show comment
Hide comment
@paurkedal

paurkedal Nov 12, 2017

I see your point about confusing -; and ; since it's prone to the same silent acceptance from the compiler as the else-let gotcha. A remedy, though a radical one, may be for the PPX to reject occurrences of ;.

Added: This linting behaviour may need to come first in the PPX pipeline, since it may not be possible to tell if a ; occurs in source code or is generated by a PPX.

paurkedal commented Nov 12, 2017

I see your point about confusing -; and ; since it's prone to the same silent acceptance from the compiler as the else-let gotcha. A remedy, though a radical one, may be for the PPX to reject occurrences of ;.

Added: This linting behaviour may need to come first in the PPX pipeline, since it may not be possible to tell if a ; occurs in source code or is generated by a PPX.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment