VinylRecords / Vinyl

Extensible Records for Haskell. Pull requests welcome! Come visit us on #vinyl on freenode.
http://hackage.haskell.org/package/vinyl
MIT License
260 stars 49 forks source link

ToARec class not exported #157

Closed msakai closed 2 years ago

msakai commented 2 years ago

ToARec class was introduced in 9375bbd434ea3f8c71793fe9965c967725b9bafc but is not exported. This prevents us from using the toARec function polymorphically.

E.g. backdrop package has the following code:

-- | @since 0.2.6.3
instance (ReifyConstraint Backprop f rs, RMap rs, RApply rs, RecApplicative rs, NatToInt (RLength rs), RPureConstrained (IndexableField rs) rs)
      => Backprop (ARec f rs) where
    zero = toARec . zero . fromARec
    {-# INLINE zero #-}
    add xs ys = toARec $ add (fromARec xs) (fromARec ys)
    {-# INLINE add #-}
    one  = toARec . zero . fromARec
    {-# INLINE one #-}

If I compile the code with the current master of the vinyl (79dcb5911e6da07151d6e68d470a0dadb73a3ceb), it results in the following error:

.../backprop/src/Numeric/Backprop/Class.hs:988:12: error:
    • Could not deduce (Data.Vinyl.ARec.Internal.ToARec rs)
        arising from a use of ‘toARec’
      from the context: (ReifyConstraint Backprop f rs, RMap rs,
                         RApply rs, RecApplicative rs, NatToInt (RLength rs),
                         RPureConstrained (IndexableField rs) rs)
        bound by the instance declaration
        at src/Numeric/Backprop/Class.hs:(986,10)-(987,29)
    • In the first argument of ‘(.)’, namely ‘toARec’
      In the expression: toARec . zero . fromARec
      In an equation for ‘zero’: zero = toARec . zero . fromARec
    |
988 |     zero = toARec . zero . fromARec
    |            ^^^^^^

It is necessary to add ToARec rs to the class context, but it is impossible because the ToARec class is not exported.

acowley commented 2 years ago

@Philonous Any objection to exporting the class to support this use case?

Philonous commented 2 years ago

No, I think the only reason I didn't export it was not to bloat up the API.

Mikolaj commented 2 years ago

It seems this wasn't fixed in 0.14.1, right? I'm still having the error compiling backprop...

acowley commented 2 years ago

Sorry I lost track of getting this in, and thank you for the patient reminder!

Mikolaj commented 2 years ago

Thank you!