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

Improve CE Combine/Delay error reporting ranges #18422

Open
edgarfgp opened this issue Mar 27, 2025 · 0 comments · May be fixed by #18394
Open

Improve CE Combine/Delay error reporting ranges #18422

edgarfgp opened this issue Mar 27, 2025 · 0 comments · May be fixed by #18394

Comments

@edgarfgp
Copy link
Contributor

Is your feature request related to a problem? Please describe.

Currently when implementing Combine and Delay CE members the error reporting uses the the complete ranges of all the expressions involved. e.g.

Describe the solution you'd like

Instead of using the complete range, we should be using the ranges of the expressions after the first one as this are the expressions that we are trying to Combine

type ListBuilder() =

    member this.Yield(x) = [x]

    member this.For(m,f) =
        m |> List.collect f

    member this.Combine (a,b) =
        List.concat [a;b]

    // member this.Delay(f) = f()
    
let listBuilder = new ListBuilder()

let result = listBuilder {
    let a = 10
    yield a
    if true then
    ^^^^^^^^^^^^
        yield 20
        ^^^^^^^^
    else
    ^^^^
        yield 30
        ^^^^^^^^
}	

Alternatively we can only uses the range for the places where we are actually using return or yield. For this we will need to use error recovery errorR but that might show errors hidden on purpose due to unhandled cases in the following phases.

type ListBuilder() =

    member this.Yield(x) = [x]

    member this.For(m,f) =
        m |> List.collect f

    member this.Combine (a,b) =
        List.concat [a;b]

    // member this.Delay(f) = f()
    
let listBuilder = new ListBuilder()

let result = listBuilder {
    let a = 10
    yield a
    if true then
        yield 20
        ^^^^^
    else
        yield 30
        ^^^^^
}

Describe alternatives you've considered

A clear and concise description of any alternative solutions or features you've considered.

Additional context

If the issue is about:

Add any other context or screenshots about the feature request here.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
Status: New
Development

Successfully merging a pull request may close this issue.

1 participant