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

Poor error message when defining a function in a require #1517

Open
ag-eitilt opened this issue Feb 12, 2024 · 2 comments
Open

Poor error message when defining a function in a require #1517

ag-eitilt opened this issue Feb 12, 2024 · 2 comments
Labels
backlog Task that isn't actively being worked on bug Something isn't working

Comments

@ag-eitilt
Copy link
Collaborator

I don't know if this is actually possible to detect with the parser as it is, but the current message when require is given something which isn't headed by a constructor (e.g. require bad = Pass "") is just the required pattern can never fail; use a def instead. This in itself could maybe use a bit of rewording (if nothing else, a capital The), but that same message is used in cases where the message is confusing or even a bit misleading: require Pass fn arg = ... where the intention is to define fn as a function taking arg and producing a Result a Error (and likely which automatically unwraps that value by inserting the appropriate require in at any of its callsites).

My wild guess at what's happening is that we're running into another case of the reverse typechecker -- Pass fn being resolved as an b => c, then getting applied to arg for something like require c = ... and we never get far enough to realize that Pass fn is actually a known type which doesn't work as a function. Or in other words, Wake is so convinced that it's dealing with a two-field type constructor that it's willing to hallucinate a single-constructor type rather than matching against the single-field constructor that's actually in scope. Ultimately, this should probably be a clear syntax error rather than a typechecker error, but even as a typechecker error something weird's happening.

@ag-eitilt ag-eitilt added bug Something isn't working backlog Task that isn't actively being worked on labels Feb 12, 2024
@V-FEXrt
Copy link
Collaborator

V-FEXrt commented Feb 14, 2024

@ag-eitilt can you post an example snippet?

@ag-eitilt
Copy link
Collaborator Author

The example which was throwing the error (lightly edited) was:

            require Pass getPathsAt (attribute: String) =
                query parsedFile
                $ jField attribute
                $ jFlatten
                $ jString

That would be a viable function definition if using def rather than require Pass, but as it is, the problem I'd want to have pointed out would be something along the lines of "Pass only takes 1 argument (given 2)" -- which is why I say it's a reverse type checker error under everything.

This was being used in (again edited) the following block, which I do see as being a natural assumption to make given how require Pass object seems to "magically" work when someone doesn't have a deep knowledge of the match equivalence for require.1

            def constructPathPair (attribute: String): Pair String (Result (List Path) Error) =
                Pair attribute
                (
                    getPathsAt attribute
                    | map source
                    | findFail
                )

Footnotes

  1. In fact, I do actually quite like how succinct it would make it if the thought behind those blocks were indeed possible: attach the match desugaring to the function, without resolving it at the definition site, so that the call would be resolved as if it were:

    Pair attribute
    (
        match (getPathsAt' attribute)
            Pass a -> a | map source | findFail
            Fail b -> Fail b
    )
    

    It's nothing which the current parser would be able to support, and would be rather tricky to do right and do ergonomically even if it were, but it might be something I'd throw on the pile of "features to think about once we have a better parser and much more bandwidth". That's a separate concern, though, and not actually part of the main issue being filed.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
backlog Task that isn't actively being worked on bug Something isn't working
Projects
None yet
Development

No branches or pull requests

2 participants