Opened 6 months ago

Last modified 6 months ago

#16311 merge task

Suggest -XExistentialQuantification for 'forall' in data declarations

Reported by: int-index Owned by:
Priority: normal Milestone: 8.8.1
Component: Compiler (Parser) Version: 8.7
Keywords: Cc: RyanGlScott
Operating System: Unknown/Multiple Architecture: Unknown/Multiple
Type of failure: None/Unknown Test Case: rename/should_fail/rnfail053
Blocked By: Blocking:
Related Tickets: Differential Rev(s): https://gitlab.haskell.org/ghc/ghc/merge_requests/363
Wiki Page:

Description

Phab:D5180 introduced a slight regression to the error messages. In this code

data T = forall a. MkT a

GHC used to complain

rnfail053.hs:5:10:
    Not a data constructor: ‘forall’
    Perhaps you intended to use ExistentialQuantification

but then the message has become

rnfail053.hs:5:18: error:
    Illegal symbol '.' in type
    Perhaps you intended to use RankNTypes or a similar language
    extension to enable explicit-forall syntax: forall <tvs>. <type>

Change History (6)

comment:1 Changed 6 months ago by RyanGlScott

Cc: RyanGlScott added

comment:2 Changed 6 months ago by RyanGlScott

I'm especially interested in this ticket because I recently implemented a prototype of the forall-always-a-keyword-in-types proposal, and to my surprise, this caused the stderr for rnfail053 to change even further:

  • rename/should_fail/rnfail053.run/rnfail053.

    diff -uw "rename/should_fail/rnfail053.run/rnfail053.stderr.normalised" "rename/should_fail/rnfail053.run/rnfail053.comp.stderr.normalised"
    old new  
    11
    22rnfail053.hs:5:10:
    3     Illegal symbol ‘forall’ in type
    4     Perhaps you intended to use RankNTypes or a similar language
    5     extension to enable explicit-forall syntax: ‘forall <tvs>. <type>’
     3     Data constructor ‘MkT’ has existential type variables, a context, or a specialised result type
     4        MkT :: forall a. a -> T
     5        (Enable ExistentialQuantification or GADTs to allow this)
     6     In the definition of data constructor ‘MkT’
     7      In the data type declaration for ‘T’

It wasn't clear to me at the time whether this was an upgrade or downgrade in error message quality, so I worked around it by making this change to the parser:

  • compiler/parser/Parser.y

    diff --git a/compiler/parser/Parser.y b/compiler/parser/Parser.y
    index 820144d930..e3423669bf 100644
    a b constr :: { LConDecl GhcPs } 
    22752293                       (fst $ unLoc $2) }
    22762294
    22772295forall :: { Located ([AddAnn], Maybe [LHsTyVarBndr GhcPs]) }
    2278         : 'forall' tv_bndrs '.'       { sLL $1 $> ([mu AnnForall $1,mj AnnDot $3], Just $2) }
     2296        : 'forall' tv_bndrs '.'       {% do { hintExplicitForall $1
     2297                                            ; pure $ sLL $1 $>
     2298                                                ( [ mu AnnForall $1
     2299                                                  , mj AnnDot $3 ]
     2300                                                , Just $2) } }
    22792301        | {- empty -}                 { noLoc ([], Nothing) }

That reverted the error message back to what it was before (mentioning RankNTypes). But perhaps the error message really ought to mention ExistentialQuantification.

Some related questions: there are currently two distinct error messages that mention not enabling ExistentialQuantification:

  • There's the one in the original description of this ticket (the "Not a data constructor: ‘forall’" one).
  • There's also the one I mentioned in this comment (the "has existential type variables, a context, or a specialised result type" one).

Do we need both? Or is there a way we could consolidate the two somehow? Would the answer change if we made forall permanently a keyword in types?

comment:3 Changed 6 months ago by int-index

Do we need both? Or is there a way we could consolidate the two somehow? Would the answer change if we made forall permanently a keyword in types?

No, we don't need both, and the forall proposal makes the matters trivial. See https://gitlab.haskell.org/ghc/ghc/merge_requests/363

comment:4 Changed 6 months ago by int-index

Milestone: 8.8.1
Status: newpatch

comment:5 Changed 6 months ago by int-index

Differential Rev(s): https://gitlab.haskell.org/ghc/ghc/merge_requests/363
Version: 8.6.38.7

comment:6 Changed 6 months ago by RyanGlScott

Status: patchmerge
Test Case: rename/should_fail/rnfail053
Note: See TracTickets for help on using tickets.