Opened 5 years ago

Closed 3 years ago

Last modified 3 years ago

#5682 closed bug (fixed)

Properly parse promoted data constructor operators

Reported by: lunaris Owned by:
Priority: normal Milestone: 7.8.1
Component: Compiler (Parser) Version: 7.8.1-rc1
Keywords: PolyKinds, ghc-kinds Cc: jpm@…, eir@…, trevor@…
Operating System: Unknown/Multiple Architecture: Unknown/Multiple
Type of failure: GHC rejects valid program Test Case: parser/should_compile/T5682
Blocked By: Blocking:
Related Tickets: #8486 Differential Rev(s):
Wiki Page:

Description

% ghci -XPolyKinds -XTypeOperators
GHCi, version 7.3.20111204: http://www.haskell.org/ghc/  :? for help
Loading package ghc-prim ... linking ... done.
Loading package integer-gmp ... linking ... done.
Loading package base ... linking ... done.
Prelude> :k (:)

<interactive>:1:2: parse error on input `:'
Prelude> :k '(:)

<interactive>:1:3: parse error on input `:'
Prelude> :k (':)

<interactive>:1:3: parse error on input `:'
Prelude> :k (Int ': '[Bool])
(Int ': '[Bool]) :: [*]

Change History (21)

comment:1 Changed 5 years ago by dreixel

Cc: jpm@… added
Summary: Cannot parse (:) in a KindSignature with -XPolyKindsProperly parse kind operators (from promoted type operators)

We need to handle kind operators properly in the parser. This is not restricted to the wired-in promoted types; consider, for instance:

data a :+: b = LEFT a | RIGHT b

Currently, the following all fail to parse:

data Sum0 :: * :+: * -> *
data Sum1 :: * ':+: * -> *
data Sum2 :: '(:+:) * * -> *
data Sum3 :: ':+: * * -> *

I guess that Sum3 should indeed fail to parse, but all the others should work.

comment:2 Changed 5 years ago by igloo

difficulty: Unknown
Milestone: 7.4.1

comment:3 Changed 5 years ago by igloo

Milestone: 7.4.17.6.1
Priority: normallow

comment:4 Changed 5 years ago by igloo

Milestone: 7.6.17.6.2

comment:5 Changed 4 years ago by goldfire

Cc: eir@… added

I agree with dreixel's examples in spirit, but generally ' is not applied to kinds.

comment:6 in reply to:  5 Changed 4 years ago by dreixel

Replying to goldfire:

generally ' is not applied to kinds.

Oh, yes, that's true. I think only Sum0 should parse, in fact.

comment:7 Changed 4 years ago by elliottt

Cc: trevor@… added

comment:8 Changed 3 years ago by dreixel

Component: CompilerCompiler (Parser)
Owner: dreixel deleted
Priority: lownormal

I don't think this is low priority; #8486 (inability to derive Typeable for promoted :), in particular, is rather annoying. But I'm removing myself as the owner, as I don't really feel comfortable hacking around in the parser...

comment:9 Changed 3 years ago by goldfire

There seems to be two different (but closely related) issues here: (1) parsing promoted data constructor operators, and (2) parsing promoted type constructor operators.

The motivating example at the top of the report is only about issue (1). And, I believe that Pedro (dreixel)'s recent comment is also only about (1).

Pedro's first comment to this ticket, however, is about issue (2). I think issue (2) is unsolvable, due to the presence of * in the grammar of kinds. See #8706.

But, I do think (1) should be doable. Do I feel comfortable in the parser? By no means. But I've done a little poking about in there and might be able to fix this. Hopefully tomorrow. If someone out there does feel comfortable in the parser, I'm happy to let you do it!

comment:10 Changed 3 years ago by dreixel

Summary: Properly parse kind operators (from promoted type operators)Properly parse promoted data constructor operators

Richard, I agree that parsing promoted type operators (thus when parsing kinds) is trickier. Let's make this ticket be only about parsing promoted constructor operators (thus when parsing types), and leave #8706 for kinds.

comment:11 Changed 3 years ago by Richard Eisenberg <eir@…>

In 473f12a3be27a00b035f1fdc7050a0ff31bf12ff/ghc:

Fix #5682. Now, '(:) parses.

comment:12 Changed 3 years ago by goldfire

Milestone: 7.6.27.8.1
Status: newmerge

All set here -- I see no reason not to merge.

comment:13 Changed 3 years ago by goldfire

Test Case: parser/should_compile/T5682

comment:14 Changed 3 years ago by goldfire

Version: 7.37.8.1-rc1

comment:15 Changed 3 years ago by thoughtpolice

Resolution: fixed
Status: mergeclosed

Merged in 7.8.

comment:16 Changed 3 years ago by guest

Resolution: fixed
Status: closednew

Promoted tuple constructors (e.g. '(,,)) still don't parse. This patch to compiler/parser/Parser.y.pp fixes it. With this patch, ' can be used with any qcon so the redundant rules have been removed. Using an un-promotable data constructor will give a meaningful error message such as Data constructor ‛(##)’ comes from an un-promotable type ‛(##)’ instead of a parse error.

'(' varop ')' was changed to var because 'id is more consistent than '(`id`) and btype SIMPLEQUOTE varop type (in type and typedoc) allows backquoted identifiers as operators.

1154,1155c1154
<         | SIMPLEQUOTE qconid                          { LL $ HsTyVar $ unLoc $2 }
<         | SIMPLEQUOTE  '(' ')'                        { LL $ HsTyVar $ getRdrName unitDataCon }
---
>         | SIMPLEQUOTE qcon                            { LL $ HsTyVar $ unLoc $2 }
1158,1159c1157
<         | SIMPLEQUOTE '(' qconop ')'                  { LL $ HsTyVar (unLoc $3) }
<         | SIMPLEQUOTE '(' varop  ')'                  { LL $ HsTyVar (unLoc $3) }
---
>         | SIMPLEQUOTE var                             { LL $ HsTyVar $ unLoc $2 }

comment:17 Changed 3 years ago by simonpj

I have not reviewed in detail, but what you say sounds good.

Would someone like to update the user-manual documentation (if necessary), validate, and commit?

Simon

comment:18 Changed 3 years ago by Austin Seipp <austin@…>

In d3af9807ca8a1db0bc9298ea50895ee9df55edb7/ghc:

Really fix #5682 (parsing of promoted datacons)

Patch submitted by an anonymous friend on the bug tracker.

This also fixes TH_RichKinds2 which had a slight message output wibble
(it uses the qualified name of the promoted datacon)

Signed-off-by: Austin Seipp <austin@well-typed.com>

comment:19 Changed 3 years ago by thoughtpolice

Status: newmerge

Thanks, merged!

comment:20 Changed 3 years ago by thoughtpolice

Resolution: fixed
Status: mergeclosed

comment:21 Changed 3 years ago by Austin Seipp <austin@…>

In 5a576754d745171422d13cd1dba69dd874714cf1/ghc:

Add a test for d3af980 (#5682)

Signed-off-by: Austin Seipp <austin@well-typed.com>
Note: See TracTickets for help on using tickets.