Skip to content

Remove basic meet #3689

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

Closed

Conversation

bclement-ocp
Copy link
Contributor

@bclement-ocp bclement-ocp commented Mar 14, 2025

This patch removes the old/basic meet and makes the new/advanced meet the only and default meet algorithm.

The configuration options (-flambda2-basic-meet, -flambda2-advanced-meet, and the flambda2-meet-algorithm OCAMLPARAM) are kept to avoid breaking scripts, but are now silently ignored.

Note that the Meet_and_join_new module is NOT renamed to Meet_and_join, and the Meet_and_join module simply forwards to Meet_and_join_new. This is intentional in order to re-use the same dispatch mechanism for the new join algorithm in #3538. Changed this because it makes more sense to use different names in #3538. Meet_and_join_new is now renamed to Meet_and_join.

This PR also takes the opportunity to clean up the interfaces:

  • Remove the now unused meet_env_extension;
  • Remove join from the Flambda2_types interface

@bclement-ocp bclement-ocp requested review from mshinwell and lthls March 14, 2025 11:28
@bclement-ocp bclement-ocp added the flambda2 Prerequisite for, or part of, flambda2 label Mar 14, 2025
This patch removes the old/basic meet and makes the new/advanced meet
the only and default meet algorithm.

The configuration options (`-flambda2-basic-meet`,
`-flambda2-advanced-meet`, and the `flambda2-meet-algorithm` OCAMLPARAM)
are kept to avoid breaking scripts, but are now silently ignored.

Note that the `Meet_and_join_new` module is *NOT* renamed to
`Meet_and_join`, and the `Meet_and_join` module simply forwards to
`Meet_and_join_new`. This is intentional in order to re-use the same
dispatch mechanism for the new join algorithm in ocaml-flambda#3538.

This PR also takes the opportunity to clean up the interfaces:

 - Remove the now unused `meet_env_extension`;
 - Remove `join` from the `Flambda2_types` interface
@bclement-ocp bclement-ocp force-pushed the bclement/kill-old-meet branch from 2d23086 to 554d401 Compare March 18, 2025 13:25
Copy link
Contributor

@lthls lthls left a comment

Choose a reason for hiding this comment

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

Looks good to me.

@bclement-ocp
Copy link
Contributor Author

Closing in favor of #3726 and #3727

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
flambda2 Prerequisite for, or part of, flambda2
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants