ocaml-flambda / flambda-backend

The Flambda backend project for OCaml
93 stars 67 forks source link

Implicit braces in ocamltest trees #2693

Open lukemaurer opened 3 weeks ago

lukemaurer commented 3 weeks ago

The new tree syntax for ocamltest greatly improves the common case where several test cases are sequentially composed, each depending on the last. We can now write such tests as

test1;
test2;
test3;
test4;
...
test27;

without increasing the indentation level. This makes sense because these tests are not “nested” in any meaningful sense, any more than consecutive statements in imperative code are nested. Practically speaking, the great advantage is that we can now add test1a between test1 and test2 without increasing indentation (or other markers of nested-ness).

Unfortunately, this only works if test1a belongs in the sequence with the other tests. But it might be completely independent: for example, it might check an error case that is only meaningful after test1 but has no bearing on the other tests. In that case, we have three unpalatable options:

  1. Write it in the sequence anyway. This obscures the structure of the test cases and causes the entire rest of the sequence to be skipped if test1a fails.
  2. Indent every test in the sequence after test1a and risk emotional damage the next time you try to rebase.
  3. Add braces around every test in the sequence after test1a but don't indent, then try to invent a convention that will help you keep the formatting straight.

The annoying thing here is that test4 is still not meaningfully “more nested” than test1. It just happens that there's an extra branch jutting out of the sequence somewhere between test1 and test4, and the syntax makes that everyone's problem.

My proposed syntax is this:

test1;
{
 test1a;
}
test2;
test3;
test4;
...
test27;

This is exactly as if we'd put braces around everything from test2 to test27, but it once again expresses that this is simply a sequence of tests in a chain. It's just that now there are some tests that branch off from the sequence.

stedolan commented 3 weeks ago

I like this feature and the implementation is certainly straightforward!

But this is an area that I'd really prefer to avoid diverging from upstream OCaml, because resolving ocamltest conflicts is already annoying. Could you make this as an upstream PR and see if it gets accepted there, then backport?

lukemaurer commented 3 weeks ago

I like this feature and the implementation is certainly straightforward!

But this is an area that I'd really prefer to avoid diverging from upstream OCaml, because resolving ocamltest conflicts is already annoying. Could you make this as an upstream PR and see if it gets accepted there, then backport?

Sure, will do

lukemaurer commented 2 weeks ago

Submitted upstream as ocaml/ocaml#13259