Skip to content
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

[META] Engine: enhancements to the concrete ident view API #973

Open
2 of 4 tasks
W95Psp opened this issue Oct 8, 2024 · 2 comments
Open
2 of 4 tasks

[META] Engine: enhancements to the concrete ident view API #973

W95Psp opened this issue Oct 8, 2024 · 2 comments
Labels
engine Issue in the engine enhancement New feature or request meta

Comments

@W95Psp
Copy link
Collaborator

W95Psp commented Oct 8, 2024

As pointed out by @paulmure in #793, the current view API for concrete ident is limited.
As discussed there, we might either (1) expose the raw types as #793 suggests or (2) build a more expressive API.

I'm in favor of (2) because:

  • the representation of concrete idents in the engine is complex: it's basically rustc DefIds + a kind we compute in import_thir
  • there are invariants on those concrete idents, I'd like them to be contained in this module
  • the view API exists because I want to ensure monolithically that we don't produce a same backend identifier for two different Rust identifiers.

The danger of (2) is to end up with a complex and messy view API.

The purpose of this issue is to describe the various missing functionality of this view API, and to track refactoring work to make it more modular.

Expressiveness: name policy

  • reserved words: a list of keywords that should be escaped (valid identifiers in Rust are not always valid in the target backend)
  • transform numeric fields on structs: tuple structs can be accessed via e.g. x.1 in Rust, sometimes not in the backend, so we need to rename
  • control prefixes added by the name API (as suggested by @paulmure in Raw view on concrete_ident #793). We currently hardcode many rules, e.g. rename constructor that begins with anything else but an uppercase Latin letter. We should be able to configure those rules: what prefix is added, when.
  • control how much of the fully qualified path is used based on what’s currently imported (quoting @paulmure)
  • Please comment if you think about something here!
@W95Psp W95Psp added engine Issue in the engine enhancement New feature or request meta labels Oct 8, 2024
Copy link

This issue has been marked as stale due to a lack of activity for 60 days. If you believe this issue is still relevant, please provide an update or comment to keep it open. Otherwise, it will be closed in 7 days.

@github-actions github-actions bot added the stale label Dec 10, 2024
@W95Psp
Copy link
Collaborator Author

W95Psp commented Dec 16, 2024

Still relevant, and related to #1135.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
engine Issue in the engine enhancement New feature or request meta
Projects
None yet
Development

No branches or pull requests

1 participant