classP recently deleted from TH.Lib

[Gabor Greif]

On 5/12/14, Richard Eisenberg <eir at cis.upenn.edu> wrote:
> This one was my fault/decision. The TH.Lib functions tend to mirror exactly
> the constructors in TH.Syntax. We removed the constructors (for good reason

Hmmm, okay, I see you added `equalityT` which is just a shallow
wrapper around a new constructor.

> -- predicates and types really are the same now), so I thought it best to
> remove the TH.Lib function, too. In the cases where code had to be updated,
> would keeping classP in be enough to prevent the breakage?

Cannot speak for many packages, but adding `classP` back would make
llvm-general-pure compilable again with HEAD. Maybe we could add it
back and mark it as deprecated?

>
> We could, of course, just leave the functions there with new
> implementations, but that feels like it could accumulate legacy functions
> over time.

I see no serious drawbacks with an explicitly deprecated API.

>
> Here are some ideas for this, and other similar situations, going forward:
> 1) Don't remove functions from TH.Lib unless absolutely necessary (which
> should probably never happen).
> 2) Remove functions from TH.Lib, but support some other package
> ("th-compat") which fills in the compatibility gap. This package would not
> be tied in with GHC. It would use CPP to export functions in order to remain
> compatible with a range of GHC versions. In this example, the package would
> export a fresh classP in 7.9+ but just re-export TH's in 7.8-.

This sounds like a good idea, but the dependent packages would need to
adopt this scheme. I.e. import `classP` from th-compat, and hiding
`classP` from import TH.Lib.

Not sure what the best practices are. Anyway, if this scheme works,
one could remove TH.Lib.classP from GHC 7.12.

> 3) Do what I've done -- keep TH.Lib parallel with TH.Syntax and leave it to
> users to sort it out as they see fit.

This would imply that some packages need to perform preprocessor
conditional compilation to include `classP`. IMHO the worst option, as
it causes duplication.

Just my few cents.

Cheers,

    Gabor

>
> It is worth noting that a change of this sort in TH.Lib corresponds to a
> breaking change in TH.Syntax. But, perhaps most people rely on Lib and not
> on Syntax.
>
> I'm happy to follow what the community thinks is best here -- I don't have
> any vested opinion.
>
> Thanks -- and sorry for the breakage!
> Richard
>
> On May 12, 2014, at 10:35 AM, Johan Tibell <johan.tibell at gmail.com> wrote:
>
>> That would be nice. I had to fix some breakage caused by this in one of
>> Bryan's libraries.
>>
>>
>> On Mon, May 12, 2014 at 4:12 PM, Gabor Greif <ggreif at gmail.com> wrote:
>> The last two commits from (Apr 9) on
>>
>> <https://github.com/ghc/packages-template-haskell/commits/master/Language/Haskell/TH/Lib.hs>
>>
>> removed a helper to construct applied type class constraints (`classP`).
>>
>> Some packages (notably `llvm-general-pure`) though, depend on it.
>>
>> Can we add back something like this:
>>
>> {{{
>> classP :: Name -> [Q Type] -> Q Pred
>> classP cla tys
>>   = do
>>       tysl <- sequence tys
>>       return (foldl' AppT (ConT cla) tysl)
>> }}}
>>
>> and export it again? Or is there such a helper with a different name
>> already?
>>
>> Cheers,
>>
>>     Gabor
>> _______________________________________________
>> ghc-devs mailing list
>> ghc-devs at haskell.org
>> http://www.haskell.org/mailman/listinfo/ghc-devs
>>
>
>