Skip to content

Conversation

@Julow
Copy link
Contributor

@Julow Julow commented Apr 22, 2025

This helps find ignored Lwt threads, which are a challenge to translate into direct-style concurrency. Ignoring a Lwt thread makes it implicitly fork in the background, which requires an explicit call with other concurrency libraries.
This makes finding these implicit forks a lot easier at the expense of added type annotations.

Occurrences of ignored values were found using this tool. Running this again in the future is not needed and type annotation can be removed later if needed.

Thanks @raphael-proust for suggesting the idea.

This helps find ignored Lwt threads, which are a challenge to translate
into direct-style concurrency.
@vouillon
Copy link
Member

Can you change the Gihtub Action workflow to build with 4.13 instead of 4.08, which is getting quite old?
The latest version of re has a breaking change and requires OCaml 4.12 at least.

ocaml-compiler: "4.08"

The type annotations constrained 're' version which in turn constrained
ocaml's version.
@Julow Julow force-pushed the annotate-ignored-values branch from 1ddcf4c to 8774eba Compare April 23, 2025 13:45
@Julow
Copy link
Contributor Author

Julow commented Apr 23, 2025

The CI already runs on 4.14.2, 5.3.0 and 4.08. I changed the PR to not add a constraint on re.

@smorimoto smorimoto requested review from balat and vouillon April 23, 2025 14:49
@vouillon vouillon merged commit 33b05b6 into ocsigen:master Apr 23, 2025
10 checks passed
@Julow
Copy link
Contributor Author

Julow commented Apr 23, 2025

Thanks!

@raphael-proust
Copy link

and type annotation can be removed later if needed

IMHO having type annotations for ignored values is a strict positive in terms of readability. It also defends against some common issues (such as a type changing from some unit Lwt.t (intended for an implicit fork) to a unit Lwt.t lazy or a unit Lwt.t thunk or someother type which probably means the forked computation is actually not started)

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.

3 participants