Opened 2 years ago

Closed 2 years ago

Last modified 2 years ago

#10959 closed bug (invalid)

MINIMAL pragma and default implementations

Reported by: basvandijk Owned by:
Priority: normal Milestone:
Component: Compiler Version: 7.10.2
Keywords: Cc:
Operating System: Unknown/Multiple Architecture: Unknown/Multiple
Type of failure: Other Test Case:
Blocked By: Blocking:
Related Tickets: Differential Rev(s):
Wiki Page:

Description

The ToJSON class from the aeson library recently gained the new method toEncoding which has a default implementation based on the existing toJSON method.

A MINIMAL pragma was added to toJSON to make sure instances will define it.

However, toJSON also has a default implementation but only if the type has an instance for Generic:

class ToJSON a where
    toJSON     :: a -> Value
    {-# MINIMAL toJSON #-}

    default toJSON :: (Generic a, GToJSON (Rep a)) => a -> Value
    toJSON = genericToJSON defaultOptions

    toEncoding :: a -> Encoding
    toEncoding = Encoding . E.encodeToBuilder . toJSON

The problem is that users of aeson who are using the generic implementation get warnings that their toJSON method is not defined:

No explicit implementation for
  ‘toJSON’
In the instance declaration for ‘ToJSON MyType’

It would be nice if GHC is a bit smarter in this case so that when a a default implementation is used it won't give the warning.

Change History (9)

comment:1 in reply to:  description Changed 2 years ago by hvr

Why is there a MINIMAL pragma in the first place btw?

comment:2 Changed 2 years ago by basvandijk

I guess Bryan added because it's the only required method to implement since toEncoding is defined in terms of it.

However I just read in the GHC user guide that:

"If no MINIMAL pragma is given in the class declaration, it is just as if a pragma {-# MINIMAL op1, op2, ..., opn #-} was given, where the opi are the methods (a) that lack a default method in the class declaration..."

So this means we can just safely remove the pragma.

comment:3 Changed 2 years ago by simonpj

So is the conclusion here that GHC doesn't need to be smarter; it's a source-code issue?

comment:4 Changed 2 years ago by basvandijk

So is the conclusion here that GHC doesn't need to be smarter; it's a source-code issue?

Not really. I can fix it in aeson by simply removing the MINIMAL pragma but the issue does remain for other cases.

comment:5 Changed 2 years ago by simonpj

OK. Can you give a small standalone case that illustrates the issue you are raising? I don't get it yet.

comment:6 Changed 2 years ago by rwbarton

{-# LANGUAGE DefaultSignatures #-}

module Min where
class C a where
  meth :: a -> a
  {-# MINIMAL meth #-}

  default meth :: Enum a => a -> a
  meth = succ

instance C Int
{-
Min.hs:11:10: Warning:
    No explicit implementation for
      ‘meth’
    In the instance declaration for ‘C Int’
-}

It's true there is no explicit implementation, but surely the intention of the MINIMAL pragma was to warn if we end up with the meth = error "..." implementation that the compiler would supply if there was no other implementation at all. If we end up using the default implementation meth = succ then it should count towards satisfying the MINIMAL pragma.

Incidentally, as I just discovered in testing, the MINIMAL pragma is totally useless in this case: if you don't include an implementation for meth in an instance for C, ghc will try to use the default implementation, and if it doesn't type check (if there is no instance of Enum), that's an error; it doesn't fall back on the meth = error "..." implementation for a missing method. So there is no way you can ever not have an implementation for meth. (That's not how I thought it worked; I thought the default implementation would just be ignored if it doesn't type check. But it is consistent with the documentation. default meth :: Enum a => a -> a is a type signature on the default implementation, not a description of when to provide a default implementation.)

So I guess an even more minimal example would be

module Min where
class C a where
  meth :: a -> a
  meth = id
  {-# MINIMAL meth #-}

instance C Int
{-
Min.hs:7:10: Warning:
    No explicit implementation for
      ‘meth’
    In the instance declaration for ‘C Int’
-}

and now I've convinced myself that there is no bug here at all, just wrong use of MINIMAL. A correct use in conjunction with DefaultSignatures would be if you had two methods in a class, each with a default implementation (using DefaultSignatures) in terms of the other. Then the default implementations should not count for satisfying MINIMAL!

comment:7 Changed 2 years ago by simonpj

surely the intention of the MINIMAL pragma was to warn if we end up with the meth = error "..." implementation that the compiler would supply if there was no other implementation at all

Not only that. Bur also

class Eq a where
  (==), (/=) :: a -> a -> Bool
  (==) a b = not (a /= b)
  (/=) a b = not (a == b)

Here we have code for both (==) and (/=) (no error here), but the MINIMAL pragma allows you to say that you should supply a "real" implementation for one or the other.

comment:8 Changed 2 years ago by basvandijk

Resolution: invalid
Status: newclosed

Thanks, I now understand MINIMAL better. Sorry for the noise.

comment:9 Changed 2 years ago by simonpj

Great. Could you suggest some improvements to the wording in the user manual that would make it easier to understand what it does? Maybe less ambiguous?

Better text would be v helpful.

Simon

Note: See TracTickets for help on using tickets.