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

Linear types amendment: Linear lets #624

Merged
merged 7 commits into from Feb 21, 2024

Conversation

aspiwack
Copy link
Contributor

@aspiwack aspiwack commented Nov 24, 2023

Would be best consumed using the rich diff view, except it doesn't appear to work. Fortunately, this is almost entirely added section, so I can point to them individually.

This is a rather straightforward amendment which specifies multiplicity-annotated let bindings which, for reasons unknown to me, were only implied before. Pretty much all the choices are forced, the design space is really tiny. In consequence I intend to leave this open for comments a few work days before I submit it to the committee.

@simonpj , you may want to take part of the pre-committee reviews.

@VitWW
Copy link

VitWW commented Nov 24, 2023

@aspiwack Maybe you forget to add Rendered link to your first comment

@aspiwack
Copy link
Contributor Author

@VitWW

Maybe you forget to add Rendered link to your first comment

I didn't: there isn't a rendered link because it's an amendment. Instead, go to File changed and choose to display the rich diff. I'm afraid there isn't a better way.

PS: I had inadvertently included another proposal in this PR, I've fixed it as soon as it was pointed out to me.

@aspiwack
Copy link
Contributor Author

Except… the rich diff isn't working for me at the moment. Is it just me? Is it just my document?

@googleson78
Copy link
Contributor

Except… the rich diff isn't working for me at the moment. Is it just me? Is it just my document?

Doesn't work for me either at the moment

@JakobBruenker
Copy link
Contributor

Can you have multiple bindings in one let? As in

let %1 x = u
    %Many y = v
in 

@aspiwack
Copy link
Contributor Author

Can you have multiple bindings in one let?

I don't see any reason not to. What makes you doubt it, so that I clarify in the proposal?

@JakobBruenker
Copy link
Contributor

JakobBruenker commented Nov 27, 2023

What makes you doubt it

Only the lack of such an example in the proposal

@aspiwack
Copy link
Contributor Author

aspiwack commented Dec 1, 2023

I don't know what in my PR breaks the rich diff view, but I'm convinced at this point that it isn't transient. I've added link, in the PR description, to the various sections that I've added, hopefully I didn't forget any.

@aspiwack
Copy link
Contributor Author

aspiwack commented Dec 7, 2023

The conversation died out. @nomeata I'd like to submit this proposal amendment to the committee.

@nomeata nomeata added the Pending shepherd recommendation The shepherd needs to evaluate the proposal and make a recommendataion label Dec 7, 2023

Non-variable linear patterns can't be lazy (see `Lazy patterns`_). As
a consequence, non-variable let-bound patterns must be annotated with
a ``!`` (because let-bound patterns are lazy by default, as opposed to
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

What happens if -XStrict is in effect?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

We still require a !. (we could do better, but it's unclear that it's worth it, especially since -XStrict is so rarely used)

Copy link
Contributor

@goldfirere goldfirere left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I've made a number of comments throughout. Many are simple typos or small corrections. Regardless, I'm in support of this -- especially because we are having debates about stability elsewhere, and thus seem to be carving out a space for more experimentation here. That is, maybe the interaction between generalization and the desire to infer linear bindings is a bit wrong in this proposal, something that will be hard to know until it's implemented. But that's fine: users of -XLinearTypes know that it's subject to change.

Small open question: how many users of -XLinearTypes do we have? If it's "lots", then this might want a new -XLinearLets extension that's less stable. But if it's "not that many, and we're friends with most of them", let's just keep it simple in one extension.

proposals/0111-linear-types.rst Outdated Show resolved Hide resolved
proposals/0111-linear-types.rst Show resolved Hide resolved
-- Doesn't work
let %p f x y = rhs in body

instead write
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

What's the reason for this restriction? It seems surprising.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Honestly, I haven't really given it any deep thought. let x :: sig is a PatBind in the implementation, I made let %p x a PatBind too. It's a more minimal change in the code base. Like previous comments: this choice is conservative and doesn't prevent from extending the syntax when we have more experience about it.

proposals/0111-linear-types.rst Outdated Show resolved Hide resolved
proposals/0111-linear-types.rst Outdated Show resolved Hide resolved
But this means pattern-matching on a lambda abstraction (albeit a lambda over
type variables), which is not something that Core understands. Also, ``f`` and ``g``
should be polymorphic, despite the fact that both fields of ``p`` are
monomorphic. It's not clear that this makes sense. Even assuming that it makes
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This bit is true regardless of linear lets and such. The fields of p are monomorphic, but the type they work on is a variable, bound in an outer big lambda. This point is not important for the proposal, though.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Right. The point was that the way we work around it for unrestricted bindings doesn't work for linear types. And that the way we compile linear pattern bindings doesn't apply to this situation.

prevents the type-checker from generating ``AbsBinds``, and, as such,
makes more inferred lets linear, which is almost certainly the right default
(at least it's the least surprising: a binding doesn't change from
linear to unrestricted because a small change makes it generalisable).
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I fear there is trouble here, because MonoLocalBinds is not as powerful as you think.

This also clarifies that annotated variables (which do not get the AbsBinds treatment) and polymorphic field projections are just fine. That's good. But I do think that there will remain a tension between inferring a linear binding and generalization.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I can't deny that there is this nagging part of my brain that suspects that maybe something is still amiss. But I haven't been able to find an issue yet.

patterns are monomorphic`_) that

1. ``-XLinearTypes`` implies ``-XMonoLocalBinds``.
2. Polymorphic non-multiplicity-annotated let-bound non-variable patterns are inferred to be unrestricted
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think this isn't quite what you mean. I think you just want to say that generalized bindings are unrestricted. If a binding binds a polymorphic variable without generalization, then it's fine.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I changed “Polymorphic” to “Generalised” hopefully this is clear.

proposals/0111-linear-types.rst Outdated Show resolved Hide resolved
are annotated with ``%Many``. But it invites a lot of complication
(what if it's a type synonym that is equivalent to ``Many``? What if
it's a type family?) that are hard to justify. In particular since we
take ``-XMonoLocalBinds`` for granted, where generalisation doesn't
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Again, this last point isn't accurate, but it doesn't matter. I'm fine with let %Many disabling generalization. If you want it, just use a type signature!

aspiwack and others added 2 commits January 10, 2024 17:54
Co-authored-by: Richard Eisenberg <rae@richarde.dev>
@aspiwack
Copy link
Contributor Author

@goldfirere in addition to the threads above, the part about generalisation vs linearity was born of the implementation. I hadn't realised the tension prior to implementing all this.

@nomeata nomeata added Pending committee review The committee needs to evaluate the proposal and make a decision and removed Pending shepherd recommendation The shepherd needs to evaluate the proposal and make a recommendataion labels Jan 22, 2024
@goldfirere
Copy link
Contributor

OK. I declare this accepted.

@goldfirere goldfirere merged commit 909f558 into ghc-proposals:master Feb 21, 2024
1 check failed
@adamgundry adamgundry added Accepted The committee has decided to accept the proposal and removed Pending committee review The committee needs to evaluate the proposal and make a decision labels Feb 23, 2024
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
Development

Successfully merging this pull request may close these issues.

None yet

9 participants