Skip to content

Sort mutually-recursive constants by linkage name, not id #315

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

Open
wants to merge 1 commit into
base: flambda2.0-stable
Choose a base branch
from

Conversation

lukemaurer
Copy link

Since flexpect does a straightforward one-to-one comparison, the order
in which the bindings occur is important even within a recursive group,
and we can't have that order depend on something fragile like what
integer ids things happen to be assigned. Specifically, we get a
spurious test failure from a generated .flt test if the ids in the
original run happen to be different from the ids in the test run.
If we sort them by linkage name instead, the ordering should be
consistent.

Since flexpect does a straightforward one-to-one comparison, the order
in which the bindings occur is important even within a recursive group,
and we can't have that order depend on something fragile like what
integer ids things happen to be assigned. Specifically, we get a
spurious test failure from a generated .flt test if the ids in the
original run happen to be different from the ids in the test run.
If we sort them by linkage name instead, the ordering should be
consistent.
@mshinwell
Copy link

Two comments:

  1. Please add a signature on CIS_by_name.
  2. We need some way of using the default CIS when not running through flexpect for performance. Maybe instantiate the SCC functor twice then use a first-class module to choose?

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.

2 participants