Opened 3 years ago

Closed 3 years ago

#5499 closed feature request (wontfix)

Tagging constructors with record/product phantom type

Reported by: basvandijk Owned by:
Priority: normal Milestone:
Component: libraries (other) Version: 7.2.1
Keywords: Cc:
Operating System: Unknown/Multiple Architecture: Unknown/Multiple
Type of failure: None/Unknown Test Case:
Blocked By: Blocking:
Related Tickets: Differential Revisions:

Description

As mentioned in this discussion on the glasgow-haskell-users mailinglist. I would like to propose making the following changes to GHC.Generics:

Proposal

  • Add a phantom type to the C type which specifies whether it's a record or a normal product using the empty datatypes:
data Record
data Product
  • Optionally add the type synonyms:
type R1 = M1 (C Record)
type P1 = M1 (C Product)
  • Optionally remove the conIsRecord method of the Constructor type class.

Of course GHC has to be modified too, so that the generated Cs are properly tagged.

Motiviation

Having the information, whether a constructor is a record or not, statically available instead of only dynamically, has the following advantages:

  • More efficient: programs can make a static instead of a dynamic choice. (To be fair, the case branches on conIsRecord should in most cases be optimized away)
  • No unnecessary constraints for constructor instances:

See the code that triggered this proposal:

instance (Constructor c, GRecordToObject a, GToJSON a) => GToJSON (C1 c a) where
    gToJSON m1@(M1 x)
        | conIsRecord m1 = Object $ gRecordToObject x
        | otherwise = gToJSON x

Note the GRecordToObject constraint. This constraint is only really necessary in case of a record. Down the line this extra constraint requires the following ugly undefined instances:

instance GRecordToObject (a :+: b)  where gRecordToObject = undefined
instance GRecordToObject U1         where gRecordToObject = undefined
instance GRecordToObject (K1 i c)   where gRecordToObject = undefined
instance GRecordToObject (M1 i c f) where gRecordToObject = undefined

Process

Since GHC.Generics is part of the ghc-prim package I don't think this proposal has to go through the whole library submission process. But if it must I will have no problem supervising it.

Change History (9)

comment:1 Changed 3 years ago by dreixel

I see the advantage in doing this statically. But I would hope this can already be done. And indeed it seems like it can, unfortunately not with type families, but with functional dependencies...:

{-# LANGUAGE TypeFamilies #-}
{-# LANGUAGE TypeOperators #-}
{-# LANGUAGE DeriveGeneric #-}
{-# LANGUAGE UndecidableInstances #-}
{-# LANGUAGE FlexibleInstances #-}
{-# LANGUAGE OverlappingInstances #-}
{-# LANGUAGE MultiParamTypeClasses #-}
{-# LANGUAGE FunctionalDependencies #-}

module Test where

import GHC.Generics

data Pair a   = Pair a a                  deriving Generic
data Sum a b  = SL { recm :: a } | SR b   deriving Generic

data True
data False

type family IsRec (f :: * -> *) :: *
type instance IsRec (M1 D c f) = IsRec f
-- Simplified: we should instead use a type-level Or
type instance IsRec (f :+: g) = IsRec f
type instance IsRec (M1 C c f) = IsRec f
type instance IsRec (f :*: g) = IsRec f
type instance IsRec (M1 S NoSelector f) = False
-- Unfortunately we cannot give the following type instance, as they overlap
--type instance IsRec (M1 S c f) = IsRec f

-- With fundeps it works, though...
class IsRecord (f :: * -> *) b | f -> b
instance (IsRecord f b) => IsRecord (M1 D c f) b
-- Simplified: we should instead use a type-level Or
instance (IsRecord f b) => IsRecord (f :+: g) b
instance (IsRecord f b) => IsRecord (M1 C c f) b
-- For products we don't need an Or, either branch will do
instance (IsRecord f b) => IsRecord (f :*: g) b
instance IsRecord (M1 S NoSelector f) False
-- We cannot set b directly to True here...
instance (IsRecord f b) => IsRecord (M1 S c f) b
-- ... but we can do it in what comes after the M1 S, namely the K1:
instance IsRecord (K1 i c) True

-- Testing if a type uses record syntax, statically
class Test a where test :: a -> Bool

instance (IsRecord (Rep (Sum x y)) True ) => Test (Sum x y) where
  test _ = True

instance (IsRecord (Rep (Pair x) ) False) => Test (Pair x)  where
  test _ = False

comment:2 Changed 3 years ago by simonpj

Interesting. So, first, does Pedro's solution (dreixel) solve your problem, Bas?

Second, the "via fundep" issue is something I've been thinking about. I have some ideas, and Pedro is here on an internship in a week or two, so we'll take it up then.

Simon

comment:3 Changed 3 years ago by basvandijk

I will try out Pedro's solution this evening. One question: why can't we set the b to True in:

instance (IsRecord f b) => IsRecord (M1 S c f) b

comment:4 Changed 3 years ago by dreixel

Simply because GHC complains:

    Functional dependencies conflict between instance declarations:
      instance [overlap ok] IsRecord (M1 S NoSelector f) False
      instance [overlap ok] IsRecord (M1 S c f) True

comment:5 follow-up: Changed 3 years ago by basvandijk

I'm having a bit of trouble using IsRecord for changing the dynamic choice using conIsRecord into a static choice. This is the current code that uses a dynamic choice:

instance (Constructor c, GRecordToObject a, GToJSON a) => GToJSON (C1 c a) where
    gToJSON m1@(M1 x)
        | conIsRecord m1 = Object $ gRecordToObject x
        | otherwise = gToJSON x

Naively rewriting this to a static choice using IsRecord:

instance (IsRecord a True, GRecordToObject a) => GToJSON (C1 c a) where
    gToJSON = Object . gRecordToObject . unM1

instance (IsRecord a False, GToJSON a) => GToJSON (C1 c a) where
    gToJSON = gToJSON . unM1

Doesn't obviously work because we get a duplicate instance declarations error.

So somehow the information, whether a constructor is a record or not, has to move to the right of the =>.

comment:6 in reply to: ↑ 5 ; follow-up: Changed 3 years ago by dreixel

Replying to basvandijk:

So somehow the information, whether a constructor is a record or not, has to move to the right of the =>.

Right. I believe your GToJSON will also need the type-level boolean as a class argument.

comment:7 in reply to: ↑ 6 ; follow-up: Changed 3 years ago by basvandijk

Replying to dreixel:

Right. I believe your GToJSON will also need the type-level boolean as a class argument.

I was hoping I could localize this change only to constructors. I think it's weird to make a M1 D or a :+: an instance of IsRecord. "Recordness" (for lack of a better name) is a property of a constructor, not of a datatype or a sum.

Secondly I see a problem where GToJSON is used. If GToJSON had an extra b parameter which value does it get in:

class ToJSON a where
    toJSON   :: a -> Value

    default toJSON :: (Generic a, GToJSON (Rep a)) => a -> Value
    toJSON = gToJSON . from

comment:8 in reply to: ↑ 7 Changed 3 years ago by dreixel

Replying to basvandijk:

Secondly I see a problem where GToJSON is used. If GToJSON had an extra b parameter which value does it get in:

class ToJSON a where
    toJSON   :: a -> Value

    default toJSON :: (Generic a, GToJSON (Rep a)) => a -> Value
    toJSON = gToJSON . from

I *think* that a fresh type variable will do fine at that point.

comment:9 Changed 3 years ago by basvandijk

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

I solved it using both Pedro's IsRecord type-level predicate and the technique from Simon and Oleg for choosing an instance based on a context.

Although the solution requires some advanced techniques and some extra language extensions, I'm happy about it. I also managed to keep the change local (only the instance for constructors has changed).

A tag on C would make all of this a lot easier and doesn't require as much type-level computation. But it would mean changing GHC which may not be worth it. So I'm closing the ticket as wontfix.

Pedro and Simon, much thanks for your help!

Note: See TracTickets for help on using tickets.