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

Provide access to item attributes #83

Open
celinval opened this issue Jun 18, 2024 · 6 comments · Fixed by rust-lang/rust#127022
Open

Provide access to item attributes #83

celinval opened this issue Jun 18, 2024 · 6 comments · Fixed by rust-lang/rust#127022
Assignees

Comments

@celinval
Copy link
Contributor

celinval commented Jun 18, 2024

A definition in Rust can have attributes attached to them, .e.g., tool attributes. They are used by different tools to add metadata to the user code that can provide more information on how the code must be analyzed.

StableMIR should provide an API to retrieve the attributes of a crate definition. We can probably add a new method to the CrateDef that returns a list of Attributes attached to them.

@adwinwhite
Copy link

@rustbot claim

@adwinwhite
Copy link

rustc_ast::ast::Attribute's spec is quite complicated so we probably don't want to mirror it.

My plan:

  • define stable Attribute similar to Span, merely as an index to internal struct.
  • add two most needed methods value_str and path for our stable Attribute.

Is this design okay?

@celinval
Copy link
Contributor Author

celinval commented Jun 25, 2024

That's a great start! Maybe add span() too.

I would like to eventually expose the tokens() method that returns proc_macro::TokenStream, so users can use crates such as syn to extract the contents of the attribute. That said, I haven't done any thorough investigation to see if that would be feasible and a reasonable implementation.

@celinval
Copy link
Contributor Author

@adwinwhite
Copy link

Good idea. Perhaps I should start with span() and to_string() for now. String can be easily parsed into TokenStream if needed.

tokens() could save us some parsing, but it would also introduce rustc_ast as a dependency to stable_mir which has basically no dependency for now.

matthiaskrgr added a commit to matthiaskrgr/rust that referenced this issue Jun 28, 2024
Support fetching `Attribute` of items.

Fixes [rust-lang/project-stable-mir#83

`rustc_ast::ast::Attribute` doesn't impl `Hash` and `Eq`. Thus it cannot be directly used as key of `IndexMap` in `rustc_smir::rustc_smir::Tables` and we cannot define stable `Attribute` as index to `rustc_ast::ast::Attribute` like `Span` and many other stable definitions.

Since an string (or tokens) and its span contain all info about an attribute, I defined a simple `Attribute` struct on stable side.

I choose to fetch attributes via `tcx::get_attrs_by_path()` due to `get_attrs()` is marked as deprecated and `get_attrs_by_name()` cannot handle name of multiple segments like `rustfmt::skip`.

r? `@celinval`
rust-timer added a commit to rust-lang-ci/rust that referenced this issue Jun 28, 2024
Rollup merge of rust-lang#127022 - adwinwhite:attrs, r=celinval

Support fetching `Attribute` of items.

Fixes [rust-lang/project-stable-mir#83

`rustc_ast::ast::Attribute` doesn't impl `Hash` and `Eq`. Thus it cannot be directly used as key of `IndexMap` in `rustc_smir::rustc_smir::Tables` and we cannot define stable `Attribute` as index to `rustc_ast::ast::Attribute` like `Span` and many other stable definitions.

Since an string (or tokens) and its span contain all info about an attribute, I defined a simple `Attribute` struct on stable side.

I choose to fetch attributes via `tcx::get_attrs_by_path()` due to `get_attrs()` is marked as deprecated and `get_attrs_by_name()` cannot handle name of multiple segments like `rustfmt::skip`.

r? `@celinval`
@celinval
Copy link
Contributor Author

Let's resolve this for now. We can always add token() later. Thanks again @adwinwhite

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 a pull request may close this issue.

2 participants