Skip to content

Rust Engine: refactor names, change tuples representation#1693

Merged
W95Psp merged 11 commits intorengine-intern-ids-spanfrom
rengine-tuples
Sep 22, 2025
Merged

Rust Engine: refactor names, change tuples representation#1693
W95Psp merged 11 commits intorengine-intern-ids-spanfrom
rengine-tuples

Conversation

@W95Psp
Copy link
Copy Markdown
Contributor

@W95Psp W95Psp commented Sep 17, 2025

This PR introduces two major changes:

  • Enforces a strong abstraction over global identifiers:
    • The internal structure of identifiers is now fully hidden, improving encapsulation and simplifying usage.
  • Changes the representation of tuples:
    • Tuples were previously treated as regular names. Tuples are the only "parametric" identifiers: there exists an infinity of tuple identifiers. Thus, destructuring identifiers as tuples identifier was not possible without hacks.
    • They are now explicitly represented using GlobalIdInner::Tuple, which is very simple to pattern match on.

How to review

Skip the code regeneration commits

@W95Psp W95Psp requested a review from a team as a code owner September 17, 2025 15:41
@W95Psp W95Psp requested review from a team, jschneider-bensch and maximebuyse and removed request for a team September 17, 2025 15:41
Copy link
Copy Markdown
Contributor

@jschneider-bensch jschneider-bensch left a comment

Choose a reason for hiding this comment

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

This might be really basic, but I need to understand more about the different Id types. I believe none of them are new to the Rust engine, but have been ported from the OCaml engine, right? Is there some documentation in the OCaml engine (or elsewhere) for GlobalId, ConcreteId, DefId, ExplicitDefId and LocalId?

@W95Psp W95Psp force-pushed the rengine-intern-ids-span branch from 27b7b07 to 4a3ed0b Compare September 18, 2025 09:00
@W95Psp
Copy link
Copy Markdown
Contributor Author

W95Psp commented Sep 18, 2025

Thanks for your review!
No, that makes sense, the hierachy of identifier types is a big overwhelming 😅

Some are similar to the OCaml engine, but it was mostly redesigned.

I added contents in the module doc of identifiers and global_id, does that help?

Copy link
Copy Markdown
Contributor

@maximebuyse maximebuyse left a comment

Choose a reason for hiding this comment

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

I added a small comment, otherwise looks good!

@jschneider-bensch
Copy link
Copy Markdown
Contributor

I added contents in the module doc of identifiers and global_id, does that help?

Thanks, that really helps me understand what's happening!

Copy link
Copy Markdown
Contributor

@jschneider-bensch jschneider-bensch left a comment

Choose a reason for hiding this comment

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

I think it looks good, thanks for adding more documentation!

@W95Psp
Copy link
Copy Markdown
Contributor Author

W95Psp commented Sep 18, 2025

This PR is now reviewed, I'm now waiting for #1689.
When it is reviewed as well, I will rebase that PR.

@W95Psp W95Psp force-pushed the rengine-intern-ids-span branch from 923280a to 8bd565d Compare September 22, 2025 12:14
@W95Psp
Copy link
Copy Markdown
Contributor Author

W95Psp commented Sep 22, 2025

I'm merge that branch in its parent branch

@W95Psp W95Psp merged commit 4f71ab2 into rengine-intern-ids-span Sep 22, 2025
17 of 18 checks passed
@W95Psp W95Psp deleted the rengine-tuples branch September 22, 2025 12:28
@W95Psp W95Psp restored the rengine-tuples branch January 15, 2026 15:21
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.

3 participants