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
Lazy unboxed tuples / warn on unbanged strict patterns #35
Lazy unboxed tuples / warn on unbanged strict patterns #35
Conversation
|
||
is *not* an unlifted bind. | ||
|
||
Change (A) says that unlifted binds, and only unlifted binds, are strict by default. (Before Change (A), unboxed tuple patterns |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This paragraph is incomplete.
z = () | ||
where I# x = 4 | ||
|
||
This code is rejected by GHC 8.0 with an error. Change (B) makes this error into a warning. The binding is strict. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Typesetting mistake. Unindent this paragraph by one or two character, I guess.
Does this mean that
would still diverge (because
would now evaluate to |
@rwbarton , yes, precisely. It also means that
will evaluate to
will throw an exception. However, this kind of silliness is present in GHC 8. If we have
and then say
we can evaluate
now I get an error saying I should bang the pattern. Under this proposal, I would get the My |
To be clear, an "unlifted id", in the definition of an "unlifted bind", is an freshly-bound identifier whose type is unlifted, right? |
Also, might "unlifted pattern" be a better term than "unlifted bind"? IIUC the single where clause in foo = ()
where (# #) = undefined would be affected by this (would evaluate to |
I like both parts of this proposal. It's a dark corner, and we should strive for simplicity in the specification. This does exactly that. |
Clarified points as requested by @Ericson2314 , who is right on both counts. |
I actually like it that unboxed tuple bindings are strict (even though it was probably accidental) because it matches my intuition that every unlifted binding is strict. It feels like a special case to make unboxed tuple bindings lazy. Do we have a compelling use case for this, or is it just that (a) it's easy and (b) it's slightly more general? It might lead to less efficient code in some cases, and if someone is using unboxed tuples they probably care about performance. It would be nice if By all means make the error about nonbanged strict patterns into a warning. I'm not sure why it was made an error, but we did go through a series of changes where it was originally made a warning and then an error, and I recall a lot of churn particularly with making code work across multiple GHC versions. |
This is tricky stuff to specify, and the proposal still isn't very clear. The proposal say "Define an "unlifted pattern" to be any pattern that binds variable with an unlifted type.". That's good. Then it uses the term "strict pattern"; I think that's precisely the same as "unlifted pattern", but I'm not sure. I'll stick to "unlifted pattern" alone. Now, a binding
So it's pretty restrictive. Define a "banged binding" to be one of form
desugars to
This might diverge; but it also might not. The banged binding means that before evaluating OK, having set the scene, what about unboxed tuple bindings?
We could:
Richard's proposal is (1). It's unclear whether Simon M is suggesting (2) or (3). I think (2) would be better: it is more expressive, but in the common case of a non-recursive binding you get the case expression you probably expected. But I suppose we could chose (3). There are no technical issues at stake here; it's a free stylistic choice. |
Simon, in point (3) I think you meant "unlifted pattern" rather than "strict pattern", correct? So yes, I was thinking of (3), I hadn't considered (2). (3) seems simpler (to me), because it desugars directly to the case expression rather than requiring subsequent optimisations, but (2) is more expressive. I don't feel very strongly about any of this, but it does seem intuitive to me to treat an unboxed tuple binding like other unlifted pattern bindings. |
Yes! I've updated my comment. Sorry. |
Since "~" cancels "!", that desire would be uniformly supported with (2); but not with (3). So that argues for (2) |
In response to Simon M:
I don't feel very strongly about any of this, but it does seem intuitive to me to treat an unboxed tuple binding like other unlifted pattern bindings.
But that is exactly what this proposal is arguing for: treat unlifted tuples precisely like unlifted patterns. It seems you are using "unlifted pattern" to mean a pattern whose overall type is unlifted. Let's call that an "unlifted-type pattern". This is *different* from an unlifted pattern, as I've defined it. Here are examples:
```
x, y :: Bool
(# x, y #) = blah -- unlifted pattern: no; unlifted-type pattern: yes
data T = K Int#
K x = blah -- unlifted pattern: yes; unlifted-type pattern: no
3# = 4# -- unlifted pattern: no; unlifted-type pattern: yes
```
About that last (rather silly) example: should `let 3# = 4# in ()` throw an exception? GHC 8.0 does. Under my proposal, this happily evaluates to `()`.
Perhaps complicating this is that unboxed tuples (and sums) are currently the only way to have a unlifted-type pattern that binds variables, so it's hard to apply these rules in full generality. Hopefully the future will contain other unlifted types that store lifted ones.
So, to rephrase what others have said about option (2): (2) means to treat unlifted patterns and unlifted-type patterns uniformly. But do note that this
does add one more small wrinkle to the specification, because we have to specify what an unlifted-type pattern is. My original proposal is agnostic to the type of the pattern.
Will update the proposal w.r.t. "strict patterns" when I have a connection (on a train at the moment).
Agreed with Simon PJ that this is a knob we can turn freely -- no technical issues at stake or implementation challenge.
|
Sorry for being a bit loose with the terms. (perhaps "unlifted pattern" is an unfortunate terminology since it doesn't mean that the pattern is unlifted. How about "unlifted-variable pattern"?) |
This commit implements the proposal in ghc-proposals/ghc-proposals#29 and ghc-proposals/ghc-proposals#35. Here are some of the pieces of that proposal: * Some of RuntimeRep's constructors have been shortened. * TupleRep and SumRep are now parameterized over a list of RuntimeReps. * This means that two types with the same kind surely have the same representation. Previously, all unboxed tuples had the same kind, and thus the fact above was false. * RepType.typePrimRep and friends now return a *list* of PrimReps. These functions can now work successfully on unboxed tuples. This change is necessary because we allow abstraction over unboxed tuple types and so cannot always handle unboxed tuples specially as we did before. * We sometimes have to create an Id from a PrimRep. I thus split PtrRep * into LiftedRep and UnliftedRep, so that the created Ids have the right strictness. * The RepType.RepType type was removed, as it didn't seem to help with * much. * The RepType.repType function is also removed, in favor of typePrimRep. * I have waffled a good deal on whether or not to keep VoidRep in TyCon.PrimRep. In the end, I decided to keep it there. PrimRep is *not* represented in RuntimeRep, and typePrimRep will never return a list including VoidRep. But it's handy to have in, e.g., ByteCodeGen and friends. I can imagine another design choice where we have a PrimRepV type that is PrimRep with an extra constructor. That seemed to be a heavier design, though, and I'm not sure what the benefit would be. * The last, unused vestiges of # (unliftedTypeKind) have been removed. * There were several pretty-printing bugs that this change exposed; * these are fixed. * We previously checked for levity polymorphism in the types of binders. * But we also must exclude levity polymorphism in function arguments. This is hard to check for, requiring a good deal of care in the desugarer. See Note [Levity polymorphism checking] in DsMonad. * In order to efficiently check for levity polymorphism in functions, it * was necessary to add a new bit of IdInfo. See Note [Levity info] in IdInfo. * It is now safe for unlifted types to be unsaturated in Core. Core Lint * is updated accordingly. * We can only know strictness after zonking, so several checks around * strictness in the type-checker (checkStrictBinds, the check for unlifted variables under a ~ pattern) have been moved to the desugarer. * Along the way, I improved the treatment of unlifted vs. banged * bindings. See Note [Strict binds checks] in DsBinds and #13075. * Now that we print type-checked source, we must be careful to print * ConLikes correctly. This is facilitated by a new HsConLikeOut constructor to HsExpr. Particularly troublesome are unlifted pattern synonyms that get an extra void# argument. * Includes a submodule update for haddock, getting rid of #. * New testcases: typecheck/should_fail/StrictBinds typecheck/should_fail/T12973 typecheck/should_run/StrictPats typecheck/should_run/T12809 typecheck/should_fail/T13105 patsyn/should_fail/UnliftedPSBind typecheck/should_fail/LevPolyBounded typecheck/should_compile/T12987 typecheck/should_compile/T11736 * Fixed tickets: #12809 #12973 #11736 #13075 #12987 * This also adds a test case for #13105. This test case is * "compile_fail" and succeeds, because I want the testsuite to monitor the error message. When #13105 is fixed, the test case will compile cleanly.
Note that this proposal is now half-way through its four week discussion period. At the end of this period we ask @goldfirere to summarize the discussion and bring the proposal to the GHC committee for consideration. Thanks! |
This proposal has not attracted discussion for a while. Please consider submitting it to the Committee for discussion, if you think it is ready, or close it, if you have abandoned this proposal. |
Another ping. @goldfirere, whither this proposal? |
Oops. I missed the first ping. I will update this proposal with respect to recent commentary and then submit for a decision. |
@goldfirere Another ping about this proposal… :-) |
I have updated the proposal to incorporate feedback here. I would like to submit this for a decision, @nomeata. |
For the intermediate proposal, can we also have |
Philosophically, I see as the lazy matching of unlifted types as a sort of "static laziness". With this static laziness, the compiler is allowed to delay evaluation (or perhaps more broadly rewrite the term just worrying about preserving semantics in the case that the bound expression terminates) as long as no new thunking or boxing is introduced. @goldfirere's variable binding rule is somewhat a special case of this principle where, because of the lack of unlifted bindings with the pattern, thunks are "already available" for every possible data dependency with the potential to force the match. |
Oh, I missed that this was submitted for review… will act immediately. |
I tend to agree with @simonmar that we should do what is best for performance, which is where we need explicit use of unboxed tuples. But ... I guess there's some performance risk to going too strict by default as it might introduce extra "thunk-evaluated?" checks in scenarios where we know the thunks are already evaluated? I wish there were a simple rule like |
@goldfirere, I'd like to reply here to some of the text in the proposal:
Wait, does this language mean that we could need double bangs to avoid warnings? Again, I believe we want to generate the strict behavior by default most of the time. That is, that we don't want to make it easy for the user to accidentally generate this code shown in the manual: f x = let t = case h x of { (# p,q #) -> (p,q) }
p = fst t
q = snd t
in ..body.. Almost always, when I'm mucking with unboxed tuples because I'm explicitly trying to squeeze the allocation out of a loop. This terrible code above would reintroduce mystery thunk allocations (for I'm using unboxed tuples to get away from boxed tuples -- reintroducing boxed tuples implicitly is cruel! So that leads me to want to leave laziness as "opt in" rather than "opt out". But there are two distinct ways to accomplish that. One is the "option (2)" above, which is described in the Alternatives section of the proposal as:
Ok, that's fine. And @simonmar and @simonpj seem to be at-least-not-opposed to it. An add-on to that idea seems to be to issue warnings when any unboxed tuple binding lacks the bang, just like we do for other unlifted-type variable bindings in patterns. Uh, except this bumps up against the fact that we are not uniform in requiring "pattern variables of unlifted types should have a bang" (e.g. The good thing is that the meaning with the |
@goldfirere - Do all the discussed designs satisfy these two "theorems"?:
|
I will number your theorems (A) and (B). (A) is not true today, and this proposal does not change that fact. Counterexample: (B) is true today and with this proposal. I don't believe my proposal gets near to these theorems. The more interesting space (which my proposal does affect) is what happens when there is no bang or twiddle. |
I agree with @rrnewton, making an unboxed tuple binding lazy by default seems to be intuitively the wrong choice. I guarantee I would get tripped up by this! Giving unboxed tuples an implicit bang seems reasonable to me. |
I am also worried about existing code. People that do careful tricks with unboxed tuples probably require the particular semantics that GHC currently does (and they probably didn’t read the manual :-)). Since evaluation order is not something that is captured by our types, changing something here might cause hard-to-track-down bugs somewhere. I would expect (without looking) that Is there a good example why the documented behavior is actually better than the implementation? I.e. a use-case? Otherwise I’d be inclined to just update the manual to reflect reality that suited our users for several years, and consider it a bug in the manual, not the code :-) |
Catching up on this again, I mis-read the proposal. Could you add to the proposal something about desugaring? That is, you propose that
desguars to
I was worried about having a thunk with unboxed-tuple type; but in the desugaring we see that all thunks have lifted types. As to an implicit bang on outermost unboxed tuples, I defer to users. It's a free choice. I agree that making an unboxed tuple lazy (and making the desugaring create a boxed tuple) might be unexpected. |
Since this is implemented, should this PR have the "Implemented" label added? |
The proposal has been accepted; the following discussion is mostly of historic interest.
This proposes to make unboxed tuple patterns lazy-by-default, and to turn GHC's existing error about unbanged strict patterns (which are strict by virtue of binding a variable of an unlifted type) into a warning.
Rendered