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

Parse where clause into the corresponding let (fix #209) #251

Closed
wants to merge 1 commit into from

Conversation

ulysses4ever
Copy link
Collaborator

@ulysses4ever ulysses4ever commented Feb 1, 2024

I transform where in let in the HaskellFrontend.hs. This is cheap but can lead to confusion in error messages. Not too bad, perhaps, because the bindings are still the same as in the source. The expensive way would be to extend L0 with where, which doesn't sound fun. So, do we want to pay for extending the source language with possible confusion in error messages about this extension? I think it's a fair deal. It also leaves the door open for future re-implementation in the proper way via extending L0...

fix #209

@ulysses4ever
Copy link
Collaborator Author

I could also look into implementing it in L0 if this is what people prefer.

@vidsinghal
Copy link
Collaborator

I think doing this transformation in the Haskell frontend, though an easier task, is not the best idea.
Since its entirely possible gibbon may shift to Ghc core in the future.
Adding a case of where in the ADT that makes up the L0-L4 IR seems like the way to go IMO.

@ulysses4ever
Copy link
Collaborator Author

Cool, thank you @vidsinghal! I can look into that and estimate if I have the resource to pull it out.

@vidsinghal
Copy link
Collaborator

Cool, thank you @vidsinghal! I can look into that and estimate if I have the resource to pull it out.

I think, optionally, if you don't have enough time. Hiring undergraduate students is an option. This could be a part time micro-project.

@ulysses4ever
Copy link
Collaborator Author

Indeed, this sounds like a good idea, but the issue is: I personally don't have any connection to undergrads! No idea how to poll them…

@vidsinghal
Copy link
Collaborator

Indeed, this sounds like a good idea, but the issue is: I personally don't have any connection to undergrads! No idea how to poll them…

I think Ryan or some other Prof. in the group can recruit on your behalf.

@ckoparkar
Copy link
Member

I actually support desugaring wheres into lets in the frontend.

I think doing this transformation in the Haskell frontend, though an easier task, is not the best idea.
Since its entirely possible gibbon may shift to Ghc core in the future.
Adding a case of where in the ADT that makes up the L0-L4 IR seems like the way to go IMO.

Here's my counterargument. If where is added to the AST from L0-L4, we'll keep around a pure source-level construct all the way up to codegen! What will we gain by doing this isn't clear to me. The downside is clear that every pass will now an additional binder to deal with. In GHC, by the time we reach Core the wheres are already desugared into lets. So even if Gibbon does start using GHC Core at some point, we won't encounter any wheres in a Core program.

@ulysses4ever
Copy link
Collaborator Author

Chai's argument sounds convincing to me. I can play the advocate of the devil and say that the analogy between L0-4 and the GHC Core is good but has a notable exception: GHC does most (all?) error-reporting at the source syntax level, while in Gibbon it's done in L0-4 mostly (except for outright parse errors). So, there's a higher chance the desugaring will leak into error messages. But it's not too much of a big deal for a research language, I feel. And the rest of the argument holds perfectly, I think: L0-4 feels like it should be kept as a pretty succinct IR like the GHC Core.

@ulysses4ever
Copy link
Collaborator Author

Maybe would be great to have @rrnewton to chime in!

@vidsinghal
Copy link
Collaborator

Chai's argument sounds convincing to me. I can play the advocate of the devil and say that the analogy between L0-4 and the GHC Core is good but has a notable exception: GHC does most (all?) error-reporting at the source syntax level, while in Gibbon it's done in L0-4 mostly (except for outright parse errors). So, there's a higher chance the desugaring will leak into error messages. But it's not too much of a big deal for a research language, I feel. And the rest of the argument holds perfectly, I think: L0-4 feels like it should be kept as a pretty succinct IR like the GHC Core.

Oh I did not know that GHC core parses where to lets. In that case the current approach seems fair.

@ulysses4ever
Copy link
Collaborator Author

It's not exactly that it "parses" where's to let's. It desugars where's to lest's. But before desugaring, it does a lot of work, unlike Gibbon. E.g. typechekcing in GHC works over the source syntax with where's, so that if there's a type error, the error message will preserve the where's...

@ulysses4ever
Copy link
Collaborator Author

Ryan's comment on today's meeting:

RN: acknowledge that there’s a tension -- we aren’t full “nanopass” where it is easy to change the IR between every pass. This would need an tweaked IR inserted before L0 -- with where in the AST.. and then it would desugar to L0. We probably don’t want to pollute L0 with where.

  • we can use our lame extensibility mechanism to do this somewhat cheaply
  • BUT I don’t think the current solution is bad… can leave it but make a task to factor it out. Maybe a tracking task for all-the-things-we-want-to remove from the parser.

I understand the concern that the parser becomes more complicated. But I don't see how to avoid it. Introducing a full another AST before we get to L0 (as seems to be implied) just to desugar where seems like shooting at sparrows with a canon. Also, I don't know about other things that we'd want to remove from the parser.

Not sure how to proceed. If someone feels strong enough that this patch is Good Enough™, they need to formally approve it. Otherwise, we can just leave it be here, and wait for another time...

@ulysses4ever
Copy link
Collaborator Author

@vollmerm it occurred to me you didn't express your opinion on the matter (or I missed it). tldr; of the above: how do you feel about desugaring where in the HaskellFrontend rather than adding it as a form somewhere in L0-L4? Ryan stipulated there may be a language in front of L0 with where, and that gets desugared into L0. Not sure what we win by this.

Just a reminder, this PR needs one formal approval to get merged.

@ulysses4ever
Copy link
Collaborator Author

ulysses4ever commented Jul 1, 2024

All right, not much support here, so closing for now.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Support where expressions in Gibbon
3 participants