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

Explore if-let syntax more? #5628

Closed
cristianoc opened this issue Sep 1, 2022 · 29 comments
Closed

Explore if-let syntax more? #5628

cristianoc opened this issue Sep 1, 2022 · 29 comments
Labels
stale Old issues that went stale
Milestone

Comments

@cristianoc
Copy link
Collaborator

Does it make sense to explore the if-let syntax more seriously?

Here's some imaginary example where several different language features could be handled at the same time:
https://github.com/cristianoc/rescript-compiler-experiments/pull/1/files#diff-81c7d8224cb32d4696c8931f3c4fa65fd0e1a0b036e6fe9b4fffd730d86d3412R176

Basic, non-nested version, discussed here: https://forum.rescript-lang.org/t/are-if-let-constructs-going-to-be-supported/3373

Precedent for a more advanced version: rust-lang/rust#94927 (comment)

@cristianoc cristianoc added this to the v10.2 milestone Sep 1, 2022
@cristianoc
Copy link
Collaborator Author

CC @IwanKaramazow

@IwanKaramazow
Copy link

The main use case would be the async chain?

@cristianoc
Copy link
Collaborator Author

cristianoc commented Sep 2, 2022

The main use case is to address user requests from the past.

There was a time when the community was quite vocal in asking for monadic let support. This covers roughly the same application area (without runtime artefacts for e.g. optionals etc that the compiler currently can't eliminate).

Judging from the forum https://forum.rescript-lang.org/t/is-it-a-good-idea-to-explore-if-let-more-seriously/3698, initial comments seem to indicate that the community does not have pain points in this area at the moment.

So there seems to be no basis for investigating syntax extensions for the time being.

@cristianoc cristianoc changed the title Explore if-let syntax more Explore if-let syntax more? Sep 2, 2022
@fhammerschmidt
Copy link
Member

fhammerschmidt commented Sep 2, 2022

The same goes for async/await. The existing members are already used to Promises or their favorite promise library.

However if if let helps making async/await more useful, it may help to draw in more users or even move people away from promises.

@cristianoc
Copy link
Collaborator Author

However if if let helps making async/await more useful, it may help to draw in more users or even move people away from promises.

One would need good confidence in that assumption before investing resources. How could one test the hypothesis that it would help new users?

@cristianoc
Copy link
Collaborator Author

For example, in the case of async/await, it has been reported all the time that not having it is an obstacle to onboarding new team members / getting adoption inside a company. So that case is pretty clear.

@cristianoc
Copy link
Collaborator Author

Perhaps it makes sense to consider taking a smaller, idiomatic step, and explore how optional chaining https://www.typescriptlang.org/docs/handbook/release-notes/typescript-3-7.html would fit in the language, how well it would go with records which have optional fields.

@cristianoc
Copy link
Collaborator Author

From what I can see e?.f is simply an abbreviation for

switch e {
| None => None
| Some(x) => Some(x.f)
}

and e?.f?.g by composition

switch {
  switch e {
  | None => None
  | Some(x) => Some(x.f)
  }
} {
| None => None
| Some(x) => Some(x.g)
}

@cristianoc
Copy link
Collaborator Author

Actually not sure what should happen when f has optional type, the old problem of nested optionals.

@cknitt
Copy link
Member

cknitt commented Sep 2, 2022

For me the issue with something like

 if let Ok(response1) = await FetchResult.fetch("https://www.google.com")
    and Some(url) = nextFetch(response1)
    and Ok(response2) = await FetchResult.fetch(url) {
   Js.log2("FetchResult response2", response2->Fetch.Response.status)
}

is that it seems to encourage happy-path programming. Each of these steps might fail and need different handling/logging, and currently ReScript forces me to think about that.

I would also prefer to look into optional chaining instead, and/or into guard syntax / early return (see rescript-lang/syntax#20).

@cristianoc
Copy link
Collaborator Author

Being a bit concrete with option chaining.
Setup:

type a = {a?: int}

type b = {b: a}

type c = {c?: b}

Let's assume x:c. Then:

x.c: option<b>
x.c?.b : option<a>
x.c?.b.?a : option<option<int>>

The last one is unlikely to be what one intended, as one cannot continue a chain from that. So one would need to invent some additional thing, say ??. so that

x.c?.b.??a : option<int>

This looks a lot like option accounting instead of expressing intentions efficiently.

@cknitt
Copy link
Member

cknitt commented Sep 2, 2022

Did you mean

x.c?.b?.a : option<option<int>>

?

Basically the issue is map vs. flatMap:

x.c->Option.map(b => b.b)->Option.map(a => a.a)

vs.

x.c->Option.map(b => b.b)->Option.flatMap(a => a.a)

TypeScript let's you do just

type a = { a?: number };

type b = { b: a };

type c = { c?: b };

const f = (x: c) => x.c?.b.a;

(but of course cannot infer the type of f correctly if the type of x is not known)

@cristianoc
Copy link
Collaborator Author

Indeed.
Optional chaining fits well with TS, and is more complicated with ReScript in a way that's not clear how to work around.

In terms of records with optional values, there could be opportunities coming from having nominal typing. Namely, one might be able to directly infer user intent without any additional notation. Could be interesting to think about a little bit, and try to understand if such magic would come with undesirable gotchas.

@cristianoc
Copy link
Collaborator Author

Now to explore adding a little bit of "magic" to how record field access is performed.

First some background. To infer the type of e.f currently there are essentially two cases:

1)Type inference has already determined that e has record type, then look up the record definition and produce the type of field f (perhaps with instantiation of type variables).
2) If not, look for record declarations in scope that define a field called f, and take the first one. Then, apply unification with that record definition and proceed as above.

Omitting those details, the typing rule is essentially the standard one:

e:{f:t, ...}
------------
    e.f:t

@cristianoc
Copy link
Collaborator Author

The "magic" approach would consist of adding typing rules without changing the language.
Where currently there is a type error, the new typing rule would interpret that as a specific intent instead:

e: option<{f:t, ...}>
---------------------
    e.f: option<t>

Then one for optional fields (the declarative nature of optional fields allows to special case them and avoid nested option types in the result):

e: option<{f:?t, ...}>
----------------------
    e.f: option<t>

@cristianoc
Copy link
Collaborator Author

cristianoc commented Sep 3, 2022

Back to the example, we would get:

x.c: option<b>
x.c.b: option<a>
x.c.b.a: option<int>

@cknitt
Copy link
Member

cknitt commented Sep 3, 2022

Sounds interesting! I am a bit afraid it might feel like too much magic though.
By just looking at the code, it will not be clear anymore that any optional values are involved.

Would it be also possible to, just like TypeScript, only enable the magic after a first ?.?

Like

x?.c: option<b>
x?.c.b: option<a>
x?.c.b.a: option<int>

?

@cknitt
Copy link
Member

cknitt commented Sep 3, 2022

BTW, in TypeScript, in addition to accessing nested object properties, ?. also provides the following functionality (examples from https://javascript.plainenglish.io/the-beauty-of-optional-chaining-in-typescript-32dd58ce1380):

Function Invocation — Executing an instance method if the object/function exists

// making sure employee isn't nullish before executing the function
employee?.sayHowCanIHelp();

// making sure the function exists before executing it
employee?.sayHowCanIHelp?.();

Indexing — Prevent invalid object/array indexing

// making sure employees and employees[0] isn't nullish
company.employees?.[0]?.name

@CarlOlson
Copy link

Just my two cents:

We've been adopting async/await for new code in place of our custom effect library. I've noticed async/await helps reduce nesting of asynchronous code and makes code look more familiar to Javascript. The drawback is proper error handling can increase nesting compared to our previous solution. We've decided that nesting switch statements is the easiest to read, makes data flow obvious, and allows catching exceptions.

I decided to review some of this real world code. I cannot find a spot where if-let would be an improvement. For the times we can ignore None and Error, it is more clear to use something like Option.forEach or Option.getExn.

I understand that optional chaining is popular, but again, I can't find real-world use-cases. In most situations it feels like feature envy and leads to competing code styles. And I believe optional chaining would encourage using records where variants should be used instead.

I decided to think what would make an improvement in code I've written recently and thought of Zig. Zig requires errors to be handled or returned, example from docs. It could look like the following in ReScript:

let connect: () => promise<result<t, [#HttpError(exn)]>>

let authenticate: t => promise<result<user, [#Unauthorized]>>

// let process: unit => promise<result<_, [#HttpError(exn) | #Unauthorized]>>
let process = async () => {
  let t = try await connect()
  defer t->disconnect
  let user = try await t->authenticate
  // do work
}

This would be less powerful than the Zig version. In Zig, each try adds call information similar to a stack trace. Maybe this could be based on exn or a new error type, but I'm unsure how that might look.

@jmagaram
Copy link
Contributor

jmagaram commented May 6, 2023

F# has a clever generalized solution to this called computation expressions. Works for options, results, async, sequences. It is extensible.

https://learn.microsoft.com/en-us/dotnet/fsharp/language-reference/computation-expressions

https://fsharpforfunandprofit.com/posts/computation-expressions-intro/

We don't have classes in ReScript. So I think the idea is that if you have a module type with functions called bind, return, yield, zero, etc. then you can use special syntax like...

option {
let! x = Some(5)
let! y = Some(8)
return x+y
}

And the compiler translates that into more complicated nested code.

@cristianoc
Copy link
Collaborator Author

One attractive property of if-let is that it's nothing more than syntactic sugar for pattern matching. While more expressive constructs tend to compile to code that is more complex for the compiler to optimise.
I guess that distinction is not a hard consequence, just an observation of what tends to happen.

@jmagaram
Copy link
Contributor

jmagaram commented May 6, 2023

I don't understand. The F# approach is just syntax sugar. Every let! or yield! or return! gets rewritten probably according to some straightforward rules that in the end generate highly indented code you would have written yourself. One interesting thing for F# is the built-in computation expressions don't even support the use case everyone is talking about - dealing with Option. They have them for lazy sequences, async, tasks (lazy promises), and database queries. I really like the F# approach because it is extensible. You could have a computation expression for Result that exits early when an Error happens. Did you read through the F# docs to see how they do it?

@cristianoc
Copy link
Collaborator Author

It's quite simple. If you want to add, say, an operation such as let-op for optionals, then you end up with a closure every time you open up an option. As the closure is passed as the continuation.
So you get with pretty inefficient code up until the compiler implements special optimisations to remove the closures just introduced.

Then rinse and repeat for every possible use of custom operators.

This does not mean that it is impossible, it only means that the natural implementation comes with a perf cost.

@jmagaram
Copy link
Contributor

jmagaram commented May 6, 2023

Ok I partially understand. I just read about how C# async methods get compiled to a state machine which definitely is NOT a naive approach of rewriting it as ugly nested code. Maybe the naive approach also creates the possibility of stack overflows if there are loops or recursion involved.

@jmagaram
Copy link
Contributor

jmagaram commented May 6, 2023

Did you look at those F# links? It seems like that approach is very flexible, not tied to options in particular, and I think you end up with the same code you would have written yourself. It takes a nested/indented mess and flattens it. Here is what the F# approach could look like...

module ResultBuilder = {
  let flatMap = (r, f) => // let! feeds right side of expression into this
    switch r {
    | Ok(x) => Ok(f(x))
    | Error(_) as err => err
    }

  let return = x => Ok(x)
}

let divide = (top, bottom) => bottom == 0 ? Error("Divide by zero!") : Ok(top / bottom)

let calculate = make(ResultBuilder) {
  let a! = divide(24, 2)
  let b! = divide(a, 0)
  let c! = divide(b, 3)
  return c
}

@cristianoc
Copy link
Collaborator Author

cristianoc commented May 7, 2023

Looks like this is for another, related issue, rather than this one which is about if-let syntax specifically.
The way to study this further is to write down explicitly what ReScript code this would desugar to, just for this specific example. And paste it in an issue together with the generated JS.

@jmagaram
Copy link
Contributor

jmagaram commented May 7, 2023

Ok I'll consider doing that. But did you read any of the F# docs? Does it seem interesting/promising to you? Doesn't this F# computation expression stuff solve the problem this issue is about, or is it totally unrelated?

@cristianoc
Copy link
Collaborator Author

I had a glance. Seems similar to ocaml's let+.
Kind of difficult to read more in detail without a specific goal in mind about the specific community pain points one aims to solve.
E.g. the treatment of optionals is well established. Others are not. Does not mean there are no other important paint points. It only means less is known about them afaik.
E.g. the other mayor one was promises, and async/await was intentionally introduced to address them specifically, instead of going for a generic mechanism. In the cases of promises, this was a very deliberate choice. Other cases can be different, depending on the situation.

Copy link

This issue has been automatically marked as stale because it has not had recent activity. It will be closed if no further activity occurs. Thank you for your contributions.

@github-actions github-actions bot added the stale Old issues that went stale label Sep 29, 2024
@github-actions github-actions bot closed this as not planned Won't fix, can't repro, duplicate, stale Oct 8, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
stale Old issues that went stale
Projects
None yet
Development

No branches or pull requests

6 participants