Skip to content

Fix a bug in slot_offsets when two separate sets of closures have a slot in common, but not all#2649

Merged
Ekdohibs merged 1 commit intooxcaml:mainfrom
Ekdohibs:fix-slot-offsets
Jun 19, 2024
Merged

Fix a bug in slot_offsets when two separate sets of closures have a slot in common, but not all#2649
Ekdohibs merged 1 commit intooxcaml:mainfrom
Ekdohibs:fix-slot-offsets

Conversation

@Ekdohibs
Copy link
Copy Markdown
Contributor

@Ekdohibs Ekdohibs commented Jun 4, 2024

Extracted from #2642.

@mshinwell mshinwell added the flambda2 Prerequisite for, or part of, flambda2 label Jun 4, 2024
@mshinwell
Copy link
Copy Markdown
Collaborator

Please add a test for this...

@Ekdohibs
Copy link
Copy Markdown
Contributor Author

Ekdohibs commented Jun 4, 2024

I'm not sure how to add a test, given it's something that isn't generated by the compiler as of now, and this can't be tested by flexpect either.

@Ekdohibs
Copy link
Copy Markdown
Contributor Author

Ekdohibs commented Jun 6, 2024

So, this fixes a bug, but there are other remaining. Indeed, we cannot put space between function slots in set of closures, as Gc.compact will scan that area (especially the arity). This will require more changes, the scope of which I am not sure about yet.

@Gbury
Copy link
Copy Markdown
Contributor

Gbury commented Jun 10, 2024

I'll look at the problem with consecutive slots as soon as I have the time.

@Ekdohibs
Copy link
Copy Markdown
Contributor Author

The consecutive slots problem has been addressed in #2674.

@Ekdohibs Ekdohibs merged commit 7d944ef into oxcaml:main Jun 19, 2024
TheNumbat pushed a commit that referenced this pull request Jun 21, 2024
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.

3 participants