Importing constructor of associated data type fails

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.

Trac metadata

Trac field	Value
Version	7.8.4
Type	Bug
TypeOfFailure	OtherFailure
Priority	normal
Resolution	Unresolved
Component	Compiler
Test case
Differential revisions
BlockedBy
Related
Blocking
CC
Operating system
Architecture

changed weight to 5

added Tbug Trac import labels

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

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?

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?

Re ticket:10027#comment:95626, 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?

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.

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?

Replying to [ticket:10027#comment:95745 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?

Putting ticket:10027#comment:95745 and ticket:10027#comment:95756 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.

I just ran into this issue and it looks like you can indeed use pattern DataCon to import the hidden constructor. Does that mean this issue could be marked as resolved?

I think so. There is a way now to get to the constructor, and everything else in this ticket is conjecture. If someone wants to pursue a particular design mooted here, they should submit a ghc-proposal. Thanks for bringing this up, @ruhatch!

closed

reopened

Actually, we should add a regression test for this; reopening for that purpose.

added Pnormal label

Is this related to #11164 (closed)?