Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Make Q (TExp a) into a newtype #195

Merged
merged 6 commits into from May 26, 2020
Merged

Conversation

mpickering
Copy link
Contributor

@mpickering mpickering commented Jan 15, 2019

The proposal has been accepted; the following discussion is mostly of historic interest.


View rendered

@simonpj
Copy link
Contributor

simonpj commented Jan 15, 2019

Formatting is funny... fix?

Why just Q (TExp a)? Would it not make sense to do the same for Q Exp?

splices can fail to run which goes against the principle of typed staged programming.

Can you be more explicit? What is "the principle of typed staged programming"? What splices can fail to run? Examples would help a lot.

It is hard to properly write instances for Q (TExp a)

Can you give examples of what instances you might want to write, and what evolutions you have to go through?

@mpickering
Copy link
Contributor Author

Why just Q (TExp a)? Would it not make sense to do the same for Q Exp?

I don't have a real opinion about untyped template haskell but it seems to me that it's primarily about constructing code fragments explicitly and unsafely using operations from the Q monad so making it abstract wouldn't be appropriate.

splices can fail to run which goes against the principle of typed staged programming.

Can you be more explicit? What is "the principle of typed staged programming"? What splices can fail to run? Examples would help a lot.

The principle is that we should never fail when running a splice. Errors should occur whilst constructing the value that we want to splice in so that we ensure that we can only construct well-scoped and well-typed terms.

For a particular situation, imagine if you call qAddTopDecls with a declaration which is invalid in some manner then it will cause a failure when running the splice.

It is hard to properly write instances for Q (TExp a)

Can you give examples of what instances you might want to write, and what evolutions you have to go through?

I added this example of a simple DSL where it is useful to write an instance for Code.

class Lang r where                                                        
  _int :: Int -> r Int                                                    
   _if  :: r Bool -> r a -> r a -> r a                                     
                                                                            
instance Lang Identity where                                              
   _int = Identity                                                         
   _if (Identity b) (Identity t) (Identity f) = Identity (if b then t else f)
                                                                     
instance Lang Code where                                                  
   _int = liftTyped                                                        
   _if cb ct cf = [|| if $$cb then $$ct else $$cf ||] 

@mpickering
Copy link
Contributor Author

Seems like there is little interest in this proposal. How long does the committee usually wait before considering them?

@nomeata
Copy link
Contributor

nomeata commented Jan 23, 2019

I guess a week is reasonable. Has this proposal been advertised anywhere (mailing lists, reddit), i.e. is there a good chance that people who care about TH are aware of it? But if it is small, and obviously desirable, then a week of silence is reasonable.

@harpocrates
Copy link
Contributor

Isn't this going to break pretty much every non-trivial piece of TH in existence? More importantly, it looks like there isn't going to be any easy way to fix that code in a backwards compatible fashion. It would be great if the proposal could talk a bit more about what this will break and what the recommendation will be to unbreak it...

In short: the breakage to benefit ratio seems a bit high.

@mpickering
Copy link
Contributor Author

@harpocrates Firstly the potential breakage is only in programs which use typed template haskell of which there are not that many judging by the very few bug reports and number of quite serious bugs. However, users who use the API safely by quoting and splicing would just have to change their type signatures with this modification. Users who use the operations from the Q monad might have to make some more serious modifications.

Reflecting on this compared to other changes like, making Semigroup a superclass of Monoid which broke every single program containing a monoid definition, the breakage here is minimal.

@mpickering
Copy link
Contributor Author

@Ericson2314 what is your objection to this proposal? Will it break the way you use typed template haskell?

@harpocrates
Copy link
Contributor

@harpocrates Firstly the potential breakage is only in programs which use typed template haskell of which there are not that many judging by the very few bug reports and number of quite serious bugs. However, users who use the API safely by quoting and splicing would just have to change their type signatures with this modification. Users who use the operations from the Q monad might have to make some more serious modifications.

Reflecting on this compared to other changes like, making Semigroup a superclass of Monoid which broke every single program containing a monoid definition, the breakage here is minimal.

Yes, Semigroup was a painful change, but there was a straightforward way of adapting code in a backwards compatible way (add a conditional package dependency on semigroups or guard your Semigroup instance with CPP).

Also, you propose changing Lift to have a liftTyped :: a -> Code a method, but today it only has a lift :: a -> Q Exp method. This means that all Lift instances will also be broken (fixable in a backwards compatible manner only with CPP). I'm guessing you also intend to define a lift :: Lift a => a -> Q Exp in terms of liftTyped, right?

@mpickering
Copy link
Contributor Author

What happens with Lift depends on your proposal #175. This can be easily adapted.

This proposal fixes a mistake in the original design, there is no very straightforward backwards compatibility story precisely because Q (TExp a) is not abstract currently.

One option for a backwards compatible change (that I am strongly against) is introducing a new module which exposes the new interface and to teach GHC to be able to quote either Code or Q (TExp a). This would introduce some kind of limited overloading in the manner I suggested in https://ghc.haskell.org/trac/ghc/ticket/16178.

@gbaz
Copy link

gbaz commented Jan 23, 2019

The principle is that we should never fail when running a splice. Errors should occur whilst constructing the value that we want to splice in so that we ensure that we can only construct well-scoped and well-typed terms.

I don't think this is a guiding principle that makes sense to me. The promise is not that we never fail -- it is that we produce well typed code, if we happen to succeed. It is always possible to fail -- e.g., exceptions, nontermination, etc.

@Ericson2314
Copy link
Contributor

Ericson2314 commented Jan 24, 2019

Yes mainly I was confused on what failures there are, but secondarily exactly what @gbaz said. There are issues with Q's expansiveness (e.g. IO), but I don't think they are particular to typed TH.

@mpickering
Copy link
Contributor Author

The principle is that we should never fail when running a splice. Errors should occur whilst constructing the value that we want to splice in so that we ensure that we can only construct well-scoped and well-typed terms.

I don't think this is a guiding principle that makes sense to me. The promise is not that we never fail -- it is that we produce well typed code, if we happen to succeed. It is always possible to fail -- e.g., exceptions, nontermination, etc.

Gershom, you are right to clarify this and I agree that I assume people write programs that terminate and don't throw exceptions but I don't see how it is related to the content of the proposal? Which is

  1. allow people to write instances for Q (TExp and
  2. prevent people doing unsafe things like qAddTopDecls which can definitely fail the guarantee regardless of how you phrase the guarantee.

If you are referring to (2), the point is that if a user throws an exception in their program then they should expect it to fail. However, if they use the currently advertised API by calling qAddTopDecls then the program can fail at splice time. So it is the distinction that the advertised API shouldn't provide unsafe functions rather than preventing people shooting themselves in their own foot.

Yes mainly I was confused on what failures there are, but secondarily exactly what @gbaz said. There are issues with Q's expansiveness (e.g. IO), but I don't think they are particular to typed TH.

@Ericson2314 What do you mean by "what failures there are"? The issue you mention about Q is irrelevant here. In fact, if it is harder to use operations from Q that will help cross-compilation cases.

@Ericson2314
Copy link
Contributor

Ericson2314 commented Jan 26, 2019

@mpickering

@Ericson2314 What do you mean by "what failures there are"? The issue you mention about Q is irrelevant here. In fact, if it is harder to use operations from Q that will help cross-compilation cases.

Yes, the things I brought up indeed are irrelevant here, that was my point! When you write "The principle is that we should never fail when running a splice." I couldn't think of any failures that relate to typed template Haskell in particular.

So if like @gbaz I think splicing failure in general has nothing to do with typed TH (the point is to prevent errors after splicing (type errors) not during), and there are no specific splicing failures which are relevant to typed TH, then splicing failure disciplines and the typedness of template Haskell variants are truly orthogonal.

@mpickering
Copy link
Contributor Author

So to summarise the discussion here. There is one objection of substance which is that of backwards compatibility. However, I would consider it an absolute travesty if the proposal was rejected on these grounds due to the far more invasive breakages which happen every release. Given that no users have come forward to express this concern, I believe that is should also be disregarded.

I also grepped my old copy of Hackage (from 2017) for occurences of [|| and found the following usages:

@int-index
Copy link
Contributor

int-index commented Feb 3, 2019

@mpickering I wonder why it must be a newtype over Q (TExp a) rather than WQ (TExp a) where WQ is some sort of "weakened Q" that does not offer a possibility of failure or reification.

Indeed, the name Code suggests that this is just an AST for some expression, and I wouldn't expect splicing it to either fail or inspect its environment (by reification). It appears to me that the only thing that WQ needs to support is generating fresh variables, so it could be defined along the lines of

newtype WQ a = WQ { runWQ :: forall m. Monad m => (String -> m Name) -> m a }

It is obvious how to interpret it in Q:

qRunWQ :: WQ a -> Q a
qRunWQ = runWQ newName

And this opens up a possibility of writing other interpreters for Code.

@mpickering
Copy link
Contributor Author

@int-index My primary problem is that you can't write instances for Q (TExp a) easily.

Generating fresh variables is also one of the most dangerous operations! It's very easy to end up with an AST which contains unbound variables.

(FWIW, I have also implemented an alternative serialisation mechanism for quotes which serialises core expressions rather than to the template haskell AST, this is sufficient if you don't need to inspect it. This isn't relevant to this proposal though.)

@int-index
Copy link
Contributor

Generating fresh variables is also one of the most dangerous operations! It's very easy to end up with an AST which contains unbound variables.

At least that's one dangerous operation instead of the ability to embed arbitrary IO via Q!

Generating fresh variables can be easily done in pure code with the State Int, and there's something satisfying about being able to splice Code in a pure context.

It would help me greatly to reason about what Code can do and what it cannot do.

In case the only motivation is merely to write instances, I suggest the name TExpQ which is explicit about having the full power of Q.

@yallop
Copy link

yallop commented Feb 4, 2019

On the original proposal: it would help in the cases where you need need an instance for Code, but not the cases where you need an instance for Code T (for some T). In my (limited, possibly atypical) experience the latter are more common.

On the "weakened Q" proposal:

It appears to me that the only thing that WQ needs to support is generating fresh variables

It's often useful to mix code generation with other effects --- in particular, effects that can make non-local transformations to code. For example, this is from the abstract of Kameyama, Kiselyov and Shan's (2014) Combinators for impure yet hygienic code generation:

Code generation is the leading approach to making high-performance software reusable. Effects are indispensable in code generators, whether to report failures or to insert let-statements and if- guards.

So I'm concerned that the weakened WQ is likely to be a bit too weak in practice.

@mpickering
Copy link
Contributor Author

You're right that it wouldn't "help" instances for Code T but it wouldn't hinder them either. Given the popularity of classes with higher-kinded parameters like Functor, Applicative, Monad and so on, the usefulness of an instance for Code can be easily imagined.

I think the best path to "weakened Q" is to disentangle Q from the quoting mechanism by making brackets and splices overloaded. This way you can then easily embed them into your own library which provides effectful combinators for manipulating the code fragments. https://ghc.haskell.org/trac/ghc/ticket/16178

@int-index
Copy link
Contributor

int-index commented Feb 5, 2019

@mpickering If we make quoting and splicing overloaded, as you propose in Trac #16178, would it not subsume this proposal?

class IsBracket p where
  fromBracket :: Q (TExp t) -> p t

class IsSplice p where
  toBracket :: p t -> Q (TExp t)

Then separately, perhaps in another library,

newtype Code a = Code { unCode :: Q (TExp a) }

instance IsBracket Code where
  fromBracket = Code

instance IsSplice Code where
  toBracket = unCode

@int-index
Copy link
Contributor

I'm not sure how it would help the WQ case, there is seemingly no way to define fromBracket :: Q (TExp t) -> WQ (TExp t).

@mpickering
Copy link
Contributor Author

@int-index I think the point would be to make fromBracket :: WQTExp t -> p t and then embed WQ actions inside Q. The quote generation does use some operations from Q but it might just be ones which make names. I would need to look at the code.

There would still be the same problems with backwards compatibility with the overloaded proposal as you can't easily write an instance for IsBracket for Q . TExp. So making Q (TExp a newtype and then later adding the overloading would be a possible implementation path.

@int-index
Copy link
Contributor

OK. I still have a few gripes about the name Code. It looks nice, but for me it has connotations that do not hold true for Q (TExp t).

  1. Types/declarations are code, but they cannot be represented by Q (TExp t).
  2. Doing any sort of effects (even name generation, come to think of it) is not part of the code. TExp is code, Q (TExp t) is code generation. And if WQ becomes a reality, then Q (TExp t) is just one possible way of code generation.

@gbaz
Copy link

gbaz commented Feb 5, 2019

Improving support for instances is a good motivation for something. But this "semi-purity" stuff feels weird to me. Another reason is that I think it makes sense to have IO effects inside Q TExp. For example, you might want to do IO and based on that generate different code, but all the code being of the same type.

So having a weakened Q seems entirely orthogonal to the question of having a newtype for the purpose of writing instances, and I think the fromBracket approach is probably a much better way to tackle the latter.

@mpickering
Copy link
Contributor Author

Gershom,I think the intention is that "weakened Q" supports only the operations that are necessary to make the representations of code. This doesn't include doing IO. Then WQ embeds into Q which supports things like liftIO. GHC still interprets Q but other consumers can more easily use quotation for runtime code generation by providing their own interpretation of WQ which doesn't include difficult to implement operations like reification.

I agree you're right that this is orthogonal to the proposal.

@mpickering
Copy link
Contributor Author

Is there anything else blocking this proposal? All objections have been dealt with.

@nomeata nomeata added the Pending committee review The committee needs to evaluate the proposal and make a decision label Mar 7, 2019
@simonpj
Copy link
Contributor

simonpj commented Mar 8, 2019

Earlier I wrote

Would it not make sense to do the same for Q Exp?

To answer my own question: we also have Q Type, Q Decl, Q Pattern. But we do not have Q (TType a) (whatever that might mean), etc.

That is, in the untyped world we construct fragments of many different syntactic categories. But in the typed TH world we can only construct expressions. To me that helps explain why we don't want lots of newtypes for Q Exp but we do for Q (TExp a).

@Ericson2314
Copy link
Contributor

Ericson2314 commented Mar 8, 2019

@mpickering While you pointed out correctly that IsBracket should use WQ, and we embed WQ into Q with an IsBracket instance., hat doesn't address @gbaz's other point that IsBracket/fromBracket solves the same problem as Code. I also agree that fromBracket + WQ is the right approach, and one that subsumes Code.

Seeing that you yourself proposed IsBracket, would you still want Code after? Even if you did, there's no reason to define it in template-haskell is there?

@phadej
Copy link
Contributor

phadej commented May 22, 2020

@Ericson2314 Foo a -> (a -> Bar b) -> Foo b is a module over monad operation. https://ncatlab.org/nlab/show/module+over+a+monad#modules, there are left and right ones (depending from which side you "join").

EDIT; TL;DR you might want to have joinCode :: m (Code m a) -> Code m a for completeness. Then you would write:

myCode :: ... => Code m a
myCode = joinCode $ do
  x <- someSideEffect
  return (makeCodeWith x)

Simple.

@Ericson2314
Copy link
Contributor

Ericson2314 commented May 22, 2020

I am just going to retract my parenthetical, as anything about Code.do is far less important to me than the missing pieces of the motivation.

@phadej
Copy link
Contributor

phadej commented May 22, 2020

If TExp is still exported, shouldn't

unsafeTExpCoerce :: m Exp -> m (TExp a) 

be kept at current type, and rather new

unsafeCodeCoerce :: m Exp -> Code m a

to be introduced. It's weird to have function with TExp in name to work on Code.

Also note, that we have unType :: TExp a -> Exp and different unTypeQ :: TExpQ a -> ExpQ at the moment. Proposal talks about the first one with the type of second one. Here as well, I'd try to come up with new name, rather than changing the existing signature.

@phadej
Copy link
Contributor

phadej commented May 22, 2020

A something to consider. I just run (into my previously mentioned experiment), that

instance (Lift a, Num a, Quote m) => Num (Code m a) where
    fromInteger x = liftTyped (fromInteger x)
    ...

Would be nice to have. And same for IsString.

@simonpj
Copy link
Contributor

simonpj commented May 22, 2020

The API for code is already in the proposal (in one place) in the "Proposed Change Specification".

My apologies -- it's looking much clearer now, thank you.
Remaining questions:

  • What other operations, if any, are available for TExp? That is, what
    functions (other than those listed here) have TExp in their type
    signatures. I thikn the answer may be "none", but I'm not certain.
    It would be great to say so explicitly in this proposal; and it
    would be good to include the API for TExp, if it has anything
    further.

  • Do you really want to specify handleState in this API, since it is just a specialisation of hoistCode? And if you do, do you want to specialise for Q or could you have

    handleState :: Code (StateT Int m) a -> Code m a
    
  • Since we have liftCode and examineCode, can we make Code opaque?
    That gives a little more wiggle room if you ever want to change its implementation later.

@mpickering
Copy link
Contributor Author

@Ericson2314 Using effects if a fundamental part of code generation, for example, if you construct a code generator which constructs division, then you probably want to fail at compile time if you would construct a division by zero. Therefore having the monadic context is a big advantage that TTH has over metaocaml and dotty. Another example, I have implemented let insertion using the monadic context ( https://github.com/mpickering/let-insert/blob/master/LetInsert.hs ).

@phadej

Thanks, I was going to let all this stuff shake out in the implementation where it becomes clearer.
So I will update the proposal to

  • Introduce unsafeCodeCoerce
  • unTypeCode as the variant of unTypeQ and keep unType.

@simonpj

  • You can use unsafeTExpCoerce, TExp or unType to interact with TExp. This hasn't changed because of this proposal.
  • handleState is an example of using hoistCode. I modified the comment to reflect this.
  • Making Code opaque is just annoying rather than serving any measurable benefit.

I have also started the implementation.

@simonpj
Copy link
Contributor

simonpj commented May 26, 2020

Thanks Matthew. Can you just add at the beginning of Change Specification that the section gives the complete API for both Code and TExp?

Also, I was a bit boggled by

unsafeTExpCoerce :: Quote m => m Exp -> m (TExp a)
TExp :: Exp -> TExp a
unType :: TExp a -> Exp

We really really shouldn't be exposing TExp as a constructor. We should have

unsafeExpToTExp :: Exp -> TExp a

(It's a pity that unsafeTExpCoerce is already taken.) I know these this is a pre-existing hole in our type system. But this would be a good moment to fix it.

And there is no reason for unsafeTExpCoerce to need Quote m; it only needs Functor m. Perhaps not a big deal.

Moreover, like bindCode, it's a pure abbreviation, with a two-token definition, so maybe we can give it a definition, as you do for bindCode:

unsafeTExpCoerce = fmap unsafeExpToTExp

@mpickering
Copy link
Contributor Author

Updated the proposal again with your comments. Patch is also available to review:

https://gitlab.haskell.org/ghc/ghc/-/merge_requests/3358

It makes no difference other than potentially annoying users to hide TExp.

The reason unsafeTExpCoerce takes a Quote m constraint is that it made the implementation a little bit easier. I think I could probably change that quite easily though but not in scope for this proposal.

@simonpj
Copy link
Contributor

simonpj commented May 26, 2020

It makes no difference other than potentially annoying users to hide TExp.

Bu tit makes a big difference? If I see "unsafe" something I know something is unsafe. If I see TExp x I have not such clue. Yet every such occurrence is a potential seg-fault. Asking clients to use unsafeExpToTExp is a positive feature, I think.

I accept that this bug predates this proposal, but the proposal refines the client API of Code and TExp. Let's get it right.

The reason unsafeTExpCoerce takes a Quote m constraint is that it made the implementation a little bit easier.

Interesting. The implementation is:

                      Quote m => m Exp -> m (TExp a)
unsafeTExpCoerce m = do { e <- m
                        ; return (TExp e) }

That uses Monad. Does Quote make it easier. And as I say fmap unsafeExpToTExp is shorter still. Maybe I'm missing something.

@Ericson2314
Copy link
Contributor

Ericson2314 commented May 26, 2020

Using effects if a fundamental part of code generation, for example, if you construct a code generator which constructs division, then you probably want to fail at compile time if you would construct a division by zero. Therefore having the monadic context is a big advantage that TTH has over metaocaml and dotty. Another example, I have implemented let insertion using the monadic context ( https://github.com/mpickering/let-insert/blob/master/LetInsert.hs ).

Yes, I agree the monad is fundamental! That's why I think it's weird to put it in a newtype. I've never seem anyone newtype M (G a) where G isn't even a functor (i.e. has no relevant semantics WRT the Monad, it's just somewhat we happen to be producing), and yet that we are doing this here.

The motivation of "just avoiding some wrapping/unwrapping" still seems incredibly weak to me, put forth by those that write the code at the expensive of those that need to read the code later. Imagine if you need to read your coworker's or classmate's TTH code, and are new to this stuff yourself. Would you rather read something with plain old wrapping/unwrapping, or a Code.do?

@Ericson2314
Copy link
Contributor

Ericson2314 commented May 26, 2020

On a meta note, I feel bad being the chief nay-sayer here. I have been and am working with @mpickering on other things, for example. And I completely understand it's been frustrating for him to see this to drag on and on, especially when it seemed on the verge of acceptance a few times over already, only to go back into limbo for another month+. I take no joy in dragging it on further. But just because a proposal is overdue getting resolved doesn't mean it deserves to be accepted.

I am really surprised there really hasn't been much discussion of the motivation, one way or the other, from others. Can somebody who is not a current TTH user say why this is compelling to them?

@mpickering
Copy link
Contributor Author

@simonpj

You can't get into the situation of a segfault by using unsafeTExpCoerce (or TExp) so it is less unsafe than say, unsafeCoerce. Modifying TExp is out of scope for this proposal in my opinion. If it had been suggested at the start then perhaps I would be more willing to modify the scope but it's far too late for that. I do not wish to modify how TExp behaves.

Interesting. The implementation is:

I am talking about when I implemented Overloaded Quotations internally to the compiler, not the Haskell implementation. In that case you have convenient access to a wrapper which applies the Quote dictionary but getting access to the Functor constraint was a bit more work. If you open a GHC ticket then I could probably modify this. It is also not in scope for this proposal though, which does not modify this function.

In this thread, a large proportion of known users indicate that this change would be beneficial and yet I have spent an inordinate amount of emotional energy attempting to make it happen. Please can we move on from this proposal!

@int-index
Copy link
Contributor

int-index commented May 26, 2020

I am really surprised there really hasn't been much discussion of the motivation, one way or the other, from others. Can somebody who is not a current TTH user say why this is compelling to them?

Personally, I don't find this proposal compelling at all. Let's consider the three points of the motivation section:

  1. Defining EDSLs using TTH

    class Lang r where
      _int :: Int -> r Int
      _if  :: r Bool -> r a -> r a -> r a
    
    instance Lang Identity where
      _int = Identity
      _if (Identity b) (Identity t) (Identity f) = Identity (if b then t else f)
    
    instance Quote m => Lang (Code m) where
      _int = liftTyped
      _if cb ct cf = [|| if $$cb then $$ct else $$cf ||]
    

    But there's already manual newtype wrapping/unwrapping for Identity. I would even say it'd be more consistent to require it for Code as well:

    instance Quote m => Lang (Code m) where
      _int = Code . liftTyped
      _if (Code cb) (Code ct) (Code cf) = Code [|| if $$cb then $$ct else $$cf ||]
    

    So this particular example is hardly compelling. Just define Code separately, without changing GHC, and use it to wrap/unwrap by hand.

  2. Putting Code in heterogeneous collections. Once again, I see the need for Code, but I don't see why one would avoid manual newtype wrapping/unwrapping. Just introduce wrappers for lookup and insert:

    lookupTExp :: forall k f v. GCompare k => k v -> DMap k (Code f) -> Maybe (f (TExp v))
    insertTExp :: forall k f v. GCompare k => k v -> f (TExp v) -> DMap k (Code f) -> DMap k (Code f)
    
  3. m (TExp a) is ugly to read and write, understanding Code m a is easier. This is a weak reason but one everyone can surely agree with.

    Well, this reason I cannot take seriously, because it is so obviously false. Not everyone can agree that Code m a is easier to understand than m (TExp a) (I don't think it's easier), nor do I believe that m (TExp a) is ugly. It's fine.

So, no part of the motivation seems compelling to me.

That said, I don't feel too strongly either way, whereas @mpickering seems to care about this change, so I try not to object too strenuously or prevent this proposal from getting accepted. If there was voting I'd say it's a weak -0.5 from me for technical reasons, but +1 for social reasons (let people have their changes), totaling +0.5

@phadej
Copy link
Contributor

phadej commented May 26, 2020

Putting Code in heterogeneous collections.

-Code [|| 1 ||] :* Code [|| True ||] :* Code [|| "foo" ||] :* Nil
+[|| 1 ||] :* [|| True ||] :* [|| "foo" |]] :* Nil

having written (yet unpublished) library which has code like that, the upper variant is simply not user friendly for public API (It would fine internally, and that's what it looks like now there).

Similarly, the using of EDSL would be not nice with wrapping and unwrapping in the user code. Instances can be ugly, but writing

if_ (Code [|| foo $$x ||]) ... ... has that unnecessary Code there. It's just noise around already quite noisy [|| ... ||].


Defining wrappers: I see that one as an example for this proposal. This proposal removes the need to define these wrappers.


EDIT: The changes in this proposal improve things for all TTH usages I can imagine myself, and don't make anything worse. I'm surprised by amount of low-quality counterexamples.

@int-index
Copy link
Contributor

int-index commented May 26, 2020

@phadej The :* example is a testament to the issues of heterogeneous collections, which are not exclusive to TTH. If you have HListF Identity, you also must write Identity 1 :* Identity "hello" :* Nil instead of the much nicer 1 :* "hello" :* Nil.

It's a reason to define different APIs for heterogeneous collections, not for TTH.

As with lookupTExp and insertTExp, my suggestion would be to define another operation:

consTExp e ts = Code e :* ts 

and then write it as:

[|| 1 ||] `consTExp`
[|| True ||] `consTExp`
[|| "foo" |]] `consTExp`
Nil

The if_ (Code [|| foo $$x ||]) ... ... indeed demonstrates the extra noise, but the same would be the case for Identity:

if_ (Identity x) ... ...

and don't make anything worse

That's a bold claim. It does make the use of monadic effects more complicated, hence the talk about bindCode, Code.do, etc.


Overall, I see how Code makes things nicer, but baking it into the compiler still seems like a half-measure to me. A proper solution would address the extra noise both in the Identity case and the Code case.

@Ericson2314
Copy link
Contributor

Ericson2314 commented May 26, 2020

OK thank you so much @int-index --- I no longer need to feel like the sole crazy person out here.

In fact agree with your -0.5 technical +1 social---this thing is not a big deal when way or the other, and I don't want to just waste @mpickering's time with tons of bureaucracy when TTH is basically being completely revamped.

But, if we're going to merge this for the social reasons and not the technical ones, we should at least be explicit about that. Just taking the motivation on blind faith seems no good.

@Ericson2314
Copy link
Contributor

Ericson2314 commented May 26, 2020

having written (yet unpublished) library which has code like that, the upper variant is simply not user friendly for public API (It would fine internally, and that's what it looks like now there).

I do think there is a desire to make the common cases for TTH as convenient as possible. But as @int-index says.

Overall, I see how Code makes things nicer, but baking it into the compiler still seems like a half-measure to me. A proper solution would address the extra noise both in the Identity case and the Code case.

I agree completely. And let's keep in mind the risk here. If one feature takes some measures to be more usable, it's mostly fine. But If every feature (e.g. TTH) tries to hack around deficiencies in other features (e.g. heterogeneous collection ergonomics), we end up with a more complex language where disparate features view each other with suspicion rather than cooperate.

For the sake of a whole, I am general pro not-hacking around things but waiting for fix the root causes of problems. We have to balance the completing interests of more beginner adoption of TTH now, vs a sensible API in conformance with the general idioms for writing Haskell later. Or, as we usually say, I am for avoiding success-at-all-costs.

@yav
Copy link
Contributor

yav commented May 26, 2020

Thank you everyone for you input---I think we've clarified the proposal, and maybe even come up with some new ideas for future improvements/interesting research directions.

Since this is a simple change, which names a common abstraction, and is clearly making current typed TH users happy, I think it is reasonable that we should accept it, and as the shepherd for this proposal I'll recommend to the committee that we do so.

@nomeata nomeata added Accepted The committee has decided to accept the proposal and removed Needs revision The proposal needs changes in response to shepherd or committee review feedback labels May 26, 2020
@nomeata nomeata merged commit 4326e64 into ghc-proposals:master May 26, 2020
@nomeata nomeata added the Implemented The proposal has been implemented and has hit GHC master label Jul 27, 2020
yallop added a commit to frex-project/haskell-frex that referenced this pull request Aug 19, 2020
Code is now a newtype, so that it can be used to instantiate FreeExt.

The tests still use Code for names throughout, which involves some extra
boilerplate.  But I think things'll become simpler again when we can
use some proposals that have recently been adopted in GHC:

    Make Q (TExp a) into a newtype
    ghc-proposals/ghc-proposals#195

    Overloaded Quotation Brackets
    ghc-proposals/ghc-proposals#246
yallop added a commit to frex-project/haskell-frex that referenced this pull request Aug 19, 2020
Code is now a newtype, so that it can be used to instantiate FreeExt.

The tests still use Code for names throughout, which involves some extra
boilerplate.  But I think things'll become simpler again when we can
use some proposals that have recently been adopted in GHC:

    Make Q (TExp a) into a newtype
    ghc-proposals/ghc-proposals#195

    Overloaded Quotation Brackets
    ghc-proposals/ghc-proposals#246
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Accepted The committee has decided to accept the proposal Implemented The proposal has been implemented and has hit GHC master
Development

Successfully merging this pull request may close these issues.

None yet