No problem - I assumed that Conal had some code that demonstrated the difference. On 29 Apr 2014 19:36, "Richard Eisenberg" <[email protected]> wrote:
> I’m surely agreed about the Note -- I also did some helpful name-changing > of related functions and am validating my work as I write. > > About a test case: this seems like a challenging thing to test. The new > code affects only coercion optimizations. The only time the extra code > would be triggered is when we, say, have a coercion built with transitivity > buried inside of a TyConAppCo that needs to be split up in order to > interact with the use of an axiom. (The obscurity of the case is one reason > I believe I didn’t implement it this way to begin with!) So, I think I’m > going to leave off the test case for now. > > Richard > > On Apr 29, 2014, at 1:08 PM, Simon Marlow <[email protected]> wrote: > > > On 29/04/2014 08:56, Simon Peyton Jones wrote: > >> Richard, > >> > >> Looks good, but *surely* worth a Note [unSubCo] in the file? It's > manifestly non-obvious or you'd have done this from day 1. Moreover, the > *reason* Conal needed it, and the new optimsations it enables, are also > non-obvious and deserve examples! > > > > And let's not forget a test too :-) > > > > Cheers, > > Simon > > > > > >> Evernyone: my obsession with Notes is born of the hours I have spent > staring at code that has cases I don't understand; or "fixing" something > turned out to be there for a good but undocumented reason. > >> > >> Going back to unSubCo, I think the start of the documentation could be > >> if g :: t1 ~R t2 > >> then unSubCo g :: t ~N t2 > >> (if it succeeds at all). > >> > >> But why do we need it? And what optimisations do we get? > >> > >> Thanks > >> > >> Simon > >> > >> > >> | -----Original Message----- > >> | From: ghc-commits [mailto:[email protected]] On Behalf > Of > >> | [email protected] > >> | Sent: 29 April 2014 00:51 > >> | To: [email protected] > >> | Subject: [commit: ghc] master: Improve implementation of > unSubCo_maybe. > >> | (a3896ab) > >> | > >> | Repository : ssh://[email protected]/ghc > >> | > >> | On branch : master > >> | Link : > >> | > http://ghc.haskell.org/trac/ghc/changeset/a3896ab5d2dc88160f710705bf23e6 > >> | e25e327da5/ghc > >> | > >> | >--------------------------------------------------------------- > >> | > >> | commit a3896ab5d2dc88160f710705bf23e6e25e327da5 > >> | Author: Richard Eisenberg <[email protected]> > >> | Date: Mon Apr 28 13:33:13 2014 -0400 > >> | > >> | Improve implementation of unSubCo_maybe. > >> | > >> | This is the result of an email conversation (off list) with > >> | Conal Elliott, who needed a stronger unSubCo_maybe. This > >> | commit adds cases to upgrade the role of a coercion when > >> | recursion is necessary to do say (for example, for a use of > >> | TransCo). As a side effect, more coercion optimizations are > >> | now possible. > >> | > >> | This was not done previously because unSubCo_maybe was used > >> | only during coercion optimization, and the recursive cases > >> | looked to be unlikely. However, adding them can cause no harm. > >> | > >> | unSubCo_maybe is now also exported from Coercion, for use > >> | cases like Conal's. > >> | > >> | > >> | >--------------------------------------------------------------- > >> | > >> | a3896ab5d2dc88160f710705bf23e6e25e327da5 > >> | compiler/types/Coercion.lhs | 20 +++++++++++++++----- > >> | 1 file changed, 15 insertions(+), 5 deletions(-) > >> | > >> | diff --git a/compiler/types/Coercion.lhs b/compiler/types/Coercion.lhs > >> | index af2b2fa..f60fcbd 100644 > >> | --- a/compiler/types/Coercion.lhs > >> | +++ b/compiler/types/Coercion.lhs > >> | @@ -38,7 +38,7 @@ module Coercion ( > >> | splitAppCo_maybe, > >> | splitForAllCo_maybe, > >> | nthRole, tyConRolesX, > >> | - nextRole, > >> | + nextRole, unSubCo_maybe, > >> | > >> | -- ** Coercion variables > >> | mkCoVar, isCoVar, isCoVarType, coVarName, setCoVarName, > >> | setCoVarUnique, @@ -1051,16 +1051,26 @@ maybeSubCo2 r1 r2 co > >> | > >> | -- if co is Nominal, returns it; otherwise, unwraps a SubCo; > otherwise, > >> | fails unSubCo_maybe :: Coercion -> Maybe Coercion > >> | +unSubCo_maybe co > >> | + | Nominal <- coercionRole co = Just co > >> | unSubCo_maybe (SubCo co) = Just co > >> | unSubCo_maybe (Refl _ ty) = Just $ Refl Nominal ty -unSubCo_maybe > >> | (TyConAppCo Representational tc cos) > >> | - = do { cos' <- mapM unSubCo_maybe cos > >> | +unSubCo_maybe (TyConAppCo Representational tc coes) > >> | + = do { cos' <- mapM unSubCo_maybe coes > >> | ; return $ TyConAppCo Nominal tc cos' } unSubCo_maybe (UnivCo > >> | Representational ty1 ty2) = Just $ UnivCo Nominal ty1 ty2 > >> | -- We do *not* promote UnivCo Phantom, as that's unsafe. > >> | -- UnivCo Nominal is no more unsafe than UnivCo Representational - > >> | unSubCo_maybe co > >> | - | Nominal <- coercionRole co = Just co > >> | +unSubCo_maybe (TransCo co1 co2) > >> | + = TransCo <$> unSubCo_maybe co1 <*> unSubCo_maybe co2 unSubCo_maybe > >> | +(AppCo co1 co2) > >> | + = AppCo <$> unSubCo_maybe co1 <*> pure co2 unSubCo_maybe (ForAllCo > tv > >> | +co) > >> | + = ForAllCo tv <$> unSubCo_maybe co > >> | +unSubCo_maybe (NthCo n co) > >> | + = NthCo n <$> unSubCo_maybe co > >> | +unSubCo_maybe (InstCo co ty) > >> | + = InstCo <$> unSubCo_maybe co <*> pure ty > >> | unSubCo_maybe _ = Nothing > >> | > >> | -- takes any coercion and turns it into a Phantom coercion > >> | > >> | _______________________________________________ > >> | ghc-commits mailing list > >> | [email protected] > >> | http://www.haskell.org/mailman/listinfo/ghc-commits > >> _______________________________________________ > >> ghc-devs mailing list > >> [email protected] > >> http://www.haskell.org/mailman/listinfo/ghc-devs > >> > > _______________________________________________ > > ghc-devs mailing list > > [email protected] > > http://www.haskell.org/mailman/listinfo/ghc-devs > >
_______________________________________________ ghc-devs mailing list [email protected] http://www.haskell.org/mailman/listinfo/ghc-devs
