Opened 3 years ago

Closed 3 years 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 Rev(s):
Wiki Page:

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 3 years ago by simonpj

Resolution: fixed
Status: newclosed
Test Case: 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 3 years ago by simonpj

Here's the commit

commit f964cd9c5c411f8a2383cf2b080581a5c3349661
Author: Simon Peyton Jones <simonpj@microsoft.com>
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 3 years ago by simonpj

Status: closedmerge

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

Simon

comment:4 Changed 3 years ago by thoughtpolice

Milestone: 7.8.4
Status: mergeclosed

Merged to 7.8.4.

Note: See TracTickets for help on using tickets.