CE compilation error with do! in a sequential expression

[baronfel]

Discovered as part of investigating demystifyfp/FsToolkit.ErrorHandling#84

FsToolkit.ErrorHandling recently added preliminary support for the applicative CE members feature of F# 5. After this was released, some constructs that users had no longer compiled.

Repro steps

Run the following script in a preview-enabled dotnet fsi: dotnet fsi --langversion:preview /path/to/script

#r "nuget: FsToolkit.ErrorHandling"

open FsToolkit.ErrorHandling

asyncResult {
    let! something = asyncResult { return 5 }
    do! asyncResult {
        return ()
    }
    return something
}

Expected behavior

This code should compile.

Actual behavior

The code fails to compile with the following error:

This construct may only be used within computation expressions

around the entire do! block

Known workarounds

The do! expression can be rewritten to

let! _ = asyncResult {
	return ()
}

and the code will compile. This is semantically the same as do!, but requires user changes.

Related information

Here is a link to the fantomas tools for this snippet. What is immediately jumps out to me is that the do!/return combination is parsed as a SynExpr.Sequential:

Sequential
  (Both,true,
    DoBang
      (App
        (NonAtomic,false,Ident asyncResult,
        CompExpr
          (false,{ contents = false },
            YieldOrReturn ((false, true), Const Unit),
            ),
        ),
      ),
    YieldOrReturn
      ((false, true),Ident something),
  )

For reference, this is coming from the following parse rule (pars.fsy, line 3231-3232):

seqExpr:
  | declExpr seps seqExpr
      { SynExpr.Sequential (DebugPointAtSequential.Both, true, $1, $3, unionRanges $1.Range $3.Range) } 

We can tell this because the isTrueSeq boolean flag is true, and this is the only place happens.

Note: this next portion is probably wrong, further digging ongoing

So moving on to typechecking, SynExpr.Sequential is checked in TcExprUndelayed (Typechecker.fs line 6196), and at line 6198 moves to check the linear exprs that are contained inside it. I think this is where we start to go wrong. TcLinearExprs takes a boolean flag isCompExpr, which is always set to false. This use case proves to me that this assertion cannot be true. So I think that we need to check for the existence of CE operations in the Sequential Exprs before checking them linearly. Once we have that boolean flag, I think in TcLinearExprs we need to check that flag in the SynExpr.Sequential pattern and check the CE expressions using TcComputationExpression`.

Am I on a sensible path here? Are there other options that I'm missing?

[cartermp]

@dsyme this looks like a regression as a consequence of the applicatives work

[baronfel]

First thing I'm seeing is that isSimpleExpr isn't returning false for SynExpr.DoBang and I think it should be: https://github.com/dotnet/fsharp/blob/master/src/fsharp/TypeChecker.fs#L8863-L8887

Adding that results in a new error:

/Users/chethusk/oss/scratch/do.fsx(5,1): error FS0030: Value restriction. The value 'it' has been inferred to have generic type
    val it : Async<Result<int,'_a>>    
Either define 'it' as a simple data term, make it a function with explicit arguments or, if you do not intend for it to be generic, add a type annotation.

[baronfel]

I'm a dummy, my test script needs to pin the generic parameters. I can confirm that adding DoBang to that check solves this issue. PR incoming, will similarly need help with adding tests.