Skip to content

We have two independent checks for "is this attribute unsafe" #131801

New issue

Have a question about this project? # for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “#”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? # to your account

Open
RalfJung opened this issue Oct 16, 2024 · 1 comment
Open

We have two independent checks for "is this attribute unsafe" #131801

RalfJung opened this issue Oct 16, 2024 · 1 comment
Labels
A-attributes Area: Attributes (`#[…]`, `#![…]`) C-cleanup Category: PRs that clean code up or issues documenting cleanup. T-compiler Relevant to the compiler team, which will review and decide on the PR/issue.

Comments

@RalfJung
Copy link
Member

Turns out #123757 was implemented by duplicating existing information previously added for #82499:

  • Attributes are marked as unsafe here:

ungated!(unsafe export_name, Normal, template!(NameValueStr: "name"), FutureWarnPreceding, EncodeCrossCrate::No),
ungated!(unsafe link_section, Normal, template!(NameValueStr: "name"), FutureWarnPreceding, EncodeCrossCrate::No),
ungated!(unsafe no_mangle, Normal, template!(Word), WarnFollowing, EncodeCrossCrate::No),

  • But separately, the unsafety checker has a list of unsafe attributes here:

if let Some(attr) = attr::find_by_name(&it.attrs, sym::no_mangle) {
self.report_unsafe(cx, attr.span, BuiltinUnsafe::NoMangleFn);
}
if let Some(attr) = attr::find_by_name(&it.attrs, sym::export_name) {
self.report_unsafe(cx, attr.span, BuiltinUnsafe::ExportNameFn);
}
if let Some(attr) = attr::find_by_name(&it.attrs, sym::link_section) {
self.report_unsafe(cx, attr.span, BuiltinUnsafe::LinkSectionFn);
}

and here:

if let Some(attr) = attr::find_by_name(&it.attrs, sym::no_mangle) {
self.report_unsafe(cx, attr.span, BuiltinUnsafe::NoMangleStatic);
}
if let Some(attr) = attr::find_by_name(&it.attrs, sym::export_name) {
self.report_unsafe(cx, attr.span, BuiltinUnsafe::ExportNameStatic);
}
if let Some(attr) = attr::find_by_name(&it.attrs, sym::link_section) {
self.report_unsafe(cx, attr.span, BuiltinUnsafe::LinkSectionStatic);
}

and here:

if let Some(attr) = attr::find_by_name(&it.attrs, sym::no_mangle) {
self.report_unsafe(cx, attr.span, BuiltinUnsafe::NoMangleMethod);
}
if let Some(attr) = attr::find_by_name(&it.attrs, sym::export_name) {
self.report_unsafe(cx, attr.span, BuiltinUnsafe::ExportNameMethod);
}

This should be cleaned up into a single source of truth for which attributes are unsafe.

@RalfJung RalfJung added the C-cleanup Category: PRs that clean code up or issues documenting cleanup. label Oct 16, 2024
@rustbot rustbot added the needs-triage This issue may need triage. Remove it if it has been sufficiently triaged. label Oct 16, 2024
@fmease fmease added A-attributes Area: Attributes (`#[…]`, `#![…]`) T-compiler Relevant to the compiler team, which will review and decide on the PR/issue. and removed needs-triage This issue may need triage. Remove it if it has been sufficiently triaged. labels Oct 16, 2024
@RalfJung
Copy link
Member Author

@jdonszelmann with the current state of your attribute rework, is this issue still open or are the checks concentrated in a single place now?

# for free to join this conversation on GitHub. Already have an account? # to comment
Labels
A-attributes Area: Attributes (`#[…]`, `#![…]`) C-cleanup Category: PRs that clean code up or issues documenting cleanup. T-compiler Relevant to the compiler team, which will review and decide on the PR/issue.
Projects
None yet
Development

No branches or pull requests

3 participants