
On 05/13/2014 12:33 AM, Gabor Greif wrote:
On 5/12/14, Richard Eisenberg
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?
Why not simply patch llvm-general-pure? I have done this with quite a few packages when the TH change was made and it seems to work. If anything, it will make the authors realise something changed upstream and try to find out how.
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.
I think this is a fair solution until TH 2.9 is phased out.
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
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
wrote: The last two commits from (Apr 9) on https://github.com/ghc/packages-template-haskell/commits/master/Language/Has...
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@haskell.org http://www.haskell.org/mailman/listinfo/ghc-devs
_______________________________________________ ghc-devs mailing list ghc-devs@haskell.org http://www.haskell.org/mailman/listinfo/ghc-devs
-- Mateusz K.