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-0.2.0.0-9d35c97e886f807a1e6d024aaa91dcec
wired-in package integer-gmp mapped to integer-gmp-0.2.0.0-9a51ffb34a83618a1a3d4e472b9977a0
wired-in package base mapped to base-4.2.0.0-b340bbd470b5859bcbb920aea62a12cf
wired-in package rts mapped to builtin_rts
wired-in package haskell98 mapped to haskell98-1.0.1.1-0fdaf3b26bc38c43ce8371edf538dbf6
wired-in package template-haskell mapped to template-haskell-2.4.0.0-fc13d9708caa0cfdc4173ee31dc2bf26
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


D:\code\overlap.hs:8:11:
    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 http://www.haskell.org/ghc/docs/latest/html/users_guide/type-class-extensions.html#instance-overlap. 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.

Simon

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

Milestone: 7.0.17.2.1

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).

Simon

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.

Simon

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  simonpj@microsoft.com
  * 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.

Simon

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.