Skip to content

Rust Engine: add interning table, intern GlobalIds#1689

Merged
W95Psp merged 27 commits intomainfrom
rengine-intern-ids-span
Sep 22, 2025
Merged

Rust Engine: add interning table, intern GlobalIds#1689
W95Psp merged 27 commits intomainfrom
rengine-intern-ids-span

Conversation

@W95Psp
Copy link
Copy Markdown
Contributor

@W95Psp W95Psp commented Sep 16, 2025

Summary

This PR introduces a simple interning mechanism and improves the handling of global identifiers.

What's Changed

  • Added interning module

    • Provides a lightweight interning mechanism.
    • Enables:
      • O(1) equality checks and hashing.
      • Copyable global identifiers.
      • Pattern matching on global identifiers.
  • Refactored names::* to constants

    • Previously: hax_rust_engine::names::core::ops::arith::Add::add was a function returning &'static GlobalId.
    • Now: it is a constant of type GlobalId.
    • This change is made possible by using hax_rust_engine::interning::InterningTable::new_with_values.

How to Review

  • Review commit by commit for clarity.
  • You can ignore code regeneration commits.

Issue

That's addressing half of #1666.
Future work: span interning.

@W95Psp W95Psp force-pushed the rengine-intern-ids-span branch from e225caf to 14c8f61 Compare September 16, 2025 08:55
@W95Psp W95Psp marked this pull request as ready for review September 16, 2025 08:59
@W95Psp W95Psp requested a review from a team as a code owner September 16, 2025 08:59
@W95Psp W95Psp requested review from a team, keks and maximebuyse and removed request for a team September 16, 2025 08:59
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 left a few comments, but it generally looks good to me! I didn't look deeply into the rust design (I leave that for the rust review)

Copy link
Copy Markdown
Member

@keks keks left a comment

Choose a reason for hiding this comment

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

This looks pretty good. Just a few questions and comments.

@W95Psp W95Psp force-pushed the rengine-intern-ids-span branch 2 times, most recently from 27b7b07 to 4a3ed0b Compare September 18, 2025 09:00
@W95Psp W95Psp requested a review from keks September 18, 2025 09:26
@W95Psp
Copy link
Copy Markdown
Contributor Author

W95Psp commented Sep 18, 2025

(I will rebase this branch and thus solve conflict after reviews)

Copy link
Copy Markdown
Member

@keks keks left a comment

Choose a reason for hiding this comment

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

Just a few nits, otherwise I think this is ready to be merged!

@W95Psp W95Psp force-pushed the rengine-intern-ids-span branch from 923280a to 8bd565d Compare September 22, 2025 12:14
@W95Psp W95Psp enabled auto-merge September 22, 2025 12:14
@W95Psp W95Psp disabled auto-merge September 22, 2025 12:15
@W95Psp W95Psp enabled auto-merge September 22, 2025 12:27
@W95Psp W95Psp disabled auto-merge September 22, 2025 12:27
Rust Engine: refactor names, change tuples representation
@W95Psp W95Psp enabled auto-merge September 22, 2025 12:28
@W95Psp W95Psp added this pull request to the merge queue Sep 22, 2025
Merged via the queue into main with commit 0e1c19a Sep 22, 2025
17 of 18 checks passed
@W95Psp W95Psp deleted the rengine-intern-ids-span branch September 22, 2025 13:39
@W95Psp W95Psp restored the rengine-intern-ids-span 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