[GHC] #15815: problem with splicing type into constraint
#15815: problem with splicing type into constraint -------------------------------------+------------------------------------- Reporter: int-e | Owner: (none) Type: bug | Status: new Priority: normal | Milestone: Component: Template | Version: 8.6.1 Haskell | Keywords: | Operating System: Unknown/Multiple Architecture: | Type of failure: GHC rejects Unknown/Multiple | valid program Test Case: | Blocked By: Blocking: | Related Tickets: Differential Rev(s): | Wiki Page: -------------------------------------+------------------------------------- Consider the following two-module example. (as gist: https://gist.github.com/int-e/a666991423c10150bd99dd0e874d6150) {{{#!hs {-# LANGUAGE TemplateHaskell #-} module A where mkFoo tyQ = [d| foo :: a ~ $(tyQ) => a foo = undefined |] }}} {{{#!hs {-# LANGUAGE TemplateHaskell, GADTs #-} module B where import A mkFoo [t| Int -> Int |] }}} This loads fine in ghc-8.4.2, but with ghc-8.6.1 and current head (commit 578012be13eb1548050d51c0a23bd1a98423f03e), the splice goes wrong: {{{ $ ghci B.hs GHCi, version 8.6.1: http://www.haskell.org/ghc/ :? for help [1 of 2] Compiling A ( A.hs, interpreted ) [2 of 2] Compiling B ( B.hs, interpreted ) B.hs:7:1: error: • Expected a type, but ‘a_a4uD ~ Int’ has kind ‘Constraint’ • In the type signature: foo :: a_a4uD ~ Int -> Int => a_a4uD | 7 | mkFoo [t| Int -> Int |] | ^^^^^^^^^^^^^^^^^^^^^^^ Failed, one module loaded. *A> }}} As a workaround one can define a type alias for the `Int -> Int` type. -- Ticket URL: http://ghc.haskell.org/trac/ghc/ticket/15815 GHC http://www.haskell.org/ghc/ The Glasgow Haskell Compiler
#15815: problem with splicing type into constraint -------------------------------------+------------------------------------- Reporter: int-e | Owner: (none) Type: bug | Status: new Priority: normal | Milestone: Component: Template Haskell | Version: 8.6.1 Resolution: | Keywords: Operating System: Unknown/Multiple | Architecture: Type of failure: GHC rejects | Unknown/Multiple valid program | Test Case: Blocked By: | Blocking: Related Tickets: | Differential Rev(s): Wiki Page: | -------------------------------------+------------------------------------- Description changed by int-e: Old description:
Consider the following two-module example. (as gist: https://gist.github.com/int-e/a666991423c10150bd99dd0e874d6150) {{{#!hs {-# LANGUAGE TemplateHaskell #-} module A where
mkFoo tyQ = [d| foo :: a ~ $(tyQ) => a foo = undefined |] }}}
{{{#!hs {-# LANGUAGE TemplateHaskell, GADTs #-} module B where
import A
mkFoo [t| Int -> Int |] }}}
This loads fine in ghc-8.4.2, but with ghc-8.6.1 and current head (commit 578012be13eb1548050d51c0a23bd1a98423f03e), the splice goes wrong: {{{ $ ghci B.hs GHCi, version 8.6.1: http://www.haskell.org/ghc/ :? for help [1 of 2] Compiling A ( A.hs, interpreted ) [2 of 2] Compiling B ( B.hs, interpreted )
B.hs:7:1: error: • Expected a type, but ‘a_a4uD ~ Int’ has kind ‘Constraint’ • In the type signature: foo :: a_a4uD ~ Int -> Int => a_a4uD | 7 | mkFoo [t| Int -> Int |] | ^^^^^^^^^^^^^^^^^^^^^^^ Failed, one module loaded. *A> }}}
As a workaround one can define a type alias for the `Int -> Int` type.
New description: Consider the following two-module example. (as gist: https://gist.github.com/int-e/a666991423c10150bd99dd0e874d6150) {{{#!hs {-# LANGUAGE TemplateHaskell #-} module A where mkFoo tyQ = [d| foo :: a ~ $(tyQ) => a foo = undefined |] }}} {{{#!hs {-# LANGUAGE TemplateHaskell, GADTs #-} module B where import A mkFoo [t| Int -> Int |] }}} This loads fine in ghc-8.4.2, but with ghc-8.6.1 and current head (commit 23956b2ada690c78a134fe6d149940c777c7efcc), the splice goes wrong: {{{ $ ghci B.hs GHCi, version 8.6.1: http://www.haskell.org/ghc/ :? for help [1 of 2] Compiling A ( A.hs, interpreted ) [2 of 2] Compiling B ( B.hs, interpreted ) B.hs:7:1: error: • Expected a type, but ‘a_a4uD ~ Int’ has kind ‘Constraint’ • In the type signature: foo :: a_a4uD ~ Int -> Int => a_a4uD | 7 | mkFoo [t| Int -> Int |] | ^^^^^^^^^^^^^^^^^^^^^^^ Failed, one module loaded. *A> }}} As a workaround one can define a type alias for the `Int -> Int` type. -- -- Ticket URL: http://ghc.haskell.org/trac/ghc/ticket/15815#comment:1 GHC http://www.haskell.org/ghc/ The Glasgow Haskell Compiler
#15815: problem with splicing type into constraint -------------------------------------+------------------------------------- Reporter: int-e | Owner: RyanGlScott Type: bug | Status: new Priority: highest | Milestone: Component: Template Haskell | Version: 8.6.1 Resolution: | Keywords: Operating System: Unknown/Multiple | Architecture: Type of failure: GHC rejects | Unknown/Multiple valid program | Test Case: Blocked By: | Blocking: Related Tickets: | Differential Rev(s): Wiki Page: | -------------------------------------+------------------------------------- Changes (by RyanGlScott): * owner: (none) => RyanGlScott * priority: normal => highest Comment: This appears to be my fault, since this regression was introduced in commit b9483981d128f55d8dae3f434f49fa6b5b30c779 (`Remove HsEqTy and XEqTy`). I'll take a look. -- Ticket URL: http://ghc.haskell.org/trac/ghc/ticket/15815#comment:2 GHC http://www.haskell.org/ghc/ The Glasgow Haskell Compiler
#15815: problem with splicing type into constraint -------------------------------------+------------------------------------- Reporter: int-e | Owner: RyanGlScott Type: bug | Status: new Priority: highest | Milestone: Component: Template Haskell | Version: 8.6.1 Resolution: | Keywords: Operating System: Unknown/Multiple | Architecture: Type of failure: GHC rejects | Unknown/Multiple valid program | Test Case: Blocked By: | Blocking: Related Tickets: | Differential Rev(s): Wiki Page: | -------------------------------------+------------------------------------- Comment (by RyanGlScott): Simpler example that only requires one module: {{{#!hs {-# LANGUAGE GADTs #-} {-# LANGUAGE TemplateHaskell #-} module Bug where $([d| foo :: a ~ (Int -> Int) => a foo = undefined |]) }}} {{{ $ /opt/ghc/8.4.4/bin/ghc Bug.hs [1 of 1] Compiling Bug ( Bug.hs, Bug.o ) $ /opt/ghc/8.6.1/bin/ghc Bug.hs [1 of 1] Compiling Bug ( Bug.hs, Bug.o ) Bug.hs:5:3: error: • Expected a type, but ‘a_a44c ~ Int’ has kind ‘Constraint’ • In the type signature: foo :: a_a44c ~ Int -> Int => a_a44c | 5 | $([d| foo :: a ~ (Int -> Int) => a | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^... }}} Or, using fewer quotes: {{{#!hs {-# LANGUAGE GADTs #-} {-# LANGUAGE TemplateHaskell #-} module Bug where import Language.Haskell.TH $(do foo <- newName "foo" a <- newName "a" pure [ SigD foo (ForallT [] [AppT (AppT EqualityT (VarT a)) (AppT (AppT ArrowT (ConT ''Int)) (ConT ''Int))] (VarT a)) , ValD (VarP foo) (NormalB (VarE 'undefined)) [] ]) }}} {{{ $ /opt/ghc/8.4.4/bin/ghc Bug.hs [1 of 1] Compiling Bug ( Bug.hs, Bug.o ) $ /opt/ghc/8.6.1/bin/ghc Bug.hs [1 of 1] Compiling Bug ( Bug.hs, Bug.o ) Bug.hs:7:3: error: • Expected a type, but ‘a_a452 ~ Int’ has kind ‘Constraint’ • In the type signature: foo :: a_a452 ~ Int -> Int => a_a452 | 7 | $(do foo <- newName "foo" | ^^^^^^^^^^^^^^^^^^^^^^^... }}} -- Ticket URL: http://ghc.haskell.org/trac/ghc/ticket/15815#comment:3 GHC http://www.haskell.org/ghc/ The Glasgow Haskell Compiler
#15815: problem with splicing type into constraint -------------------------------------+------------------------------------- Reporter: int-e | Owner: RyanGlScott Type: bug | Status: new Priority: highest | Milestone: Component: Template Haskell | Version: 8.6.1 Resolution: | Keywords: Operating System: Unknown/Multiple | Architecture: Type of failure: GHC rejects | Unknown/Multiple valid program | Test Case: Blocked By: | Blocking: Related Tickets: #15760 | Differential Rev(s): Wiki Page: | -------------------------------------+------------------------------------- Changes (by RyanGlScott): * cc: goldfire (added) * related: => #15760 Comment: I believe I understand what is happening here. The problem is that when you roundtrip the following declaration through Template Haskell: {{{#!hs $([d| foo :: a ~ (Int -> Int) => a foo = undefined |]) }}} Then all parentheses are stripped away when converting this to the GHC AST. This has important consequences when processing `foo`'s type signature. Before the offending commit (that I linked to in comment:2), the context of `foo`'s type signature would become: {{{ HsEqTy a (HsOpTy Int (->) Int) }}} But after the offending commit, this becomes: {{{ HsOpTy a (~) (HsOpTy Int (->) Int) }}} Upon first glance, these would appear to be identical. But as it turns out, `HsEqTy` actually had special treatment in the renamer, which means that it was renamed as though one had implicitly added `HsParTy`s around both of its arguments. On the other hand, when GHC renaming sequences of `HsOpTy`s (as in the second example), no such thing happens. In essence, that AST corresponds to: {{{#!hs a ~ Int -> Int }}} GHC thinks that should be parenthesized as: {{{#!hs (a ~ Int) -> Int }}} Which leads to the error that we see in this ticket. A quick-and-dirty way to fix this would be to change `cvtTypeKind` such that when we convert an `EqualityT`, we parenthesize `~`'s arguments if they aren't parenthesized. In other words, apply this patch: {{{#!diff diff --git a/compiler/hsSyn/Convert.hs b/compiler/hsSyn/Convert.hs index 8b12a78..74a6011 100644 --- a/compiler/hsSyn/Convert.hs +++ b/compiler/hsSyn/Convert.hs @@ -1437,7 +1437,9 @@ cvtTypeKind ty_str ty EqualityT | [x',y'] <- tys' -> - returnL (HsOpTy noExt x' (noLoc eqTyCon_RDR) y') + let px = parenthesizeHsType opPrec x' + py = parenthesizeHsType opPrec y' + in returnL (HsOpTy noExt px (noLoc eqTyCon_RDR) py) | otherwise -> mk_apps (HsTyVar noExt NotPromoted (noLoc eqTyCon_RDR)) tys' }}} That makes the original program compile again, although it's just applying a bandage over a deeper wound—namely, that TH conversion strips away parentheses in the first place. goldfire, weren't you looking into fixing the parentheses issue in #15760? If so, perhaps your patch there would make for a more elegant fix for this ticket. On the other hand, if that still needs some work, I could whip up a stopgap solution now (based on the code above) so that this could be backported to a patch release if necessary. -- Ticket URL: http://ghc.haskell.org/trac/ghc/ticket/15815#comment:4 GHC http://www.haskell.org/ghc/ The Glasgow Haskell Compiler
#15815: problem with splicing type into constraint -------------------------------------+------------------------------------- Reporter: int-e | Owner: RyanGlScott Type: bug | Status: new Priority: highest | Milestone: Component: Template Haskell | Version: 8.6.1 Resolution: | Keywords: Operating System: Unknown/Multiple | Architecture: Type of failure: GHC rejects | Unknown/Multiple valid program | Test Case: Blocked By: | Blocking: Related Tickets: #15760 | Differential Rev(s): Wiki Page: | -------------------------------------+------------------------------------- Changes (by int-index): * cc: int-index (added) Comment: Ryan, why do you believe that preserving parentheses would solve this issue? The original code does not have any parentheses (unlike your minimised version). The issue here appears to be manyfold 1. We reassociate type operators according to their fixities, so with `a ~ $(tyQ)` and `[t| Int -> Int |]` from the original report we get `a ~ Int -> Int` (no parens!) which gets correctly associated as `(a ~ Int) -> Int`. That's not the behaviour I would expect, but I've just learned that it's documented in `Note [Converting UInfix]`. 2. However, we map all three of `(~) a b`, `a ~ b`, and `(a ~ b)` to `AppT (AppT EqualityT a) b`. So we discard both parenthesization information (as you noted) and whether equality was applied in an infix or a prefix fashion. 3. When we convert back to `HsType`, we use `HsOpTy` if there are two operands, even if `(~)` was prefix in the original code. This means that even if we fix #15760, we will still get incorrect behaviour in the `(~) a b` case. For expressions, we have a distinction between `UInfixE` and `InfixE`: * `UInfixE` gets reassociated as necessary (documented in `See Note [Operator association]`) * `InfixE` gets parenthesised as necessary (documented in `Note [Converting UInfix]`) In types we have a similar distinction – `UInfixT` and `InfixT`. The former must get reassociated, and the latter parenthesised. NB. Looking at the code that handles `InfixT` it appears broken to me (because unlike code for expressions, it does not call `parenthesizeHsType`). But this is either a bug or I'm missing something. For the sake of the argument let's assume `InfixT` is treated the same way as `InfixE` – by parenthesising arguments as necessary. The question is: do we treat `AppT (AppT (EqualityT a)) b` as a moral equivalent of `UInfixT` or `InfixT`? The bug seems to be that we used to treat it as `InfixT`, now we treat it as `UInfixT`. Your patch with adding `parenthesizeHsType` seems to make it treated like `InfixT` again, but ideally the fix should be * to stop confusing `(~) a b` and `a ~ b`: map the former to `AppT (AppT (EqualityT a)) b` and the latter to `InfixT a EqualityT b` * to fix the treatment of `InfixT` in `cvtTypeKind` – parenthesise as necessary in expressions * drop this silly special case: {{{ | [x',y'] <- tys' -> returnL (HsOpTy noExt x' (noLoc eqTyCon_RDR) y') }}} -- Ticket URL: http://ghc.haskell.org/trac/ghc/ticket/15815#comment:5 GHC http://www.haskell.org/ghc/ The Glasgow Haskell Compiler
#15815: problem with splicing type into constraint -------------------------------------+------------------------------------- Reporter: int-e | Owner: RyanGlScott Type: bug | Status: new Priority: highest | Milestone: 8.6.2 Component: Template Haskell | Version: 8.6.1 Resolution: | Keywords: Operating System: Unknown/Multiple | Architecture: Type of failure: GHC rejects | Unknown/Multiple valid program | Test Case: Blocked By: | Blocking: Related Tickets: | Differential Rev(s): Wiki Page: | -------------------------------------+------------------------------------- Changes (by RyanGlScott): * cc: goldfire (removed) * related: #15760 => * milestone: => 8.6.2 Comment: You're right, I misspoke about #15760 fixing this issue. Forget I said anything about that. Your fix smells correct to me, and if there weren't any other extenuating circumstances, I'd endorse it as the one true solution. Unfortunately, there's a bit of a thorny issue in that this code no longer works on GHC 8.6, and I don't see a simple way to work around the issue at the moment. We need to backport //something// to fix this, but the question is //what//. Alas, changing the way we desugar infix types to use `InfixT` would constitute a breaking change in practice, so I'm reluctant to backport that. There is quite a bit of code in the wild which, for better or worse, assumes that `InfixT` only ever appears in user written code (i.e., `InfixT` never appears in desugared or reified TH ASTs). See [https://github.com/glguy/th- abstraction/blob/5123c6d054d0949cb444590269f13e5d44699ab2/src/Language/Haskell/TH/Datatype.hs#L1156-L1160 this function] from `th-abstraction` as one example. I'm not sure what exactly would happen if we started feeding that function `InfixT`s, but I imagine that something or another would change at runtime, which would be awful for a point release. Therefore, I'm inclined to adopt the patch in comment:4 as a crude (and ideally, temporary) fix for this issue in an 8.6 point release (hopefully 8.6.2), and work towards implementing your more robust solution for 8.8. Does that sound reasonable? -- Ticket URL: http://ghc.haskell.org/trac/ghc/ticket/15815#comment:6 GHC http://www.haskell.org/ghc/ The Glasgow Haskell Compiler
#15815: problem with splicing type into constraint -------------------------------------+------------------------------------- Reporter: int-e | Owner: RyanGlScott Type: bug | Status: new Priority: highest | Milestone: 8.6.2 Component: Template Haskell | Version: 8.6.1 Resolution: | Keywords: Operating System: Unknown/Multiple | Architecture: Type of failure: GHC rejects | Unknown/Multiple valid program | Test Case: Blocked By: | Blocking: Related Tickets: | Differential Rev(s): Wiki Page: | -------------------------------------+------------------------------------- Comment (by int-index): Sounds reasonable to me, although I'm unsure if we should target 8.8 for the more robust fix rather than 8.10. When #15760 is fixed, `decomposeType` in `th-abstraction` that you linked would have to start to care about parentheses as well, so I believe it's better to do both changes at once. And since the merge window for 8.8 closes in a couple of days, and these are breaking changes, then a more realistic target is probably 8.10 – what do you think? -- Ticket URL: http://ghc.haskell.org/trac/ghc/ticket/15815#comment:7 GHC http://www.haskell.org/ghc/ The Glasgow Haskell Compiler
When #15760 is fixed, `decomposeType` in `th-abstraction` that you
#15815: problem with splicing type into constraint -------------------------------------+------------------------------------- Reporter: int-e | Owner: RyanGlScott Type: bug | Status: new Priority: highest | Milestone: 8.6.2 Component: Template Haskell | Version: 8.6.1 Resolution: | Keywords: Operating System: Unknown/Multiple | Architecture: Type of failure: GHC rejects | Unknown/Multiple valid program | Test Case: Blocked By: | Blocking: Related Tickets: | Differential Rev(s): Wiki Page: | -------------------------------------+------------------------------------- Comment (by RyanGlScott): Replying to [comment:7 int-index]: linked would have to start to care about parentheses as well Egads, I hadn't thought about that. That's a very good point, and I agree that it would be best to try and ship these fixes together.
And since the merge window for 8.8 closes in a couple of days
Wow. Really? That deadline crept up faster than I ever thought possible...
then a more realistic target is probably 8.10 – what do you think?
Sure. I'd be fine with putting this off later than 8.8 if need be (be it 8.10, 8.12, or whenever). -- Ticket URL: http://ghc.haskell.org/trac/ghc/ticket/15815#comment:8 GHC http://www.haskell.org/ghc/ The Glasgow Haskell Compiler
#15815: problem with splicing type into constraint -------------------------------------+------------------------------------- Reporter: int-e | Owner: RyanGlScott Type: bug | Status: patch Priority: highest | Milestone: 8.6.2 Component: Template Haskell | Version: 8.6.1 Resolution: | Keywords: Operating System: Unknown/Multiple | Architecture: Type of failure: GHC rejects | Unknown/Multiple valid program | Test Case: Blocked By: | Blocking: Related Tickets: | Differential Rev(s): Phab:D5274 Wiki Page: | -------------------------------------+------------------------------------- Changes (by RyanGlScott): * status: new => patch * differential: => Phab:D5274 Comment: I've submitted Phab:D5274 with the short-term fix from comment:4 (that ought to be backported to 8.6.2). int-index, can you open a new ticket tracking the long-term goal of desugaring/reifying uses of infix types to `InfixT`? -- Ticket URL: http://ghc.haskell.org/trac/ghc/ticket/15815#comment:9 GHC http://www.haskell.org/ghc/ The Glasgow Haskell Compiler
Wow. Really? That deadline crept up faster than I ever thought
#15815: problem with splicing type into constraint -------------------------------------+------------------------------------- Reporter: int-e | Owner: RyanGlScott Type: bug | Status: patch Priority: highest | Milestone: 8.6.2 Component: Template Haskell | Version: 8.6.1 Resolution: | Keywords: Operating System: Unknown/Multiple | Architecture: Type of failure: GHC rejects | Unknown/Multiple valid program | Test Case: Blocked By: | Blocking: Related Tickets: | Differential Rev(s): Phab:D5274 Wiki Page: | -------------------------------------+------------------------------------- Comment (by int-index): possible... Sorry to mislead – I thought "cut release branch in November 2018" meant November 1st, turns out it's November 18th, so there are three weeks left.
int-index, can you open a new ticket tracking the long-term goal of desugaring/reifying uses of infix types to `InfixT`?
I created #15824. -- Ticket URL: http://ghc.haskell.org/trac/ghc/ticket/15815#comment:10 GHC http://www.haskell.org/ghc/ The Glasgow Haskell Compiler
#15815: problem with splicing type into constraint
-------------------------------------+-------------------------------------
Reporter: int-e | Owner: RyanGlScott
Type: bug | Status: patch
Priority: highest | Milestone: 8.6.2
Component: Template Haskell | Version: 8.6.1
Resolution: | Keywords:
Operating System: Unknown/Multiple | Architecture:
Type of failure: GHC rejects | Unknown/Multiple
valid program | Test Case:
Blocked By: | Blocking:
Related Tickets: | Differential Rev(s): Phab:D5274
Wiki Page: |
-------------------------------------+-------------------------------------
Comment (by Ben Gamari
#15815: problem with splicing type into constraint -------------------------------------+------------------------------------- Reporter: int-e | Owner: RyanGlScott Type: bug | Status: merge Priority: highest | Milestone: 8.6.2 Component: Template Haskell | Version: 8.6.1 Resolution: | Keywords: Operating System: Unknown/Multiple | Architecture: Type of failure: GHC rejects | Unknown/Multiple valid program | Test Case: th/T15815 Blocked By: | Blocking: Related Tickets: | Differential Rev(s): Phab:D5274 Wiki Page: | -------------------------------------+------------------------------------- Changes (by RyanGlScott): * testcase: => th/T15815 * status: patch => merge -- Ticket URL: http://ghc.haskell.org/trac/ghc/ticket/15815#comment:12 GHC http://www.haskell.org/ghc/ The Glasgow Haskell Compiler
#15815: problem with splicing type into constraint -------------------------------------+------------------------------------- Reporter: int-e | Owner: RyanGlScott Type: bug | Status: closed Priority: highest | Milestone: 8.6.2 Component: Template Haskell | Version: 8.6.1 Resolution: fixed | Keywords: Operating System: Unknown/Multiple | Architecture: Type of failure: GHC rejects | Unknown/Multiple valid program | Test Case: th/T15815 Blocked By: | Blocking: Related Tickets: | Differential Rev(s): Phab:D5274 Wiki Page: | -------------------------------------+------------------------------------- Changes (by bgamari): * status: merge => closed * resolution: => fixed Comment: Merged to `ghc-8.6` with 2567e8f341ef638b8a93054d1be75c176bfaee66. -- Ticket URL: http://ghc.haskell.org/trac/ghc/ticket/15815#comment:13 GHC http://www.haskell.org/ghc/ The Glasgow Haskell Compiler
participants (1)
-
GHC