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

bug: IDE experience incorrectly flags inherit constructor chaining where no vals #554

Closed
bartelink opened this issue Aug 7, 2023 · 7 comments

Comments

@bartelink
Copy link

bartelink commented Aug 7, 2023

Using 2022.3.3 on a Mac, the IDE flags an (admittedly esoteric) bit of syntax, that compiles fine:
image
(if I add a dummy val, it works fine)

References:

@auduchinok
Copy link
Member

@bartelink Hi, thank you for reporting it. Could you please check it on 2023.2 release? It should be fixed there.

@bartelink
Copy link
Author

bartelink commented Aug 7, 2023

Apologies, doh! (I was distracted by an on March 20, 2023 in the about and didn't even consider I could be that far behind - I didn't actually read the version number I pasted above!)
Closing; will necropost if it turns out this still repros

@bartelink
Copy link
Author

Unfortunately I'm still seeing it in 2023.2
If you replace EventStoreCategory in https://github.com/jet/equinox's master with

type EventStoreCategory<'event, 'state, 'context> =
    inherit Equinox.Category<'event, 'state, 'context>
    new(context: EventStoreContext, name, codec: FsCodec.IEventCodec<_, _, 'context>, fold, initial, access, caching) =
        { inherit Equinox.Category<'event, 'state, 'context>(name,
            Category<'event, 'state, 'context>(context, codec, fold, initial, access)
            |> Caching.apply Token.isStale caching) }

it's still asserting 'this record contains fields from inconsistent types', despite it compiling...
As mentioned, changing it to:

type EventStoreCategory<'event, 'state, 'context> =
    inherit Equinox.Category<'event, 'state, 'context>
    val dummy: int
    new(context: EventStoreContext, name, codec: FsCodec.IEventCodec<_, _, 'context>, fold, initial, access, caching) =
        { inherit Equinox.Category<'event, 'state, 'context>(name,
            Category<'event, 'state, 'context>(context, codec, fold, initial, access)
            |> Caching.apply Token.isStale caching); dummy = 1 }

shuts it up
(The ids branch applies a few more in the most recent commit, but they are all equivalent in that they don't have any vals to initialize etc)

@bartelink bartelink reopened this Aug 8, 2023
@auduchinok
Copy link
Member

auduchinok commented Aug 8, 2023

@bartelink Thanks for checking! Yes, indeed, it's actually fixed in 2023.3 builds, not in 2023.2. There was an issue in the initial implementation of dotnet/fsharp#15214 in our FCS work, and I've fixed it when proposed the change upstream, and 2033.3 includes this change.

2023.2:

Screenshot 2023-08-08 at 12 00 53

2023.3:

Screenshot 2023-08-08 at 12 01 40

I'll see if it's safe to backport it to a bug fix release.

@bartelink
Copy link
Author

I'll see if it's safe to backport it to a bug fix release.

Please don't; I'll wait

@jwosty
Copy link

jwosty commented Nov 16, 2023

Just ran into something similar:

namespace FSharpSandbox
open System

type MyException =
    inherit Exception
    
    new() = { inherit Exception() }
    new(msg: string) = { inherit Exception(msg) }
    new(msg: string, innerExn: Exception) = { inherit Exception(msg, innerExn) }

I'm assuming this is the same root problem and will also be fixed in 2023.3?

@auduchinok
Copy link
Member

@jwosty Thanks for reporting it! Yes, all looks good in 2023.3:
Screenshot 2023-11-16 at 20 58 08

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

3 participants