Opened 2 years ago

Closed 2 years ago

Last modified 18 months ago

#9867 closed bug (fixed)

PatternSynonyms + ScopedTypeVariables triggers an internal error

Reported by: antalsz Owned by: cactus
Priority: normal Milestone: 8.0.1
Component: Compiler (Type checker) Version: 7.8.3
Keywords: PatternSynonyms Cc:
Operating System: Unknown/Multiple Architecture: Unknown/Multiple
Type of failure: Compile-time crash Test Case: patsyn/should_compile/T9867
Blocked By: Blocking:
Related Tickets: Differential Rev(s):
Wiki Page:

Description

When trying to give a type signature with a free type variable to a pattern in a pattern synonym, GHC dies with an internal error. Given the file

{-# LANGUAGE PatternSynonyms, ScopedTypeVariables #-}
pattern Nil = [] :: [a]

the resulting error is

Example.hs:2:22: GHC internal error: ‘a’ is not in scope during type checking, but it passed the renamer …
   tcl_env of environment: []
   In an expression type signature: [a]
   In the expression: [] :: [a]
   In an equation for ‘$WNil’: $WNil = [] :: [a]
Compilation failed.

The fix is to add RankNTypes and an explicit forall:

{-# LANGUAGE PatternSynonyms, ScopedTypeVariables, RankNTypes #-}
pattern Nil = [] :: forall a. [a]

Change History (10)

comment:1 Changed 2 years ago by simonpj

Owner: set to cactus

Gergo, would you like to look at this?

I think we should write down the rules for scoped type variables in pattern synonym definitions!

Thanks

Simon

comment:2 Changed 2 years ago by cactus

Hrm.

So what's happening here is that when we see

pattern Nil = [] :: [a]

we rename and typecheck [] :: [a] as a pattern (like in a function definition); this by itself requires ScopedTypeVariables (in rnHsBndrSig I believe), as you can see by trying the following:

{-# LANGUAGE PatternSynonyms #-}
pattern Nil = [] :: [a]
T9867a.hs:2:21:
    Illegal type signature: ‘[a]’
      Perhaps you intended to use ScopedTypeVariables
    In a pattern type-signature

If you do turn on ScopedTypeVariables, renaming succeeds and puts an unbound type variable in the type signature:

pattern main@main:Main.Nil{d rvh} = 
            ghc-prim-0.3.1.0:GHC.Types.[]{(w) d 6m} :: [a{tv awF}]

Then tcInferPatSynDecl typechecks this right-hand side, via some indirections and appropriate unifications, so everything works out just fine:

T9867.hs:2:15:
    env2 [(a{tv awF}, Type variable ‘a{tv awF}’ = a{tv awI}[sig:2])]
T9867.hs:2:9: Skolemising a{tv awI}[sig:2] := a{tv awJ}[sk]
T9867.hs:2:9: writeMetaTyVar a{tv awI}[sig:2] := a{tv awJ}[sk]

The problem shows up when we typecheck the builder, however. (Note that if we change the definition of Nil to be unidirectional, the error goes away). tcPatSynBuilderBind gets the original pattern synonym binding as output by the renamer; there is no additional tunneling from the result of typechecking the pattern synonym. And so the binding generated for the builder is:

tcPatSynBuilderBind:
  main@main:Main.Nil{v rwL}[gid] :: [a{tv awJ}[sk]]
                                    [Nothing]
   main@main:Main.Nil{v rwL}
     = ghc-prim-0.3.1.0:GHC.Types.[]{(w) d 6m} :: [a{tv awF}]
Last edited 2 years ago by cactus (previous) (diff)

comment:3 Changed 2 years ago by cactus

Note that if it turns out we need to get some information from the result of typechecking the pattern synonym to tcPatSynBuilderBind, then that could be related to https://ghc.haskell.org/trac/ghc/ticket/8581#comment:19 which is also about plumbing some data between matcher and builder typechecking.

Last edited 2 years ago by cactus (previous) (diff)

comment:4 Changed 2 years ago by cactus

Component: CompilerCompiler (Type checker)
Keywords: pattern synonyms added
Type of failure: None/UnknownCompile-time crash

comment:5 Changed 2 years ago by cactus

Some brainstorming:

The patsyn builders are typechecked from this code:

(binds', (extra_binds', thing)) <- tcBindGroups top_lvl sig_fn prag_fn binds $ do
     { thing <- thing_inside
       -- See Note [Pattern synonym builders don't yield dependencies]
     ; patsyn_builders <- mapM tcPatSynBuilderBind patsyns
     ; let extra_binds = [ (NonRecursive, builder) | builder <- patsyn_builders ]
     ; return (extra_binds, thing) }

At the time we call tcPatSynBuilderBind, all the typechecked pattern synonyms are available in the global environment, so we could look up whatever we could possibly need from the output of the typechecker. Currently, it is only easy to grab stuff from the PatSyn (which deliberately doesn't contain any details of the actual definition of the pattern synonym, only its type), but we should be able to look up the PatSynBind Id Id for a given PatSynBind Name Name; and then we should be in business.

comment:6 Changed 2 years ago by cactus

Keywords: PatternSynonyms added; pattern synonyms removed

comment:7 Changed 2 years ago by Simon Peyton Jones <simonpj@…>

comment:8 Changed 2 years ago by simonpj

Status: newmerge
Test Case: patsyn/should_compile/T9867

Fixed by

commit 3ea40e38a7ae03c05cb79485fb04a3f00c632793
Author: Simon Peyton Jones <simonpj@microsoft.com>
Date:   Mon Jan 19 09:06:21 2015 +0000

    Fix the 'builder' code for pattern synonyms with type signatures
    
    See Note [Type signatures and the builder expression] for the details

 compiler/typecheck/TcPatSyn.hs |  144 ++++++++++++++++++++++------------------
 1 file changed, 79 insertions(+), 65 deletions(-)

and follow up wibble

commit 9a1458266b8fa349c9fb58889825d899a762fa27
Author: Simon Peyton Jones <simonpj@microsoft.com>
Date:   Mon Jan 19 10:20:39 2015 +0000

    Add missing argument in Match, a merge bug (apologies)

 compiler/typecheck/TcPatSyn.hs |    4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

We could consider merging this to 7.10. It isn't huge, but let's do it if it's easy.

Simon

comment:9 Changed 2 years ago by thoughtpolice

Milestone: 7.12.1
Resolution: fixed
Status: mergeclosed

I don't think this can be easily merged to ghc-7.10 because I think it's dependent on 32973bf3c2f6fe00e01b44a63ac1904080466938, which I'm not keen on merging. So I'm going to drop this one to 7.12 instead.

comment:10 Changed 18 months ago by thoughtpolice

Milestone: 7.12.18.0.1

Milestone renamed

Note: See TracTickets for help on using tickets.