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

Guess that TypeApplications is used without being enabled #452

Closed
neongreen opened this issue Nov 6, 2019 · 15 comments · Fixed by #722
Closed

Guess that TypeApplications is used without being enabled #452

neongreen opened this issue Nov 6, 2019 · 15 comments · Fixed by #722
Labels
awaiting-pr Issues that users should solve themselves and open a PR. feature-request

Comments

@neongreen
Copy link
Collaborator

Since we don't have #56, people who have TypeApplications in their default-extensions get their f @Foo turned silently into f@Foo.

We can detect this by spotting EAsPat when rendering expressions and erroring out.

@neongreen neongreen changed the title Guess that TypeApplications is used Guess that TypeApplications is used without being declared Nov 6, 2019
@neongreen neongreen changed the title Guess that TypeApplications is used without being declared Guess that TypeApplications is used without being enabled Nov 6, 2019
@mrkkrp
Copy link
Member

mrkkrp commented Nov 6, 2019

I don't understand how this is going on work. If TypeApplications are not enabled, you can't get @ parsed as a type application. In addition to that EAsPat is for xs@(x:_) sort of thing, right?

@neongreen
Copy link
Collaborator Author

Yes, you don't get it parsed as a type application, you get it parsed as an as-pattern. The AST is not well-typed enough, so it does not prohibit as-patterns in expression context.

@neongreen
Copy link
Collaborator Author

https://gitlab.haskell.org/ghc/ghc/wikis/design/exp-pat-frame

The approach GHC uses is to parse patterns as expressions and rejig later. This turns out to be suboptimal [...]

@mrkkrp
Copy link
Member

mrkkrp commented Nov 6, 2019

What if it was a genuine "at pattern"?

@neongreen
Copy link
Collaborator Author

neongreen commented Nov 6, 2019

In p_pat we would render it as usually.

In p_hsExpr we would error out, similarly to how GHC does at a certain stage after parsing:

/tmp/ta.hs:1:8: error:
    Pattern syntax in expression context: f@Foo
    Did you mean to enable TypeApplications?

@neongreen
Copy link
Collaborator Author

We already have a comment there:

  -- These four constructs should never appear in correct programs.
  -- See: https://github.com/tweag/ormolu/issues/343
  EWildPat NoExt -> txt "_"
  EAsPat NoExt n p -> do
    p_rdrName n
    txt "@"
    located p p_hsExpr
  EViewPat NoExt p e -> do
    located p p_hsExpr
    space
    txt "->"
    breakpoint
    inci (located e p_hsExpr)
  ELazyPat NoExt p -> do
    txt "~"
    located p p_hsExpr

Btw, I believe we can error out on ELazyPat and EViewPat too. We should not error out on EWildPat because that's what holes get parsed into.

Regarding #343, I believe that it should be fixed now that UnicodeSyntax is disabled by default, so we don't have to handle EViewPat for the case described in #343 to work. We should add it to the testsuite anyway, though.

@neongreen
Copy link
Collaborator Author

Actually, it has just occurred to me – what syntax does TypeApplications steal precisely? We have a comment that says

    TypeApplications, -- steals (@) operator on some cases

but it doesn't seem to steal that operator:

main = main
  where
    (@) = (+)
/tmp/ta.hs:3:6: error: parse error on input ‘@’
  |
3 |     (@) = (+)
  |      ^

It also isn't mentioned in the summary of stolen syntax.

Perhaps we should just enable it by default?

@neongreen
Copy link
Collaborator Author

Oh damn, found it:

{-# LANGUAGE TypeApplications #-}

main = case [1] of
  xs @ (x:_) -> print (x, xs)
  xs@[] -> print xs
/tmp/ta.hs:4:3: error: Parse error in pattern: xs @(x : _)
  |
4 |   xs @ (x:_) -> print (x, xs)
  |   ^^^^^^^^^^

@mrkkrp
Copy link
Member

mrkkrp commented Nov 6, 2019

What do you mean by "error out"? Can you describe the whole plan step by step? I still do not understand what you want to implement, sorry. I'd like to have clear description of problem you want to solve and then your proposed solution.

Perhaps we should just enable it by default?

No, I remember this wasn't enabled at first and we found a failing case. @utdemir may remember what exactly.

@neongreen
Copy link
Collaborator Author

I still do not understand what you want to implement, sorry.

image

I'd like to have clear description of problem you want to solve

The problem is that currently, when a user formats f = g @Bar, they get f = g@Bar. If they have TypeApplications enabled in default-extensions, this will actually cause the code to stop compiling.

@mrkkrp
Copy link
Member

mrkkrp commented Nov 6, 2019

@neongreen Thanks. Now it's clear. I'd be OK with a PR for this.

@neongreen neongreen added the awaiting-pr Issues that users should solve themselves and open a PR. label Nov 6, 2019
@georgefst
Copy link

I'd be OK with a PR for this.

Just to be clear, do you mean a PR adding TypeApplications to the default set? It's even in the new GHC2021, which feels like another argument in favour of assuming its use.

@mrkkrp
Copy link
Member

mrkkrp commented Jan 20, 2021

Just enabling it by default is will break things because it steals syntax. There should be something more clever, ask @neongreen, because I have forgotten and it's not obvious from skimming the thread.

@neongreen
Copy link
Collaborator Author

neongreen commented Jan 20, 2021

I proposed guessing that -XTypeApplications should be enabled by detecting EAsPat in expression context.

I am not going to implement this myself, though.

@amesgen amesgen mentioned this issue Jul 23, 2021
@amesgen
Copy link
Member

amesgen commented Jul 23, 2021

Most likely, this issue can be closed as soon as ghc-lib-parser-9.0 is used, namely due to the new whitespace-sensitivity of !, ~, @ and $ which means that the example

main = case [1] of
  xs @ (x:_) -> print (x, xs)
  xs@[] -> print xs

from above will fail to parse even without TypeApplications being enabled:

<interactive>:3:6: error:
    Found a binding for the ‘@’ operator in a pattern position.
    Perhaps you meant an as-pattern, which must not be surrounded by whitespace

With ghc-lib-parser-9.0, I can't think of an example where enabling/disabling TypeApplications would make any difference (please mention it here if you find one!), so we will be able to enable TypeApplications by default in the next release!

Javran added a commit to Javran/advent-of-code that referenced this issue Aug 30, 2022
Previous version built on GHC 8.10.7, which seems to have some issue
with TypeApplications.
(Probably related to tweag/ormolu#452)
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
awaiting-pr Issues that users should solve themselves and open a PR. feature-request
Projects
None yet
Development

Successfully merging a pull request may close this issue.

4 participants