Opened 3 years ago

Closed 3 years ago

#5398 closed bug (invalid)

Multiple declarations of uniquely generated name

Reported by: basvandijk Owned by:
Priority: normal Milestone:
Component: Template Haskell Version: 7.0.4
Keywords: Cc:
Operating System: Unknown/Multiple Architecture: Unknown/Multiple
Type of failure: None/Unknown Difficulty:
Test Case: Blocked By:
Blocking: Related Tickets:

Description

I originally reported this problem in the syb-with-class bug tracker but I think it's a bug in TH possibly related to #5362.

After increasing the version range of template-haskell in syb-with-class to:

template-haskell >= 2.4 && < 2.7

the package fails to build on ghc-7.2.1 with the following error:

$ cabal build --ghc-option=-ddump-splices
Preprocessing library syb-with-class-0.6.1.2...
Building syb-with-class-0.6.1.2...
[4 of 4] Compiling Data.Generics.SYB.WithClass.Instances ( Data/Generics/SYB/WithClass/Instances.hs, dist/build/Data/Generics/SYB/WithClass/Instances.o )
Loading package ghc-prim ... linking ... done.
Loading package integer-gmp ... linking ... done.
Loading package base ... linking ... done.
Loading package array-0.3.0.3 ... linking ... done.
Loading package containers-0.4.1.0 ... linking ... done.
Loading package pretty-1.1.0.0 ... linking ... done.
Loading package template-haskell ... linking ... done.
Loading package bytestring-0.9.2.0 ... linking ... done.
Loading package ffi-1.0 ... linking ... done.
Data/Generics/SYB/WithClass/Instances.hs:1:1: Splicing declarations
    deriveTypeable ['DataType]
  ======>
    Data/Generics/SYB/WithClass/Instances.hs:544:3-29
    instance Typeable DataType where
        { typeOf _
            = mkTyConApp
                (mkTyCon "Data.Generics.SYB.WithClass.Basics.DataType")
                ghc-prim:GHC.Types.[] }
Data/Generics/SYB/WithClass/Instances.hs:1:1: Splicing declarations
    deriveData ['ByteString]
  ======>
    Data/Generics/SYB/WithClass/Instances.hs:726:4-28
    constr_a2Ya :: Constr
    constr_a2Ya = mkConstr dataType_a2Y9 "PS" [] Prefix
    dataType_a2Y9 :: DataType
    dataType_a2Y9 = mkDataType "ByteString" [constr_a2Ya]
    instance (Data ctx (ForeignPtr Word8),
              Data ctx Int,
              Sat (ctx ByteString),
              Sat (ctx (ForeignPtr Word8)),
              Sat (ctx Int)) =>
             Data ctx ByteString where
        { gfoldl _ _f_a2Yb z_a2Yc x_a2Yd
            = case x_a2Yd of {
                Data.ByteString.Internal.PS arg_a2Ye arg_a2Yf arg_a2Yg
                  -> _f_a2Yb
                       (_f_a2Yb
                          (_f_a2Yb (z_a2Yc Data.ByteString.Internal.PS) arg_a2Ye) arg_a2Yf)
                       arg_a2Yg }
          gunfold _ _k_a2Yh z_a2Yi c_a2Yj
            = case constrIndex c_a2Yj of {
                1 -> _k_a2Yh
                       (_k_a2Yh (_k_a2Yh (z_a2Yi Data.ByteString.Internal.PS)))
                _ -> error "gunfold: fallthrough" }
          toConstr _ x_a2Yk
            = case x_a2Yk of {
                Data.ByteString.Internal.PS _ _ _ -> constr_a2Ya }
          dataTypeOf _ _ = dataType_a2Y9 }
Data/Generics/SYB/WithClass/Instances.hs:1:1: Splicing declarations
    deriveData ['L.ByteString]
  ======>
    Data/Generics/SYB/WithClass/Instances.hs:727:4-30
    constr_a30f :: Constr
    constr_a30f = mkConstr dataType_a30e "Empty" [] Prefix
    constr_a30g :: Constr
    constr_a30g = mkConstr dataType_a30e "Chunk" [] Prefix
    dataType_a30e :: DataType
    dataType_a30e = mkDataType "ByteString" [constr_a30f, constr_a30g]
    instance (Data ctx ByteString,
              Data ctx L.ByteString,
              Sat (ctx L.ByteString),
              Sat (ctx ByteString)) =>
             Data ctx L.ByteString where
        { gfoldl _ _f_a30h z_a30i x_a30j
            = case x_a30j of {
                Data.ByteString.Lazy.Internal.Empty
                  -> z_a30i Data.ByteString.Lazy.Internal.Empty
                Data.ByteString.Lazy.Internal.Chunk arg_a30k arg_a30l
                  -> _f_a30h
                       (_f_a30h (z_a30i Data.ByteString.Lazy.Internal.Chunk) arg_a30k)
                       arg_a30l }
          gunfold _ _k_a30m z_a30n c_a30o
            = case constrIndex c_a30o of {
                1 -> z_a30n Data.ByteString.Lazy.Internal.Empty
                2 -> _k_a30m (_k_a30m (z_a30n Data.ByteString.Lazy.Internal.Chunk))
                _ -> error "gunfold: fallthrough" }
          toConstr _ x_a30p
            = case x_a30p of {
                Data.ByteString.Lazy.Internal.Empty -> constr_a30f
                Data.ByteString.Lazy.Internal.Chunk _ _ -> constr_a30g }
          dataTypeOf _ _ = dataType_a30e }

Data/Generics/SYB/WithClass/Instances.hs:727:4:
    Multiple declarations of `dataType'
    Declared at: Data/Generics/SYB/WithClass/Instances.hs:726:4
                 Data/Generics/SYB/WithClass/Instances.hs:727:4

Data/Generics/SYB/WithClass/Instances.hs:727:4:
    Multiple declarations of `constr'
    Declared at: Data/Generics/SYB/WithClass/Instances.hs:726:4
                 Data/Generics/SYB/WithClass/Instances.hs:727:4
                 Data/Generics/SYB/WithClass/Instances.hs:727:4

The weird thing is that the generated names dataType_a2Y9, dataType_a30e, constr_a2Ya and constr_a30 are all unique.

Change History (6)

comment:1 follow-up: Changed 3 years ago by simonpj

Sigh. See #5375, especially the first comment in the sequence.

I can see that what you want is reinerp's "totally fresh" semantics, and I can see why: the functions you need are local to that particular splice, and will never be referenced outside it.

I'm beginning to wonder whether I should recant on #5037. That is, perhaps #5037 wasn't a bug at all, and I was too eager to "fix" it.

To reprise, the issue is this. Consider

foo = True

bar1 = $([| \foo -> $( return (VarE 'foo)) |])
bar2 = $(do { foo <- newName "foo"
            ; return (LamE (VarP foo) (VarE foo)) })
  -- Both expand to (\foo -> foo)

bar3 = $([| \foo -> $( return (VarE (mkName "foo") ) |])
bar4 = $(do { foo <- newName "foo"
            ; return (LamE (VarP foo) (VarE (mkName "foo"))) })

Here, bar1 and bar2 do the same thing; its just that bar uses quotation syntax to generate the lambda and the fresh name, whereas bar2 does the same thing by steam using newName.

But what about bar3 and bar4? Again they are equivalant, except that bar4 does things by steam. But:

  • PLAN A: They generate (\foo34 -> foo), where the foo in the body of the lambda binds to the top-level foo. That is, the (mkName "foo") is not captured by a binding of a variable built with newName.
  • PLAN B: They generate (\foo -> foo), where the "foo" variable in the body of the lambda is captured by the lambda-binding.

GHC 7.0 picked PLAN A. #5037 said that was a bug: the (mkName "foo") should be captured by a variable generated with newName. So GHC 7.2 uses PLAN B.

A consequence of PLAN A is that this won't work:

$[| x = True |]    -- Under Plan A, this expands to  x34 = True
z1 = x             -- BAD: x not in scope


$(do { y <- newName "y"
     ; return [FunD y [...clause for True....]] })
   -- Under Plan A, tihs expands to   y45 = True
z2 = y    -- BAD: y not in scope

A reasonable person would expect that the use of "x" in the definition of "z1" would bind to the binding of "x" in the preceding splice; but under PLAN an A it doesn't because the splice binds a nice fresh name like "x45". The second splice plus definition of z2 shows what the first one desugars to, and should behave in precisely the same way.

You could make this work by using mkName "x" for the binders, but that is unreasonably clumsy. (Incidentally this did work in GHC 7.0, even though GHC 7.0 used PLAN A, but only because of an egregious HACK -- using mkName rather than newName when desugaring top-level binder in declaration quotes. Not only was this a hack, but it cuased a bug: #5379.)

I'm having trouble seeing a good path through this thicket. Maybe someone less buried in details can help. Interested parties please comment. Examples are useful.

Simon

comment:2 Changed 3 years ago by basvandijk

Simon, this bug is now fixed upstream. Do you want to leave this one open?

comment:3 Changed 3 years ago by simonpj

Yes, there's an open design question, and I'm really not sure what the best design is. So let's leave it open.

comment:4 in reply to: ↑ 1 Changed 3 years ago by Saizan

Replying to simonpj:

[...]
A consequence of PLAN B is that this won't work:

Did you mean PLAN A here?

comment:5 Changed 3 years ago by simonpj

Yes, you are right, thank you. I have edited the original text, and elaborated it slightly

comment:6 Changed 3 years ago by simonpj

  • Resolution set to invalid
  • Status changed from new to closed

We had a discussion at CamHac which concluded that PLAN B is indeed the right one. A principled reason for this is the identity

   $[| e |] = e

For this to hold we need that

   $[| \x -> $(return (VarE (mkName "x"))) |]
= 
   \x -> $(return (VarE (mkName "x")))

In the latter, the (mkName "x") must clearly bind to the lambda. And so it must do so in the former too.

Concerning the SYB story, you just have to generate fresh names for the top level bindings. You can do that by name-mangling the type involved (which is I believe how you fixed it) -- but it's also possible to write a function

mangleName :: Name -> Name
mangleName name@(Name occ fl) 
  = case fl of
      NameU u -> Name (mangle_occ occ u) fl
      _       -> name
  where
    mangle_occ :: OccName -> Int# -> OccName
    mangle_occ occ uniq = mkOccName (occNameString occ ++ "-" ++ show (I# uniq))

This will take a Name built with newName (which generates a NameU), and incorporate the unique in the string-name of the Name.

It might even be worth adding this to TH's API, but I'll wait to see how useful it is first.

It's also worth noting that what you really wanted for SYB was a kind of anonymous local module. You really want your splice to expand thus:

  $(derive 'T)
=======>  expands to
  module () where
     constr = ...
     dataType  ...
     instance (..blah..) => Data ctx ByteString where
         ...constr...dataType...

so that the definitions of constr and dataType are not visible in the outer scope.

But that is another story! I'll open a ticket for it, and close this as invalid.

Simon

Note: See TracTickets for help on using tickets.