Opened 10 years ago

Closed 3 years ago

Last modified 2 years ago

#2182 closed bug (fixed)

ghc sessions (--make, --interactive, ghc api) erroneously retain instances

Reported by: claus Owned by: ezyang
Priority: normal Milestone: 7.10.1
Component: Compiler (Type checker) Version: 6.9
Keywords: Cc: hvr, simonpj
Operating System: Unknown/Multiple Architecture: Unknown/Multiple
Type of failure: GHC accepts invalid program Test Case: T2182ghci, T2182ghci2, T2182
Blocked By: Blocking: #5316
Related Tickets: Differential Rev(s): Phab:D488
Wiki Page:

Description (last modified by ezyang)

The EPS (external-package state) is only ever increased, never decreased between compilation of different modules in a single batch compilation or GHCi session.

Example 1 (GHCi):

ezyang@sabre:~/Dev/labs/ezyangest$ ghci
GHCi, version 7.6.3: http://www.haskell.org/ghc/  :? for help
Loading package ghc-prim ... linking ... done.
Loading package integer-gmp ... linking ... done.
Loading package base ... linking ... done.
Prelude> (\x -> x)

<interactive>:2:1:
    No instance for (Show (t0 -> t0)) arising from a use of `print'
    Possible fix: add an instance declaration for (Show (t0 -> t0))
    In a stmt of an interactive GHCi command: print it
Prelude> :m +Text.Show.Functions
Prelude Text.Show.Functions> (\x -> x)
<function>
Prelude Text.Show.Functions> :m -Text.Show.Functions
Prelude> :r
Ok, modules loaded: none.
Prelude> (\x -> x)
<function>

Example 2 (make):

ezyang@sabre:~/Dev/labs/ezyangest$ cat A.hs
module A where
import Text.Show.Functions
ezyang@sabre:~/Dev/labs/ezyangest$ cat B.hs
module B where
y = show (\x -> x)
ezyang@sabre:~/Dev/labs/ezyangest$ ghc --make B.hs A.hs
[1 of 2] Compiling A                ( A.hs, A.o )
[2 of 2] Compiling B                ( B.hs, B.o )
ezyang@sabre:~/Dev/labs/ezyangest$ ghc --make A.hs B.hs -fforce-recomp
[1 of 2] Compiling B                ( B.hs, B.o )

B.hs:2:5:
    No instance for (Show (t0 -> t0)) arising from a use of `show'
    Possible fix: add an instance declaration for (Show (t0 -> t0))
    In the expression: show (\ x -> x)
    In an equation for `y': y = show (\ x -> x)

Change History (19)

comment:1 Changed 10 years ago by igloo

difficulty: Unknown
Milestone: 6.10 branch

Thans for the report. #333 claims to have fixed this, but if so then it looks like it broke again.

comment:2 Changed 9 years ago by simonmar

Milestone: 6.10 branch_|_
Priority: normallow

In fact this is a known bug mentioned in the User's Guide: http://www.haskell.org/ghc/docs/latest/html/users_guide/bugs.html#bugs-ghc (see the final bullet point).

We don't have a ticket open for it as far as I can tell, though, so I'll leave this one open.

comment:3 Changed 9 years ago by igloo

See also #2404

comment:4 Changed 9 years ago by claus

Summary: ghci session retains instances after :m -Moduleghc sessions (--make, --interactive, ghc api) erroneously retain instances

(changed ticket title to reflect broader scope)

This bug affects all ghc sessions, not just ghci. In particular:

  • Cabal uses ghc --make, which can fail because of this bug
  • Haddock 2 uses the ghc api, which can fail because of this bug

In both cases, the workaround is to repeat the command, so that it continues with a fresh ghc session, but that will not at all be obvious to users.

Example:

$ darcs get http://www.cs.kent.ac.uk/~cr3/toolbox/haskell/syb-utils
Finished getting.

$ ghc --version
The Glorious Glasgow Haskell Compilation System, version 6.8.3

$ cd syb-utils/

$ runhaskell.exe Setup.hs configure
Configuring syb-utils-0.0.2008.8.19...
Warning: No license-file field.

$ runhaskell.exe Setup.hs build
Preprocessing library syb-utils-0.0.2008.8.19...
Building syb-utils-0.0.2008.8.19...
[1 of 6] Compiling Data.Generics.Instances.Dubious ( Data/Generics/Instances/Dubious.hs, dist\build/
Data/Generics/Instances/Dubious.o )
[2 of 6] Compiling Data.Generics.Utils ( Data/Generics/Utils.hs, dist\build/Data/Generics/Utils.o )
[3 of 6] Compiling Data.Generics.NoInstances ( Data/Generics/NoInstances.hs, dist\build/Data/Generic
s/NoInstances.o )
[4 of 6] Compiling Data.Generics.GPS ( Data/Generics/GPS.hs, dist\build/Data/Generics/GPS.o )
[5 of 6] Compiling Data.Generics.Instances.Standard ( Data/Generics/Instances/Standard.hs, dist\buil
d/Data/Generics/Instances/Standard.o )

Data/Generics/Instances/Standard.hs:61:0:
    Duplicate instance declarations:
      instance Data Bool
        -- Defined at Data/Generics/Instances/Standard.hs:(61,0)-(68,28)
      instance Data Bool -- Defined in Data.Generics.Instances
..

$ runhaskell.exe Setup.hs build
Preprocessing library syb-utils-0.0.2008.8.19...
Building syb-utils-0.0.2008.8.19...
[5 of 6] Compiling Data.Generics.Instances.Standard ( Data/Generics/Instances/Standard.hs, dist\buil
d/Data/Generics/Instances/Standard.o )
[6 of 6] Compiling Data.Generics.Alt ( Data/Generics/Alt.hs, dist\build/Data/Generics/Alt.o )
c:\ghc\ghc-6.8.3\bin\ar.exe: creating dist\build\libHSsyb-utils-0.0.2008.8.19.a

Confusingly, the behaviour is compiler-version and tool dependent:

$ runhaskell.exe Setup.hs clean
cleaning...

$ /cygdrive/c//fptools/ghc/ghc/stage2-inplace/ghc.exe --version
The Glorious Glasgow Haskell Compilation System, version 6.9.20080816

$ runhaskell Setup configure --with-compiler=c:/fptools/ghc/ghc/stage2-inplace/ghc.exe --with-hc-pk
g=c:/fptools/ghc/utils/ghc-pkg/install-inplace/bin/ghc-pkg.exe --with-haddock=c:/Program\ Files/Has
kell/bin/haddock.exe -fGhcApi
Configuring syb-utils-0.0.2008.8.19...
Warning: No license-file field.

cr3@cr3-lt ~/tst/ghc-session-bug/syb-utils
$ runhaskell.exe Setup.hs build
Preprocessing library syb-utils-0.0.2008.8.19...
Building syb-utils-0.0.2008.8.19...
[1 of 9] Compiling Data.Generics.Instances.Dubious ( Data\Generics\Instances\Dubious.hs, dist\build\
Data\Generics\Instances\Dubious.o )
[2 of 9] Compiling Data.Generics.Instances.Standard ( Data\Generics\Instances\Standard.hs, dist\buil
d\Data\Generics\Instances\Standard.o )
[3 of 9] Compiling Data.Generics.Utils ( Data\Generics\Utils.hs, dist\build\Data\Generics\Utils.o )
[4 of 9] Compiling Data.Generics.NoInstances ( Data\Generics\NoInstances.hs, dist\build\Data\Generic
s\NoInstances.o )
[5 of 9] Compiling Data.Generics.GPS ( Data\Generics\GPS.hs, dist\build\Data\Generics\GPS.o )
[6 of 9] Compiling Data.Generics.Alt ( Data\Generics\Alt.hs, dist\build\Data\Generics\Alt.o )
[7 of 9] Compiling GHC.Syb.Instances0 ( GHC\Syb\Instances0.hs, dist\build\GHC\Syb\Instances0.o )
[8 of 9] Compiling GHC.Syb.Instances ( GHC\Syb\Instances.hs, dist\build\GHC\Syb\Instances.o )
[9 of 9] Compiling GHC.Syb.Utils    ( GHC\Syb\Utils.hs, dist\build\GHC\Syb\Utils.o )
c:\ghc\ghc-6.8.3\bin\ar.exe: creating dist\build\libHSsyb-utils-0.0.2008.8.19.a

Note that ghc head and ghc 6.8.3 process the dependencies in a different order, which probably explains why the bug isn't triggered in head: Data.Generics.Instances.Dubious and Data.Generics.Instances.Standard represent two halves of base's Data.Generics.Instances; the latter isn't explicitly imported here, but Data.Generics.GPS uses Data.IntMap, which inadvertedly re-exports the instances from Data.Generics.Instances.

If Data.Generics.Instances.* are compiled first, ghc doesn't complain about alternative instances being present, but if either of Data.Generics.Instances.* is compiled after Data.Generics.GPS, the erroneously retained instances conflict with the newly defined ones.

We can confirm the bug is still there by trying to haddock2 the package with a fresh darcs haddock, freshly compiled with the same ghc head:

$ runhaskell.exe Setup.hs clean
cleaning...

$ /cygdrive/c/Program\ Files/Haskell/bin/haddock.exe --version
Haddock version 2.2.2, (c) Simon Marlow 2006
Ported to use the GHC API by David Waern 2006-2008

$ /cygdrive/c/Program\ Files/Haskell/bin/haddock.exe +RTS --info
 [("GHC RTS", "Yes")
 ,("GHC version", "6.9.20080816")
 ,("RTS way", "rts")
 ,("Host platform", "i386-unknown-mingw32")
 ,("Build platform", "i386-unknown-mingw32")
 ,("Target platform", "i386-unknown-mingw32")
 ,("Compiler unregisterised", "NO")
 ,("Tables next to code", "YES")
 ]

$ runhaskell Setup configure --with-compiler=c:/fptools/ghc/ghc/stage2-inplace/ghc.exe --with-hc-pk
g=c:/fptools/ghc/utils/ghc-pkg/install-inplace/bin/ghc-pkg.exe --with-haddock=c:/Program\ Files/Haskell/bin/haddock.exe -fGhcApi
Configuring syb-utils-0.0.2008.8.19...
Warning: No license-file field.

cr3@cr3-lt ~/tst/ghc-session-bug/syb-utils
$ runhaskell.exe Setup.hs haddock
Preprocessing library syb-utils-0.0.2008.8.19...
Running Haddock for syb-utils-0.0.2008.8.19...
Preprocessing library syb-utils-0.0.2008.8.19...
Warning: Cannot read c:\fptools\ghc\compiler\dist-stage2\doc\html\ghc\ghc.haddock:
   "Magic number mismatch: couldn't load interface file: c:\\fptools\\ghc\\compiler\\dist-stage2\\doc\\html\\ghc\\ghc.haddock"
Skipping this interface.
Warning: Cannot read c:\fptools\ghc\libraries\base\dist\doc\html\base\base.haddock:
   "Magic number mismatch: couldn't load interface file: c:\\fptools\\ghc\\libraries\\base\\dist\\doc\\html\\base\\base.haddock"
Skipping this interface.
Warning: Cannot read c:\fptools\ghc\libraries\containers\dist\doc\html\containers\containers.haddock
:
   "Magic number mismatch: couldn't load interface file: c:\\fptools\\ghc\\libraries\\containers\\dist\\doc\\html\\containers\\containers.haddock"
Skipping this interface.

Data\Generics\Instances\Dubious.hs:59:9:
    Duplicate instance declarations:
      instance (Data a, Integral a) => Data (Ratio a)
        -- Defined at Data\Generics\Instances\Dubious.hs:59:9-46
      instance (Data a, Integral a) => Data (Ratio a)
        -- Defined in Data.Generics.Instances
..

$ runhaskell.exe Setup.hs haddock
Preprocessing library syb-utils-0.0.2008.8.19...
Running Haddock for syb-utils-0.0.2008.8.19...
Preprocessing library syb-utils-0.0.2008.8.19...
Warning: Cannot read c:\fptools\ghc\compiler\dist-stage2\doc\html\ghc\ghc.haddock:
   "Magic number mismatch: couldn't load interface file: c:\\fptools\\ghc\\compiler\\dist-stage2\\doc\\html\\ghc\\ghc.haddock"
Skipping this interface.
Warning: Cannot read c:\fptools\ghc\libraries\base\dist\doc\html\base\base.haddock:
   "Magic number mismatch: couldn't load interface file: c:\\fptools\\ghc\\libraries\\base\\dist\\doc\\html\\base\\base.haddock"
Skipping this interface.
Warning: Cannot read c:\fptools\ghc\libraries\containers\dist\doc\html\containers\containers.haddock
:
   "Magic number mismatch: couldn't load interface file: c:\\fptools\\ghc\\libraries\\containers\\dist\\doc\\html\\containers\\containers.haddock"
Skipping this interface.
Warning: in export list of "Data.Generics.NoInstances": module not found: "Data.Generics.Basics"
Warning: in export list of "Data.Generics.NoInstances": module not found: "Data.Generics.Aliases"
Warning: in export list of "Data.Generics.NoInstances": module not found: "Data.Generics.Schemes"
Warning: in export list of "Data.Generics.NoInstances": module not found: "Data.Generics.Text"
Warning: in export list of "Data.Generics.NoInstances": module not found: "Data.Generics.Twins"
Warning: in export list of "Data.Generics.Alt": module not found: "Data.Generics.Basics"
Warning: in export list of "Data.Generics.Alt": module not found: "Data.Generics.Aliases"
Warning: in export list of "Data.Generics.Alt": module not found: "Data.Generics.Schemes"
Warning: in export list of "Data.Generics.Alt": module not found: "Data.Generics.Text"
Warning: in export list of "Data.Generics.Alt": module not found: "Data.Generics.Twins"
Warning: syb-utils-0.0.2008.8.19:GHC.Syb.Instances: could not find link destinations for:
    Data.Typeable.TyCon
Warning: syb-utils-0.0.2008.8.19:GHC.Syb.Instances0: could not find link destinations for:
    Data.Typeable.TyCon
Warning: syb-utils-0.0.2008.8.19:GHC.Syb.Utils: could not find link destinations for:
    GHC.Classes.Eq GHC.Classes.Ord GHC.Show.Show Data.Generics.Basics.Data GHC.Types.Int GHC.Base.St
ring Data.Generics.Aliases.GenericQ GHC.Bool.Bool
Warning: syb-utils-0.0.2008.8.19:Data.Generics.GPS: could not find link destinations for:
    Data.Generics.Basics.Data Data.Generics.Aliases.GenericQ Data.Typeable.Typeable Data.Typeable.Typeable1 Data.Maybe.Maybe Data.IntMap.IntMap Data.IntSet.IntSet GHC.Types.Int Data.Generics.Aliases.GenericT
Warning: syb-utils-0.0.2008.8.19:Data.Generics.Utils: could not find link destinations for:
    Control.Applicative.Applicative Data.Typeable.Typeable1 Data.Typeable.Typeable Data.Generics.Basics.Data Data.Generics.Utils.X
Warning: syb-utils-0.0.2008.8.19:Data.Generics.Instances.Standard: could not find link destinations
for:
    Data.Typeable.TyCon
Documentation created: dist\doc\html\syb-utils\index.html

What are the chances of getting this fixed?

comment:5 Changed 9 years ago by simonmar

Architecture: UnknownUnknown/Multiple

comment:6 Changed 9 years ago by simonmar

Operating System: UnknownUnknown/Multiple

comment:7 Changed 3 years ago by ezyang

Cc: hvr added
Type of failure: None/Unknown

#8427 is related; this ticket is basically the GHCi variant of the problem.

comment:8 Changed 3 years ago by ezyang

Component: GHCiCompiler (Type checker)
Description: modified (diff)
Milestone: 7.10.1
Priority: lownormal
Type of failure: None/UnknownGHC accepts invalid program

I updated the bug description with some modernized, simpler test-cases.

simonpj and I have chatted about this and we have a new, improved plan of attack to solve the problem:

  1. We'll maintain a set of loaded interface files in the type-checking environment. When an interface file is loaded, we add it to this set.
  2. When a type class instance lookup is performed, IF the instance is an orphan, we'll also check if the interface file which defined the instance is in the loaded set. If it is not, we pretend it doesn't exist.

I have most of a patch to solve this but I have to shake out a bug where GHCi is not preserving the set of loaded interfaces.

comment:9 Changed 3 years ago by ezyang

Cc: simonpj added
Owner: set to ezyang

Plan changed again! Now the idea is to maintain the set of orphan modules reachable from DIRECTLY imported modules, instead of the set of interface files.

This ALMOST works. The big problem is when we mkLocalInstance, we need to know (immediately) if it's an orphan or not, because that entry in the instance environment may become enshrined forever if we have a GHCi session involved. I think we can pull this off by moving the logic from MkIface for calculating if an instance is an orphan to the typechecker.

comment:10 Changed 3 years ago by ezyang

Differential Rev(s): D488

comment:11 Changed 3 years ago by thoughtpolice

Differential Rev(s): D488Phab:D488

comment:12 Changed 3 years ago by Edward Z. Yang <ezyang@…>

In 4c834fdddf4d44d12039da4d6a2c63a660975b95/ghc:

Filter instance visibility based on set of visible orphans, fixes #2182.

Summary:
Amazingly, the fix for this very old bug is quite simple: when type-checking,
maintain a set of "visible orphan modules" based on the orphans list of
modules which we explicitly imported.  When we import an instance and it
is an orphan, we check if it is in the visible modules set, and if not,
ignore it.  A little bit of refactoring for when orphan-hood is calculated
happens so that we always know if an instance is an orphan or not.

For GHCi, we preinitialize the visible modules set based on the list of
interactive imports which are active.

Future work: Cache the visible orphan modules set for GHCi, rather than
recomputing it every type-checking round.  (But it's tricky what to do when you
/remove/ a module: you need a data structure a little more complicated than
just a set of modules.)

Signed-off-by: Edward Z. Yang <ezyang@cs.stanford.edu>

Test Plan: new tests and validate

Reviewers: simonpj, austin

Subscribers: thomie, carter

Differential Revision: https://phabricator.haskell.org/D488

GHC Trac Issues: #2182

comment:13 Changed 3 years ago by ezyang

Resolution: fixed
Status: newclosed

comment:14 Changed 3 years ago by simonpj

Owner: ezyang deleted
Resolution: fixed
Status: closednew

Edward, would you care to add a regression test? I'll re-open for that purpose.

Simon

comment:15 Changed 3 years ago by simonpj

Owner: set to ezyang

comment:16 Changed 3 years ago by ezyang

Resolution: fixed
Status: newclosed
Test Case: T2182ghci, T2182ghci2, T2182

Already did. :) T2182ghci, T2182ghci2, T2182. I'll add them to the ticket.

comment:17 Changed 3 years ago by Simon Peyton Jones <simonpj@…>

In 2a67fb3990f23391fecaec335f0d010434d2738e/ghc:

Minor refactoring of Edward's recent orphans patch (Trac #2182)

This patch is all small stuff
  - Move VisibleOrphanModules from Module to InstEnv (with the other orphan stuff)
  - Move Notes about orphans from IfaceSyn to InstEnv (ditto)
  - Make use of the record field names in InstEnvs

comment:18 Changed 2 years ago by ezyang

Blocking: 5316 added

comment:19 Changed 2 years ago by Edward Z. Yang <ezyang@…>

In 0cb1f5cf26fae946ca745abc5e302e62a8f66feb/ghc:

Filter orphan rules based on imports, fixes #10294 and #10420.

Summary:
If we have an orphan rule in our database, don't apply it
unless the defining module is transitively imported by the
module we are processing.  We do this by defining a new RuleEnv
data type which includes both the RuleBase as well as the set
of visible orphan modules, and threading this through the
relevant environments (CoreReader, RuleCheckEnv and ScEnv).

This is analogous to the instances fix we applied in #2182
4c834fdddf4d44d12039da4d6a2c63a660975b95, but done for RULES.
An important knock-on effect is that we can remove some buggy
code in LoadInterface which tried to avoid loading interfaces
that were loaded by plugins (which sometimes caused instances
and rules to NEVER become visible).

One note about tests: I renamed the old plugins07 test to T10420
and replaced plugins07 with a test to ensure that a plugin
import did not cause new rules to be loaded in.

Signed-off-by: Edward Z. Yang <ezyang@cs.stanford.edu>

Test Plan: validate

Reviewers: simonpj, austin, goldfire

Subscribers: bgamari, thomie

Differential Revision: https://phabricator.haskell.org/D950

GHC Trac Issues: #10420
Note: See TracTickets for help on using tickets.