Opened 6 years ago

Closed 6 years ago

Last modified 6 years ago

#2176 closed bug (fixed)

H98 module Array exports non-H98 instance Functor ((->) a)

Reported by: duncan Owned by:
Priority: normal Milestone: 6.10.1
Component: libraries/haskell98 Version: 6.8.2
Keywords: Cc:
Operating System: Unknown/Multiple Architecture: Unknown/Multiple
Type of failure: Difficulty: Unknown
Test Case: Blocked By:
Blocking: Related Tickets:

Description

Here's a subtle problem for which we have global class instances to thank.

Take a look at how many modules in base and the other core libraries re-export the class instances defined in Control.Monad.Instances. A lot do. Indeed the Data.Array and thus the H98 Array module does. That's very bad.

It makes it easy to accidentally write non-portable non-H98 code. This bit us in Cabal. We try to keep Cabal working with ghc, nhc98, hugs etc. Malcolm discovered that we were relying on the instance Functor ((->) a) but were not importing it for nhc98 at least. Nothing in Cabal imports Control.Monad.Instances so I was wondering how we were coming to end up with it. Turns out we're getting it at least via Data.Array.

Tracking down the source of instances is quite tricky. I wonder if there is anything we can do to make it easier? I was using ghc --show-iface on all the imports to try and find it. In this case it was easier because all I had to look for was Control.Monad.Instances as an orphan module.

We should audit which modules are importing Control.Monad.Instances and see if they're essential or just convenience. The point of Control.Monad.Instances being in a separate module was that it'd not be in scope by default. That's defeated if other standard modules use it. For example does Control.Applicative really need it? In fact Control.Applicative is probably the root offender here, it's the one that causes Data.Array to re-export the unwanted instances.

Do we need a Control.Applicative.Instances perhaps for the Applicative ((->) a) instance?

Attachments (1)

no-import-functor-instance.dpatch (1.6 KB) - added by duncan 6 years ago.
patch: Don't import Control.Applicative just to get <$> use fmap

Download all attachments as: .zip

Change History (10)

comment:1 Changed 6 years ago by ross

Here's a test for this bug:

module ShouldCompile where

-- import all Haskell 98 modules
import Array
import Char
import Complex
import CPUTime
import Directory
import IO
import Ix
import List
import Locale
import Maybe
import Monad
import Numeric
import Random
import Ratio
import System
import Time

-- This will fail if any of the Haskell 98 modules indirectly import
-- Control.Monad.Instances
instance Functor ((->) r) where
        fmap = (.)

This can be used to show that Array is the only H98 module where this bites. The problem is that it defines Foldable and Traversable instances for the Array type, and thus needs those modules and Control.Applicative, which they import. (Another vector is Control.Monad.Fix, imported by Control.Arrow, but that doesn't seem to lead to H98 modules.)

I believe that these instances are a Good Thing, and we want them everywhere, except in H98 modules.

I think the fix is to have an extra version of Data.Array, without the Foldable and Traversable instances (which are orphans anyway), and have Array import that.

comment:2 Changed 6 years ago by ross

To clarify, it's the Functor and Monad instances in Control.Monad.Instances that are the Good Thing.

comment:3 in reply to: ↑ description Changed 6 years ago by claus

Replying to duncan:

Tracking down the source of instances is quite tricky. I wonder if there is anything we can do to make it easier? I was using ghc --show-iface on all the imports to try and find it.

use ghci and :info?

GHCi, version 6.9.20080217: http://www.haskell.org/ghc/  :? for help
Loading package base ... linking ... done.
Prelude> :i Functor
class Functor f where fmap :: (a -> b) -> f a -> f b
        -- Defined in GHC.Base
instance Functor Maybe -- Defined in Data.Maybe
instance Functor [] -- Defined in GHC.Base
instance Functor IO -- Defined in GHC.IOBase
Prelude> :m +Data.Array
Prelude Data.Array> :i Functor
class Functor f where fmap :: (a -> b) -> f a -> f b
        -- Defined in GHC.Base
instance (Ix i) => Functor (Array i) -- Defined in GHC.Arr
instance Functor ((->) r) -- Defined in Control.Monad.Instances
instance Functor ((,) a) -- Defined in Control.Monad.Instances
instance Functor (Either a) -- Defined in Control.Monad.Instances
instance Functor Maybe -- Defined in Data.Maybe
instance Functor [] -- Defined in GHC.Base
instance Functor IO -- Defined in GHC.IOBase
Prelude Data.Array>

one problem with this is that instances aren't managed properly in ghci sessions:

Prelude Data.Array> :m -Data.Array
Prelude> :i Functor
class Functor f where fmap :: (a -> b) -> f a -> f b
        -- Defined in GHC.Base
instance Functor ((->) r) -- Defined in Control.Monad.Instances
instance Functor ((,) a) -- Defined in Control.Monad.Instances
instance Functor (Either a) -- Defined in Control.Monad.Instances
instance Functor Maybe -- Defined in Data.Maybe
instance Functor [] -- Defined in GHC.Base
instance Functor IO -- Defined in GHC.IOBase
Prelude>

i thought the latter was a known bug, but i can't seem to find the ticket..

Changed 6 years ago by duncan

patch: Don't import Control.Applicative just to get <$> use fmap

comment:4 Changed 6 years ago by ross

No good: Data.Foldable and Data.Traversable also import Control.Applicative.

comment:5 Changed 6 years ago by igloo

  • Difficulty set to Unknown
  • Milestone set to 6.8.3

comment:6 Changed 6 years ago by igloo

  • Milestone changed from 6.8.3 to 6.10.1

comment:7 Changed 6 years ago by ross

  • Resolution set to fixed
  • Status changed from new to closed

Fixed by elimination of orphan instances in Data.Array and then the patch

Sun Aug 17 01:23:48 BST 2008  Ross Paterson <ross@soi.city.ac.uk>
  * delete imports of Data.Foldable and Data.Traversable (fixes #2176)

comment:8 Changed 6 years ago by simonmar

  • Architecture changed from Unknown to Unknown/Multiple

comment:9 Changed 6 years ago by simonmar

  • Operating System changed from Unknown to Unknown/Multiple
Note: See TracTickets for help on using tickets.