Skip to content

Rust Engine: global identifiers: add view and rendering#1624

Merged
W95Psp merged 17 commits intomainfrom
rengine-name-views
Aug 28, 2025
Merged

Rust Engine: global identifiers: add view and rendering#1624
W95Psp merged 17 commits intomainfrom
rengine-name-views

Conversation

@W95Psp
Copy link
Copy Markdown
Contributor

@W95Psp W95Psp commented Aug 25, 2025

Identifiers in hax are wrapper around DefIds from rustc.
DefId consists in flat paths from crate name to the item name.

Printing such names in a consistent way across backend is a bit difficult. Names are not actually flat: a field for instance, always has a parent constructor, which always has a parent type.

When printing names, this is very handy to have: various backend have different namespace, and different requirements.

This PR introduces the concept of a View: a strongly-typed, hierarchical representation of a Rust path.
A View is a list of path segments, where each segment carries:

  • its kind (crate, module, type, trait, impl, assoc item, constructor, field, etc.),
  • whether it's named or unnamed (e.g. impl, anon const),
  • links to its parent segment (e.g. field → constructor → type).

This makes paths robust to traversal, disambiguation, and rendering, while directly encoding rustc's invariants about what can appear where.

The PR also provides nice traits to deal with per-backend name printing, but those are a bit rough for now.
They don't handle a lot for you out of the box (e.g. name clashes, reserved names, etc.).
I will address that in a follow up PR.

This PR fixes #1599.

@W95Psp W95Psp force-pushed the rengine-name-views branch 3 times, most recently from e9be012 to ee36444 Compare August 27, 2025 07:59
@W95Psp W95Psp marked this pull request as ready for review August 27, 2025 09:01
@W95Psp W95Psp requested a review from a team as a code owner August 27, 2025 09:01
@W95Psp W95Psp requested a review from maximebuyse August 27, 2025 09:01
@W95Psp W95Psp force-pushed the rengine-name-views branch from 639ca93 to f63c0c7 Compare August 27, 2025 09:01
@W95Psp W95Psp requested a review from clementblaudeau August 27, 2025 09:04
@W95Psp W95Psp requested review from a team, jschneider-bensch and keks and removed request for a team and jschneider-bensch August 27, 2025 09:14
@maximebuyse
Copy link
Copy Markdown
Contributor

Generally looks good. I mostly looked at documentation (and it looks great!). I tested on a Lean example and it seems to produce weird stuff. hax_machine_int_le becomes Rust_primitives.Hax.Machine_int.le so it doesn't work with names defined in the lib. And pre/post conditions have weird names like Crate._.requires or Crate.__1.ensures (that's probably less of a problem because we need to improve name rendering for those anyway). Do we want to fix this in a follow up PR? Because I guess this will break examples like Barrett or chacha.

@W95Psp
Copy link
Copy Markdown
Contributor Author

W95Psp commented Aug 27, 2025

Thanks for the review, ah indeed, I was mainly using Lean to test my rendering mechanism.
I will revert the Lean changes (will be easier for @clementblaudeau to work with probably)!

@W95Psp W95Psp removed the request for review from keks August 27, 2025 09:46
@W95Psp
Copy link
Copy Markdown
Contributor Author

W95Psp commented Aug 27, 2025

(just noticed @keks is out, since @jschneider-bensch is out as well, I'm asking you @franziskuskiefer)

Copy link
Copy Markdown
Member

@franziskuskiefer franziskuskiefer left a comment

Choose a reason for hiding this comment

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

generally lgtm but there are panics in here again.

Copy link
Copy Markdown
Member

@franziskuskiefer franziskuskiefer left a comment

Choose a reason for hiding this comment

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

Thanks for addressing the panic issue. That solution sounds good to me.
Maybe clearly separating out the binary and library use case would also help.
But I just want to avoid pushing the problem up the stack by crashing without offering a good way of handling this for the caller.

@W95Psp
Copy link
Copy Markdown
Contributor Author

W95Psp commented Aug 28, 2025

Sounds good.
I opened #1630 as a follow up, which will address your two other questions.

@W95Psp W95Psp enabled auto-merge August 28, 2025 13:25
@W95Psp W95Psp force-pushed the rengine-name-views branch from 9c1b598 to bb2ce7d Compare August 28, 2025 13:30
@W95Psp W95Psp removed the request for review from clementblaudeau August 28, 2025 13:31
@W95Psp W95Psp added this pull request to the merge queue Aug 28, 2025
Merged via the queue into main with commit 5ef2ead Aug 28, 2025
17 checks passed
@W95Psp W95Psp deleted the rengine-name-views branch August 28, 2025 14:42
@W95Psp W95Psp restored the rengine-name-views 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.

Add proper rendering of GlobalId in the Rust Engine

3 participants