Opened 3 years ago

Closed 3 years ago

Last modified 6 weeks ago

#9066 closed bug (fixed)

Template Haskell cannot splice an infix declaration for a data constructor

Reported by: goldfire Owned by: goldfire
Priority: normal Milestone:
Component: Template Haskell Version: 7.8.2
Keywords: Cc:
Operating System: Unknown/Multiple Architecture: Unknown/Multiple
Type of failure: None/Unknown Test Case: th/T9066
Blocked By: Blocking:
Related Tickets: Differential Rev(s): Phab:D424
Wiki Page:


When I say

  data Blargh = (:<=>) Int Int
  infix 4 :<=>

I get

    Illegal variable name: ‘:<=>’
    When splicing a TH declaration: infix 4 :<=>_0

The code inside the TH quote works when not used with TH.

I will fix in due course.

Change History (10)

comment:1 Changed 3 years ago by goldfire

This also fails for type constructors:

$( [d| type Foo a b = Either a b
       infix 5 `Foo` |])

comment:2 Changed 3 years ago by goldfire

Harrumph. In that second case, [d| infix 5 `Foo` |] produces an Exact RdrName for Foo that names a data constructor, not a type constructor, even when only the type constructor is in scope. Then, according to Note [dataTcOccs and Exact Names] in RnEnv, the Exact RdrNames are trusted to have the right namespace and, so a naive fix for this bug fails the Foo case.

There are two possible ways forward that I see:

  1. Don't trust Exact RdrNames in dataTcOccs. That is, when we have an Exact constructor name, also look for the type with same spelling.
  2. Duplicate the dataTcOccs logic in DsMeta.

I favor (2), because code that consumes the TH AST will want the TH.Names to have the right namespaces. It's really a bug that the fixity declaration above refers to a data constructor Foo.

Going to implement (2).

comment:3 Changed 3 years ago by goldfire

Well, option (2) is infeasible. This is because desugaring a quoted fixity declaration produces TH.Names that do not have namespace information attached. This is a consequence of the fact that namespace information is available only with TH.Name's NameG constructor, which also has package and module information. Of course, when processing a quote, we have no idea what package/module the declaration will eventually end up in, so NameG is a non-starter. Thus, we have no namespace information here, and instead must be liberal when processing Exact RdrNames.

I suppose the Right Way to fix this is to add namespace information to TH's NameU and NameL constructors, but that probably has farther-reaching implications than need to be dealt with at this moment.

Going to implement (1).

comment:4 Changed 3 years ago by simonpj

I'm a bit confused.

  • What does the TH syntax look like? Presumably InfixD fixity name where name :: TH.Name.
  • What is the flavour of that name? Presumably not a NameG? So NameS or NameL?
  • If NameS, we never generate an Exact RdrName, so I guess NameL?

comment:5 Changed 3 years ago by goldfire

This sample program was educational for me:

import Language.Haskell.TH.Syntax
import GHC.Exts      ( Int(I#) )
import Data.Generics ( listify )

$( do let getNames = listify (const True :: Name -> Bool)
          showNS VarName = "VarName"
          showNS DataName = "DataName"
          showNS TcClsName = "TcClsName"
          showFlav NameS = "NameS"
          showFlav (NameQ mod) = "NameQ " ++ show mod
          showFlav (NameU i)   = "NameU " ++ show (I# i)
          showFlav (NameL i)   = "NameL " ++ show (I# i)
          showFlav (NameG ns pkg mod) = "NameG " ++ showNS ns ++ " "
                                     ++ show pkg ++ " " ++ show mod

          toString (Name occ flav) = show occ ++ " (" ++ showFlav flav ++ ")"

      decs <- [d| type Foo a b = Either a b
                  infix 5 `Foo`
                  data Blargh = Foo |]

      runIO $ do
        putStr $ unlines $ map show decs
        putStrLn ""
        putStr $ unlines $ map toString $ getNames decs

      return [] )

The goal here is to learn more about the Names used in the desugaring.

Here is my output:

TySynD Foo_1627434972 [PlainTV a_1627434975,PlainTV b_1627434976] (AppT (AppT (ConT Data.Either.Either) (VarT a_1627434975)) (VarT b_1627434976))
InfixD (Fixity 5 InfixN) Foo_1627434974
InfixD (Fixity 5 InfixN) Foo_1627434972
DataD [] Blargh_1627434973 [] [NormalC Foo_1627434974 []] []

OccName "Foo" (NameU 1627434972)
OccName "a" (NameU 1627434975)
OccName "b" (NameU 1627434976)
OccName "Either" (NameG TcClsName PkgName "base" ModName "Data.Either")
OccName "a" (NameU 1627434975)
OccName "b" (NameU 1627434976)
OccName "Foo" (NameU 1627434974)
OccName "Foo" (NameU 1627434972)
OccName "Blargh" (NameU 1627434973)
OccName "Foo" (NameU 1627434974)

We see here a few things:

  • My solution (2) above is already somewhat implemented. Note that the quote has only 1 fixity declaration, but the desugared TH AST has 2! This was the essence of my idea (2) above.
  • GHC correctly notices the difference between the type Foo and the data constructor Foo in a quote.
  • All of the local names are NameUs.

These NameUs indeed become Exacts during splicing. But, the round trip from quote to TH AST to splice loses the namespace information, because NameUs do not carry namespace info. So, we either add namespace information to NameU or implement (1), above. Adding namespace info to NameU is slightly annoying, because fixity declarations are the only place that the namespace isn't apparent from a usage site.

Another possible solution is to add namespace info to the InfixD TH constructor. This is dissatisfactory because TH should model concrete syntax, and concrete syntax doesn't have a namespace marker there.

I'm happy to take suggestions, but my tendency is toward (1).

comment:6 Changed 3 years ago by goldfire

Differential Rev(s): Phab:D424
Status: newpatch

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

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

In d782694f47c3b05605e4564850623dbd03af7ecc/ghc:

Fix #9066.

When splicing in a fixity declaration, look for both term-level things
and type-level things. This requires some changes elsewhere in the
code to allow for more flexibility when looking up Exact names, which
can be assigned the wrong namespace during fixity declaration

See the ticket for more info.

comment:9 Changed 3 years ago by goldfire

Resolution: fixed
Status: patchclosed
Test Case: th/T9066

comment:10 Changed 6 weeks ago by mpickering

I don't think this was every really fixed properly and the wrinkle still exists in RnEnv. It seems that requiring namespace information on the InfixD constructor is the best way forward.

Note: See TracTickets for help on using tickets.