Eliminate union case remapping common in monadic functions

[kerams]

Consider an inlined version of Result.map

let inline map mapping result =
    match result with
    | Error e -> Error e
    | Ok x -> Ok (mapping x)

let ff x = map (fun y -> y.ToString ()) x

let ffs x = map id x

The last 2 functions compiled down to

public static FSharpResult<string, b> ff<a, b>(FSharpResult<a, b> x)
{
    if (x.Tag == 0)
    {
        a resultValue = x.ResultValue;
        a a = resultValue;
        return FSharpResult<string, b>.NewOk(a.ToString());
    }
    return FSharpResult<string, b>.NewError(x.ErrorValue);
}

public static FSharpResult<a, b> ffs<a, b>(FSharpResult<a, b> x)
{
    if (x.Tag == 0)
    {
        return FSharpResult<a, b>.NewOk(x.ResultValue);
    }
    return FSharpResult<a, b>.NewError(x.ErrorValue);
}

In both functions a new error case is allocated. This is necessary in ff (and in the original map), because the input result type is Result<'a, 'b> and the output is Result<string, 'b>. However, input and output types are the same in ffs, so the original union case can simply pass through instead of being recreated.

public static FSharpResult<a, b> ffs<a, b>(FSharpResult<a, b> x)
{
    if (x.Tag == 0)
    {
        return FSharpResult<a, b>.NewOk(x.ResultValue);
    }
    return x; // <---------
}

[kerams]

I've had a look at how this could be implemented in Optimizer.fs. Unfortunately, it isn't immediately obvious that a union construction expression arises from a match clause like | Error e -> Error e, because the original match expression undergoes significant changes during pattern compilation. In theory we could always replace a union construction expression in the form Case (old.Field1, old.Field2) provided that the type of this expression and old is the same and all fields are supplied from old in the right order.

This would mean that even

let copy = Case (old.Field1, old.Field2)

would be turned into

let copy = old

which is perfectly OK thanks to union immutability, but is problematic when someone relies on reference (in)equality.

[Happypig375]

@kerams Reference equality is already an issue in your original optimisation.

[kerams]

Indeed, but you have no choice - | e -> e doesn't fly, even though the author might have liked that.

[Happypig375]

@kerams or you can investigate making that syntax work.

[Happypig375]

If that syntax can work, then more complex cases of returning the original may also work:

let inline map mapping result =
    match result with
    | Ok x -> Ok (mapping x)
    | (Error e) as x ->
        printfn $"This error is {e}"
        x

[kerams]

That does work already and constrains mapping to 'a -> 'a.

[Happypig375]

Ah then this path won't work. Nevermind.