Opened 10 years ago

Closed 9 years ago

Last modified 9 years ago

#3877 closed feature request (fixed)

Require XOverlappingInstances for the most specific instance only

Reported by: traz161616 Owned by: igloo
Priority: normal Milestone: 7.4.1
Component: Compiler (Type checker) Version: 6.12.1
Keywords: XOverlappingInstances Cc:
Operating System: Unknown/Multiple Architecture: Unknown/Multiple
Type of failure: GHC rejects valid program Test Case:
Blocked By: Blocking:
Related Tickets: Differential Rev(s):
Wiki Page:

Description (last modified by simonpj)

I tried compiling the attached snippet with both ghc 6.12.1 and 6.10.4. The result is that even with XOverlappingInstances flag it still does not allow the program although it is provided with a concrete instance.

The code snippet is:

{- File overlap.hs -}
{-# OPTIONS_GHC -XOverlappingInstances -XTypeSynonymInstances #-}

type PairIS = (Int,String)
instance Show PairIS where
 show (a,b) = "(("++(show a)++","++(show b)++"))"

test :: Int -> String -> String
test a b = show (a,b)

The OS is :Linux 2.6.31-19-server #56-Ubuntu SMP x86_64 The gcc version is (irrelevant): gcc version 4.4.1

The error message is:

Glasgow Haskell Compiler, Version 6.12.1, for Haskell 98, stage 2 booted by GHC version 6.8.2
Using binary package database: /usr/local/lib/ghc-6.12.1/package.conf.d/package.cache
wired-in package ghc-prim mapped to ghc-prim-
wired-in package integer-gmp mapped to integer-gmp-
wired-in package base mapped to base-
wired-in package rts mapped to builtin_rts
wired-in package haskell98 mapped to haskell98-
wired-in package template-haskell mapped to template-haskell-
wired-in package dph-seq mapped to dph-seq-0.4.0-b8a71915f9569cbd3a6895da8311707a
wired-in package dph-par mapped to dph-par-0.4.0-68df56bb8ae18f20e3518a7b338d7ebc
Hsc static flags: -static

    Overlapping instances for Show (Int, String)
      arising from a use of `show' at D:\code\overlap.hs:8:11-20
    Matching instances:
      instance (Show a, Show b) => Show (a, b) -- Defined in GHC.Show
      instance [overlap ok] Show PairIS
        -- Defined at D:\code\overlap.hs:4:9-19
    In the expression: show (a, b)
    In the definition of `test': test a b = show (a, b) 

Change History (15)

comment:1 Changed 10 years ago by simonpj

Description: modified (diff)

Currently this is by design; see Note esp the remarks at the end, which point out that the current design is not the only possible one.

So there's no great implementation challenge here; it's a design question.


comment:2 Changed 10 years ago by igloo

Milestone: 6.14.1

comment:3 Changed 9 years ago by igloo

Resolution: wontfix
Status: newclosed
Type: bugfeature request

The key point is that the (a,b) instance in the prelude would need to be compiled with overlapping instances enabled, in order for this to be accepted.

No-one else has commented in 6 months, and it's not clear that fixing it would be better than the status quo, so I'm closing this as wontfix.

comment:4 Changed 9 years ago by gidyn

Architecture: x86_64 (amd64)Unknown/Multiple
Cc: gideon@… added
Operating System: LinuxUnknown/Multiple
Resolution: wontfix
Status: closednew
Summary: XOverlappingInstances flag not workingRequire XOverlappingInstances for the most specific instance only

I've just been his by this, and it's exceedingly annoying; we are prevented from writing custom Show instances for type synonyms.

I've changed the title to request this change, as described in the documentation.(Perhaps the rule should instead say that the overlapping instance declaration should be compiled in this way, rather than the overlapped one ... We are interested to receive feedback on these points.)

comment:5 Changed 9 years ago by igloo


comment:6 Changed 9 years ago by simonpj

I'm entirely willing to make this change. Hmm. Why not follow a process similar to that for library changes (which this nearly is):

  • Extend this ticket with a standalone description of the proposed change
  • Email the summary to ghc-users (and, I suggest, libraries@…)
  • Give a deadline for discussion of a few weeks.

That would give an opportunity for others who use overlapping instances to have their say (perhaps in support).


comment:7 Changed 9 years ago by guest

I've posted the request for discussion to users and libraries, with a proposed deadline of 16th November.

comment:8 Changed 9 years ago by guest

Deadline changed to end of November, as per SPJ's request.

comment:9 Changed 9 years ago by gidyn

Status: newpatch

Deadline passed, almost no response from the mailing lists (other than requesting that this not break a lot of existing code, which it won't).

comment:10 Changed 9 years ago by gidyn

Status: patchnew

comment:11 Changed 9 years ago by simonpj

OK so the current rule is this:

  • If an instance decl is compiled with -XOverlappingInstances then it can be overlapped by a more-specific instance. The (later) more specific instance does not need to be compiled with -XOverlappingInstances

You want precisely the reverse of this. The less-specific instance is in some library compiled without -XOverlappingInstances, and you want to write an instance that deliberately overlaps it.

To make this change would break other people's code, because a library author could not write an instance declaration (such as a "default instance" instance Foo a where...) that he expects to be overlapped.

So to meet your goals, we either need a new flag altogether (which seems a bit heavyweight), or to say

  • Overlap between two instance decls is allowed if either (or both) of them was compiled with -XOverlappingInstances.

This seems reasonable to me... one party or the other has to agree to an overlap. It's a bit more liberal than the current policy.

Ok with everyone? It's a one-line change to the code; a bit more in the documentation.


comment:12 Changed 9 years ago by gidyn

I'm happy with either change

comment:13 Changed 9 years ago by simonpj

Owner: set to igloo

OK done:

Tue Dec 14 10:05:00 PST 2010
  * Instance declaration overlap allowed if *either* has -XOverlappingInstances
  This satisfies Trac #3877.  Documentation is changed too.
  I'm not sure if this should go in 7.0.2.

    M ./compiler/types/InstEnv.lhs -8 +12
    M ./docs/users_guide/glasgow_exts.xml -19 +10

I'm agnostic about whether this should sneak into 7.0.2 (even if there is time to do so). It allows more programs to compile (like a bug fix), but it'd mean that any program relying on it would have to depend on 7.0.2 and not 7.0.1 etc.


comment:14 Changed 9 years ago by igloo

Resolution: fixed
Status: newclosed

I don't think we should make this change in the branch.

comment:15 Changed 9 years ago by gidyn

Cc: gideon@… removed
Note: See TracTickets for help on using tickets.