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.

Sign up for GitHub

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

Jump to bottom

TyCtxt::get_attr should check that no duplicates are allowed #100631

Closed
lcnr opened this issue Aug 16, 2022 · 1 comment · Fixed by #100658
Closed

TyCtxt::get_attr should check that no duplicates are allowed #100631

lcnr opened this issue Aug 16, 2022 · 1 comment · Fixed by #100658
Assignees
@chenyukang
Labels
A-attributes Area: #[attributes(..)] C-cleanup Category: PRs that clean code up or issues documenting cleanup. E-medium Call for participation: Medium difficulty. Experience needed to fix: Intermediate. E-mentor Call for participation: This issue has a mentor. Use #t-compiler/help on Zulip for discussion.

Comments

@lcnr
Copy link
Contributor

lcnr commented Aug 16, 2022 •
edited by rustbot
Loading

get_attr currently happily ignores any additional attributes in case there are more than one. While this is fine for attributes which don't care about duplicates or emit a warning/error in case there are more than one, this isn't currently checked in the compiler, potentially causing bugs.

rust/compiler/rustc_middle/src/ty/mod.rs

Lines 2282 to 2284 in a39bdb1

pub fn get_attr(self, did: DefId, attr: Symbol) -> Option<&'tcx ast::Attribute> {
self.get_attrs(did, attr).next()
}

similar to the check in has_attr we should check that the attr declaration in rustc_feature/builtin_attrs.rs has an acceptable way of dealing with duplicates.

rust/compiler/rustc_middle/src/ty/mod.rs

Lines 2287 to 2290 in a39bdb1

pub fn has_attr(self, did: DefId, attr: Symbol) -> bool {
if cfg!(debug_assertions) && !did.is_local() && rustc_feature::is_builtin_only_local(attr) {
bug!("tried to access the `only_local` attribute `{}` from an extern crate", attr);
} else {

in case there are any questions on how to implement this, feel free to open a thread on zulip and pinging me there.
@lcnr lcnr added C-cleanup Category: PRs that clean code up or issues documenting cleanup. A-attributes Area: #[attributes(..)] E-mentor Call for participation: This issue has a mentor. Use #t-compiler/help on Zulip for discussion. E-medium Call for participation: Medium difficulty. Experience needed to fix: Intermediate. labels Aug 16, 2022
@chenyukang
Copy link
Member

@rustbot claim

@chenyukang chenyukang mentioned this issue Aug 17, 2022
Merged
matthiaskrgr added a commit to matthiaskrgr/rust that referenced this issue Sep 5, 2022
@matthiaskrgr
Rollup merge of rust-lang#100658 - chenyukang:100631-check-get-attr, …
96704a6 
…r=lcnr

TyCtxt::get_attr should check that no duplicates are allowed

Fixes rust-lang#100631
JohnTitor added a commit to JohnTitor/rust that referenced this issue Sep 5, 2022
@JohnTitor
Rollup merge of rust-lang#100658 - chenyukang:100631-check-get-attr, …
d4e12d6 
…r=lcnr

TyCtxt::get_attr should check that no duplicates are allowed

Fixes rust-lang#100631
@bors bors closed this as completed in 3f22020 Sep 6, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees

@chenyukang chenyukang

Labels
A-attributes Area: #[attributes(..)] C-cleanup Category: PRs that clean code up or issues documenting cleanup. E-medium Call for participation: Medium difficulty. Experience needed to fix: Intermediate. E-mentor Call for participation: This issue has a mentor. Use #t-compiler/help on Zulip for discussion.
Projects
None yet
Milestone
No milestone
Development

Successfully merging a pull request may close this issue.

TyCtxt::get_attr should check that no duplicates are allowed chenyukang/rust
2 participants
@chenyukang @lcnr