Skip to content

Document container setup for OCaml and dune #7899

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.

Sign up for GitHub

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

Open
wants to merge 1 commit into
base: return-marker-flags
Choose a base branch
from
Open

Document container setup for OCaml and dune #7899

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
26 changes: 26 additions & 0 deletions AGENTS.md
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -12,6 +12,32 @@ make && make test
make format && make checkformat
```

## Container Tooling Setup
Copy link
Member

Choose a reason for hiding this comment

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

We also have our dev container setup (for working in a container in VS Code) at https://github.com/rescript-lang/rescript/tree/master/.devcontainer.

Do we actually need two different Docker setups?
If so, then it would be great to have a Dockerfile for this one, too, so that the agent just needs to run docker build in the correct directory.


The base container image does not ship with the OCaml toolchain or `dune`.
Install them once per container session before running any compiler builds or
tests:

```bash
# Install the OCaml 4.14 toolchain and findlib support
apt-get update
apt-get install -y ocaml-nox ocaml-native-compilers ocaml-findlib m4 pkg-config

# Build and install dune ≥3.20 from source
curl -LO https://github.com/ocaml/dune/releases/download/3.20.2/dune-3.20.2.tbz
tar -xf dune-3.20.2.tbz
cd dune-3.20.2 && make release
install -m 0755 _build/default/src/dune.exe /usr/local/bin/dune
cd .. && rm -rf dune-3.20.2 dune-3.20.2.tbz

# Verify dune is available
dune --version
```

These steps match the manual bootstrapping previously performed in the
container, ensuring subsequent `make`/`dune` commands succeed without relying
on external package mirrors such as `opam.ocaml.org`.

## ⚠️ Critical Guidelines & Common Pitfalls

- **We are NOT bound by OCaml compatibility** - The ReScript compiler originated as a fork of the OCaml compiler, but we maintain our own AST and can make breaking changes. Focus on what's best for ReScript's JavaScript compilation target.
Expand Down
2 changes: 1 addition & 1 deletion tests/ounit_tests/dune
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -11,4 +11,4 @@
(<> %{profile} browser))
(flags
(:standard -w +a-4-9-30-40-41-42-48-70))
(libraries bsb bsb_helper core ounit2 analysis))
(libraries bsb bsb_helper core frontend ounit2 analysis))
115 changes: 54 additions & 61 deletions tests/ounit_tests/ounit_ast_return_tests.ml
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -9,8 +9,7 @@ let parse_structure source =
in
result.parsetree

let test_simple_return _ctx =
let structure = parse_structure "let f = x => x + 1\n" in
let extract_fun_body structure =
match structure with
| [
{
Expand All @@ -21,79 +20,73 @@ let test_simple_return _ctx =
);
_;
};
] -> (
OUnit.assert_bool "function body should be marked as a return"
(Res_return_marker.expression_is_return body);
match body.pexp_desc with
| Pexp_apply {funct; args; _} ->
OUnit.assert_bool "callee should not be marked"
(not (Res_return_marker.expression_is_return funct));
List.iter
(fun (_, arg) ->
OUnit.assert_bool "arguments should not be marked"
(not (Res_return_marker.expression_is_return arg)))
args
| _ -> OUnit.assert_failure "expected application in function body")
] -> body
| _ -> OUnit.assert_failure "unexpected structure for simple function"

let test_simple_return _ctx =
let structure = parse_structure "let f = x => x + 1\n" in
let body = extract_fun_body structure in
OUnit.assert_bool "function body should be marked as a return"
(Res_return_marker.expression_is_return body);
match body.pexp_desc with
| Pexp_apply {funct; args; _} ->
OUnit.assert_bool "callee should not be marked"
(not (Res_return_marker.expression_is_return funct));
List.iter
(fun (_, arg) ->
OUnit.assert_bool "arguments should not be marked"
(not (Res_return_marker.expression_is_return arg)))
args
| _ -> OUnit.assert_failure "expected application in function body"

let test_sequence_return _ctx =
let structure = parse_structure "let f = x => {let y = x; y}\n" in
match structure with
| [
{
Parsetree.pstr_desc =
Pstr_value
( _,
[{pvb_expr = {Parsetree.pexp_desc = Pexp_fun {rhs = body; _}; _}; _}]
);
_;
};
] -> (
OUnit.assert_bool "function body should be marked"
(Res_return_marker.expression_is_return body);
match body.pexp_desc with
| Pexp_sequence (first, second) ->
OUnit.assert_bool "last expression in sequence should be marked"
(Res_return_marker.expression_is_return second);
OUnit.assert_bool "first expression in sequence should not be marked"
(not (Res_return_marker.expression_is_return first))
| Pexp_let (_, _, inner) ->
OUnit.assert_bool "inner expression should be marked"
(Res_return_marker.expression_is_return inner)
| _ -> OUnit.assert_failure "expected sequence or let in body")
| _ -> OUnit.assert_failure "unexpected structure for block body"
let body = extract_fun_body structure in
OUnit.assert_bool "function body should be marked"
(Res_return_marker.expression_is_return body);
match body.pexp_desc with
| Pexp_sequence (first, second) ->
OUnit.assert_bool "last expression in sequence should be marked"
(Res_return_marker.expression_is_return second);
OUnit.assert_bool "first expression in sequence should not be marked"
(not (Res_return_marker.expression_is_return first))
| Pexp_let (_, _, inner) ->
OUnit.assert_bool "inner expression should be marked"
(Res_return_marker.expression_is_return inner)
| _ -> OUnit.assert_failure "expected sequence or let in body"

let test_switch_returns _ctx =
let structure =
parse_structure "let f = x => switch x { | 0 => 1 | _ => x }\n"
in
match structure with
| [
{
Parsetree.pstr_desc =
Pstr_value
( _,
[{pvb_expr = {Parsetree.pexp_desc = Pexp_fun {rhs = body; _}; _}; _}]
);
_;
};
] -> (
OUnit.assert_bool "switch expression should be marked"
(Res_return_marker.expression_is_return body);
match body.pexp_desc with
| Pexp_match (_, cases) ->
List.iter
(fun {pc_rhs; _} ->
OUnit.assert_bool "case rhs should be marked"
(Res_return_marker.expression_is_return pc_rhs))
cases
| _ -> OUnit.assert_failure "expected switch in function body")
| _ -> OUnit.assert_failure "unexpected structure for switch body"
let body = extract_fun_body structure in
OUnit.assert_bool "switch expression should be marked"
(Res_return_marker.expression_is_return body);
match body.pexp_desc with
| Pexp_match (_, cases) ->
List.iter
(fun {pc_rhs; _} ->
OUnit.assert_bool "case rhs should be marked"
(Res_return_marker.expression_is_return pc_rhs))
cases
| _ -> OUnit.assert_failure "expected switch in function body"

let test_rewrite_loses_return_flag _ctx =
let source = "let f = x => g(x)\n" in
let structure = parse_structure source in
let body = extract_fun_body structure in
OUnit.assert_bool "pre-rewrite body should be marked"
(Res_return_marker.expression_is_return body);
let rewritten = Ppx_entry.rewrite_implementation structure in
let rewritten_body = extract_fun_body rewritten in
OUnit.assert_bool "rewritten body currently loses its return flag"
(not (Res_return_marker.expression_is_return rewritten_body))
Comment on lines +74 to +83
Copy link

Choose a reason for hiding this comment

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

[P1] Invert return-flag assertion after rewriting

The new test_rewrite_loses_return_flag intentionally asserts that Ppx_entry.rewrite_implementation drops the return marker (assert_bool … (not (Res_return_marker.expression_is_return rewritten_body))). Tests should encode the desired behaviour, i.e. the flag should be preserved after rewriting. As written, the suite now passes only when the bug exists and will fail once the bug is fixed, which both masks the regression and locks the incorrect behaviour in place.

Useful? React with 👍 / 👎.


let suites =
"return marker"
>::: [
"function body" >:: test_simple_return;
"sequence tail" >:: test_sequence_return;
"switch cases" >:: test_switch_returns;
"rewrite loses return flag" >:: test_rewrite_loses_return_flag;
]