Opened 11 months ago

Closed 4 months ago

#9006 closed bug (fixed)

GHC accepts import of private data constructor if it has the same name as the type

Reported by: Lemming Owned by:
Priority: normal Milestone: 7.8.4
Component: Compiler Version: 7.8.2
Keywords: Cc:
Operating System: Unknown/Multiple Architecture: Unknown/Multiple
Type of failure: GHC accepts invalid program Test Case: rename/should_fail/T9006
Blocked By: Blocking:
Related Tickets: Differential Revisions:

Description

$ cat A.hs B.hs
module A (T) where

data T = T

module B where

import A (T(T))

ghci-7.8.2 accepts module B, whereas ghci-7.6.3 and ghci-7.4.2 say:

B.hs:3:11: Module `A' does not export `T(T)'

For this bug to happen it is important, that the data constructor has the same name as the type.
I.e. if you rename the data constructor to, say, Cons, then ghci-7.8.2 will emit the same error as earlier versions.
The bug is not dramatic, since if you try to actually use the imported data constructor then ghc will say

B.hs:20:5:
    Not in scope: data constructor ‘T’

However this error message is very confusing, because it suggests that T is not in scope although you imported it correctly.

Change History (4)

comment:1 Changed 11 months ago by simonpj

  • Resolution set to fixed
  • Status changed from new to closed
  • Test Case set to rename/should_fail/T9006

Excellent point, thank you.

We could merge the fix to the 7.8 branch, but I'm not sure it's worth the bother

Simon

comment:2 Changed 11 months ago by simonpj

Here's the commit

commit f964cd9c5c411f8a2383cf2b080581a5c3349661
Author: Simon Peyton Jones <[email protected]>
Date:   Fri Apr 18 23:30:18 2014 +0100

    Take account of the AvailTC invariant when importing
    
    In the rather gnarly filterImports code, someone had forgotten
    the AvailTC invariant:  in AvailTC n [n,s1,s2], the 'n' is itself
    included in the list of names.

 compiler/rename/RnNames.lhs                     | 80 +++++++++++++++----------
 testsuite/tests/rename/should_fail/T9006.hs     |  3 +
 testsuite/tests/rename/should_fail/T9006.stderr |  2 +
 testsuite/tests/rename/should_fail/T9006a.hs    |  3 +
 testsuite/tests/rename/should_fail/all.T        |  3 +
 5 files changed, 59 insertions(+), 32 deletions(-)

comment:3 Changed 6 months ago by simonpj

  • Status changed from closed to merge

I don't think this ever got merged, and it showed up again as #9544

Simon

comment:4 Changed 4 months ago by thoughtpolice

  • Milestone set to 7.8.4
  • Status changed from merge to closed

Merged to 7.8.4.

Note: See TracTickets for help on using tickets.