orphan instance warnings are badly behaved

[duncan]

There are two problems with them:

• They do not respect -Werror. That is they are not fatal when using -Werror.
• They are ugly. They do not explain the issue or use Haskell syntax. They look like a dump of internal data structures and mention internally generated names.

Warning: orphan instances:
  instance HAppS.Data.Serialize.Serialize [Distribution.Server.Types.PkgInfo]
    = Distribution.Server.State.$f1
  instance HAppS.Data.Serialize.Version [Distribution.Server.Types.PkgInfo]
    = Distribution.Server.State.$f2
  instance HAppS.Data.Serialize.Serialize [Distribution.Package.PackageIdentifier]
    = Distribution.Server.State.$f3
  instance HAppS.Data.Serialize.Version [Distribution.Package.PackageIdentifier]
    = Distribution.Server.State.$f4

[simonpj]

Good suggestion. I've fixed this

Mon Aug  4 17:21:29 BST 2008  simonpj@microsoft.com
  * Fix Trac #2467: decent warnings for orphan instances

You must pull the array and mtl libraries too, because they have orphan modules that die with -Werror unless you do.

ToDo?: Ian: look into de-orphanising those modules.

Simon

[igloo]

array done.

[igloo]

In order to do mtl we'd need to move at least the MonadPlus IO instance into the base package, so I'm remilestoning this for 6.12.

[Syzygies]

It came up in my code that I needed

instance Foldable ((,) a) where foldr f z (_,y) = f y z

which is completely parallel to the Functor instance, but unlike the Functor instance is not provided by the standard libraries.

There is no way that this instance can be anything but an orphan in my code (I shouldn't change Data.Foldable on a whim, it would make my code non-portable), so in this case I would argue that it is wrong for -Wall -Werror to abort compilation over this. I added -fno-warn-orphans, but that is a clunky fix which now deprives me of that warning when it might be relevant.

[simonpj]

Indeed, your code is legitimate, and that's why warnings are just warnings!

More concretely, what would you like? A special kind of warning that (uniquely) does not make -Werror abort the compilation? That seems a bit irregular... But perhaps you have something in mind.

Simon

[Syzygies]

Reflecting further, I noticed that these manual sections go together nicely:

5.1.2. Command line options in source files

5.6.12. Orphan modules and instance declarations

In particular, the phrase stands out

in some circumstances, the OPTIONS_GHC pragma is the Right Thing.

Moving -fno-warn-orphans from the command line to a particular orphan file is idiomatic GHC for my situation. I already have a module containing helper functions such as

foldrC ∷ (Foldable d, Foldable e, Foldable f) ⇒
  (c a → d (e (f a))) → (a → b → b) → b → c a → b
foldrC gi h y = foldr (flip $ foldr (flip $ foldr h)) y . gi

for defining deep Foldable instances that go through a newtype and three layers of structure.

instance Foldable ((,) a) where foldr f z (_,y) = f y z

can arise as one of those layers, so this is a natural place to put such orphans.

Instance declarations are just as hard on the programmer as on GHC itself. For example, Functor ((,) a) is documented in Control.Monad, but importing Control.Monad doesn't expose this instance. Functor ((,) a) is not documented in Data.Foldable, but importing Data.Foldable does expose this instance. This left me perplexed as to why I could define deep Functor instances that relied on Functor ((,) a), but I couldn't write foldr (+) 2 (1,2) in ghci. I was left believing that this was some compiler magic I didn't understand, until I started using ghc --show-iface, and spelunking the library source code.

If I could have one wish related to instance declarations, it wouldn't have to do with these orphan warnings, but rather with better control over duplicate instance declarations. For example, I can't import Data.Foldable and at the same time try to redefine instance Monad ((->) a); there is no hiding clause that affects instances. Perhaps this is a moot point, as I have yet to find a standard library instance that has a reasonable alternative definition. One can imagine Haskell' providing missing instances anytime it can prove that there is only one possible definition.

[ross]

Replying to Syzygies:

It came up in my code that I needed

instance Foldable ((,) a) where foldr f z (_,y) = f y z

which is completely parallel to the Functor instance, but unlike the Functor instance is not provided by the standard libraries. There is no way that this instance can be anything but an orphan in my code (I shouldn't change Data.Foldable on a whim, it would make my code non-portable), so in this case I would argue that it is wrong for -Wall -Werror to abort compilation over this. I added -fno-warn-orphans, but that is a clunky fix which now deprives me of that warning when it might be relevant.

The compiler is right: the instance is an orphan, and it's right to warn about it, because orphan instances are inevitably a pain. The orphan instances of Monad and Functor for Prelude type constructors (which you noted) are a case in point; unfortunately they had to be orphans to avoid breaking compatibility with Haskell 98.

The right fix in this case is to get the above instance (and one for Either, and Traversable instances for (,) and Either) into Data.Foldable and Data.Traversable. Unfortunately that won't help you until next September/October (and will give you a messy changeover then).

[igloo]

Replying to simonpj:

Indeed, your code is legitimate, and that's why warnings are just warnings! More concretely, what would you like? A special kind of warning that (uniquely) does not make -Werror abort the compilation? That seems a bit irregular... But perhaps you have something in mind.

Perhaps we should have separate control for two different classes of orphan warnings:

• those where the class and all the types are in other packages (and thus you may not be able to easily avoid the instance, e.g. if everything is in the base package).
• the rest

[simonpj]

There seem to be two things going on in this ticket.

1. Do the instances in mtl and array have to include orphan instances? We should investigate and fix or punt.

2. Some reflections on orphan instances in general. Here the discussion is inconclusive, so we should probably milestone as _|_, leaving the ticket open so that we don't lose the thoughts.

Simon

[igloo]

The instances are now either removed, or their reason for existing is given in a comment, so 1. is done.