-
Notifications
You must be signed in to change notification settings - Fork 99
Dev meeting 22 06 2023
- How's the Outreachy internship going?
- Recap of ocaml.5.1.0 support
- What have been the main pain points for Ppxlib wrt the Parsetree change on value binding constraints?
- How can we handle those kinds of Parsetree changes better in the future?
- Upcoming ocaml.5.1.0 feature support
- What structure do we want for the Ast_builder/Ast_pattern unstable submodules?
- When are we going to start the creation of the current PPX universe?
- How we will detect the potential semantic breakages of PPXs after the Parsetree bump.
- trunk-support
- Let’s move
trunk-support
over to main! - Possibly: Astlib
- Let’s move
- Sonja
- Paul-Elliot
- Burnley
Burnley has already finished improving the error reporting in the first project 🎉 https://github.com/janestreet/ppx_fields_conv/pull/13 That's amazing!
Next step: Improve ppxlib
's own behavior. When a context-free rewriter raises, continue the process with the error embedded.
Action point Paul-Elliot: open an issue explaining the four points why we need to change the behaviour:
- In both cases, there are unrelated errors.
- In both cases, the first error will be the one of the first rewriter error.
- The rest of the Merlin behaviour will be better if we change (example: when derivers are skipped, values won't be found).
- There is no example of the theoretical case where we want panic from a rewriter to make
ppxlib
panic itself.
ppxlib.0.30.0
with OCaml 5.1.0 support is released! 🎉
There has been one big pain point writing the migrations this time: The Parsetree change of how value binding constraints are captured. Things that went wrong:
- We didn't analyze the challenges from the beginning on:
- Migrations (i.e. work ppxlib):
- Far more work to get the migrations right than expected.
- Also, getting the locations right was quite involved.
- Impact on PPXs (i.e. work for patch PRs, impact on the ecosystem):
- There can also be semantic breakages on PPXs, where usually breakages are purely syntactically and can be detected by building the PPX universe.
- Migrations (i.e. work ppxlib):
- We didn't write tests from the beginning, so there was no way to reason on a syntax level, all reasoning had to be done on the Parsetree level.
- We started communicating with Octachron (the person who introduced the Parsetree change) too late.
- There was a bug in the compiler Parsetree change forcing us to dive into the change weeks later again.
- Partly as a consequence of that, four different people worked on the migrations at different moments. Each of us had to dive into the Parsetree change every time.
Takeaways for next time there's a Parsetree change that's not simply a new syntax feature:
- Analyze the consequences in detail from the beginning on. That's important both for us and to communicate them to the compiler folks from the beginning on to weigh out whether the change is worth it.
- Have tests for the migrations from the beginning (good thing: now that we already have a migration tests workflow, next time, we can copy it).
- Communicate with the person from the compiler introducing the change from the beginning on.
- Avoid splitting the work among different people. It should only be one from us diving into the Parstree change, together with the person who's introduced the Parstree change. Also avoid splitting the fixes over time.
Question: Are there new syntax features in 5.1 that are reflected in the Parsetree? Are there other features in 5.1 that ppxlib
would break on before bumping the AST to 5.1?
If not, we are not in a hurry to bump the AST.
AST changes we could find:
- New way to represent generative functors -> migrations don't seem to impact the user wrt this change.
- let-bindings type constraint -> migrations don't impact the user wrt this change
So it seems like we're not in a hurry to bump the Ast! 🎉
Action points:
- Let's make sure it's true that downward migration doesn't affect a user of 5.1 wrt the introduction of generative functors.
- Add tests for generative functors.
Once we have the PPX universe and have bumped the AST, how are we going to detect semantic breakages of PPXs? Luckily, that should be easy this time: We can just grep for Ptyp_poly
🎉😏
We're going to discuss this next time, given that it looks like we won't bump the AST before the next meeting.
Pros for moving trunk-support
to main
:
- We won't need to sync the two branches anymore.
- The cron-job for the CI checking that
ppxlib
builds ontrunk
can only be run onmain
.
Cons for moving trunk-support
to main
:
- Breaking changes on
main
will affect CIs testingtrunk
. Not a huge deal: When we introduce breaking changes, we immediately open patch PRs to the affected users. So the CI can pin to those branches. - We'll need a release script that makes sure we add the upper OCaml bound when releasing.
If our proposal is accepted, we need to prepare the talk it in July, since we're off in August and the presentation is beginning September.