Opened 5 years ago

Last modified 5 years ago

#10027 new bug

Importing constructor of associated data type fails

Reported by: lspitzner Owned by:
Priority: normal Milestone:
Component: Compiler Version: 7.8.4
Keywords: constructor import associated data type Cc:
Operating System: Unknown/Multiple Architecture: Unknown/Multiple
Type of failure: None/Unknown Test Case:
Blocked By: Blocking:
Related Tickets: Differential Rev(s):
Wiki Page:

Description

I will base my description on the code you find when searching for "instance PersistEntity" on http://www.yesodweb.com/book/persistent. In summary, the code looks like

instance PersistEntity Person where
...
    data EntityField Person typ where
        PersonId   :: EntityField Person PersonId
        PersonName :: EntityField Person String
        PersonAge  :: EntityField Person Int
...

This code is generated from TH (see above). I have not tested if it matters that it is generated. A first step could be to test if manually replacing the TH part yields different results.

The problem is: When i add this code in one module, do not create an export list for that module, and try to import the constructors explicitly (i.e. with a list) in a different module, it does not work.

I read and tried to follow the description on https://wiki.haskell.org/GHC/Type_families, chapter 4.3.

Testcase 1

(not really expected to work; still intersting error message)

import MyModuleContainingInstancePerson ( PersonId )

ghc says

      ‘PersonId’ is a data constructor of ‘EntityField’
    To import it use
      ‘import’ MyModuleContainingInstancePerson( EntityField( VersionVersion ) )
    or
      ‘import’ MyModuleContainingInstancePerson( EntityField(..) )

Testcase 2

(no export list in MyModuleContainingInstancePerson)

import MyModuleContainingInstancePerson ( EntityField(..) )

ghc says

Module ‘MyModuleContainingInstancePerson’ does not export ‘EntityField(..)’

Testcase 3

But when i do provide an export list in MyModuleContainingInstancePerson, like

module MyModuleContainingInstancePerson ( Person(..), EntityField(..) ) where ..

Testcase 2 suddenly works.

Change History (8)

comment:1 Changed 5 years ago by simonpj

Very good point. The same happens with this simpler test case:

module A where
  data family D a

module B where
  import A
  data Foo = Foo
  data instance D Foo = DCon

module C where
  import B( D( DCon ) )  -- This import is rejected

Compilation of C fails because the import is rejected.

It's rejected because (GHC considers that) B really doesn't export D: the family D is defined in module A. So module B exports only Foo(Foo) and the data constructor DCon, but not the data family D.

Alas, that means that there is simply no way to import DCon explicitly. Not very clever, but that's the way it is.

This does seem wrong. The issue is: what is exported if you omit an explicit export list, and just say module M where ...? The Haskell Report says "If the export list is omitted, all values, types and classes defined in the module are exported, but not those that are imported". But it was written without thinking of type families.

So I suppose we could agree that, for the purposes of an omitted export list, a data instance is considered as defining the data family too, so that the data family is exported as well as the data constructors. Otherwise there is literally no way to import the data constructor by name. And really it is odd to have data constructors in scope without their type constructor also being in scope.

An alterantive would be to extend the use of -XExplicitNamespaces (see user manual) to allow an import to specify data constructor. Thus

import B( datacon DCon )

That could become tiresome for data types with lots of constructors.

Any opinions?

Simon

comment:2 Changed 5 years ago by goldfire

Don't we already have a herald for data constructors? I thought it was spelled pattern. I'm not being cheeky -- I seem to recall a proposal (current status/location unknown to me -- do you know?) to allow direct export/import of data constructors without their types via the pattern herald. That would seem to cover this case nicely, and this could be a motivating example for that proposal.

With the pattern proposal implemented, I have only mild support for Simon's suggestion of exporting the data family along with any data instances. Would this carry over to type families too (I vote: no)? What if a module declares a constructor-less data instance? Does that by itself provoke re-export of the data family?

comment:3 Changed 5 years ago by carter

I think i was hit by a related problem this fall, though I was treating it as a possible haddock problem rather than GHC problem. https://github.com/haskell/haddock/issues/332, might that be related? The visibility of the data family constructors etc wrt how haddock displays does tie into this ticket i believe. Thoughts?

comment:4 Changed 5 years ago by simonpj

Re comment:2, yes, you are right, I'd forgotten that. The remaining infelicity is that if the data type has 200 constructors, you'll have to say

import M( pattern C1, pattern C2, ..., pattern C200 )

rather than

import M( T(..) )

But perhaps this will happen so seldom that we don't care.

Partly it's a question of matching is it surprising that you module B above does not export D? Or would it be surprising if it did export it?

comment:5 Changed 5 years ago by goldfire

Personally, I would find it surprising if module B did export D. I don't consider writing an instance of a thing as something that will add the thing to an implicit export list. I'm OK with being surprised in this way for the sake of practicality, but I would find it surprising.

comment:6 Changed 5 years ago by lspitzner

The case without an export-list feels special-casy, but if i am not mistaken the problem is not restricted to it: It is impossible to explicitly export data constructors without exporting the data family as well:

module A where
  data family D a

module B
  ( D ( DCon ) -- cannot not export D
  )
where
  import A
  data Foo = Foo
  data instance D Foo = DCon

To me, the core of the issue seems to be that "D(DCon)" refers to both D and D.DCon. Would it not make sense to weaken this? E.g. replace the relevant section in report2010:5.2 with:

  • The form T names the type but not the constructors or field names.
  • The form T(c1,…,cn), names the sum of:

a) the type, but only if it is declared locally, b) the mentioned constructors or field names, which may be an empty list

  • The abbreviated form T(..) names the same as T(c1,…,cn) where c1,…,cn is the full list of constructors/field names currently in scope (qualified or not)

(i sneaked the "which may be an empty list" fix in; this seems to match what is implemented)

This approach would next need to define how importing is handled, as "declared locally" does not make sense on import-side. I would propose to give it "grab all you can" semantics, i.e.

import B ( D ( DCon ) ) imports DCon, and D if it is exported by B.

Examples for the export:

module B
  ( D ( DCon ) -- only D.DCon
  )

module B
  ( D          -- re-export the data family
  , D ( DCon ) -- D.DCon
  )

With this, the no-export-list case becomes trivial.

How high would the risk of this breaking existing code be? Are there other disadvantages?

comment:7 in reply to:  6 Changed 5 years ago by goldfire

Replying to lspitzner:

How high would the risk of this breaking existing code be?

I think it's quite high. I believe it's common practice to define datatypes in a package-internal module and then re-export them in a package-visible module. (Indeed, I do this in several of my released packages!) With the change as proposed, those types would not be re-exported, leading to chaos.

But it seems that you can always import/export data constructors using the pattern herald in an import/export list. It seems the remaining bit is to come up with a syntax to use in case there are lots of constructors and writing out each one is annoying.

Or have I missed something?

comment:8 Changed 5 years ago by simonpj

Putting comment:6 and comment:7 together, maybe

   pattern T( C1, C2, C3 )

exports only data constructors C1, C2, C3, but not T. And then

  pattern T(..) 

would export all the constructors, but not T.

Another alternative

  pattern C..

exports all C and all the other peer constructor from C's data type.

Note: See TracTickets for help on using tickets.