Nothing Special   »   [go: up one dir, main page]
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

Bad span in suggestion for unsafe_attr_outside_unsafe in macro expansion with cfg_attr #132908

Open
ehuss opened this issue Nov 11, 2024 · 3 comments · May be fixed by #133270
Open

Bad span in suggestion for unsafe_attr_outside_unsafe in macro expansion with cfg_attr #132908

ehuss opened this issue Nov 11, 2024 · 3 comments · May be fixed by #133270
Labels
A-diagnostics Area: Messages for errors, warnings, and lints A-edition-2024 Area: The 2024 edition A-lint Area: Lints (warnings about flaws in source code) such as unused_mut. A-macros Area: All kinds of macros (custom derive, macro_rules!, proc macros, ..) A-suggestion-diagnostics Area: Suggestions generated by the compiler applied by `cargo fix` C-bug Category: This is a bug. D-edition Diagnostics: An error or lint that should account for edition differences. D-invalid-suggestion Diagnostics: A structured suggestion resulting in incorrect code. L-unsafe_attr_outside_unsafe Lint: unsafe_attr_outside_unsafe T-compiler Relevant to the compiler team, which will review and decide on the PR/issue.

Comments

@ehuss
Copy link
Contributor
ehuss commented Nov 11, 2024

The following code:

#![warn(unsafe_attr_outside_unsafe)]

macro_rules! m {
    () => {
        #[cfg_attr(all(), link_section = ".text.startup")]
        unsafe extern "C" fn __ctor() {}
    };
}

m! {}

generates a suggestion which when applied looks like:

#[cfg_attr(all(), liunsafe(nk_section = ".text.startup)")]

Somehow the span is off by two bytes on both the start and end.

This doesn't happen outside of a macro expansion, and it seems to also require the cfg_attr.

Meta

rustc --version --verbose:

rustc 1.84.0-nightly (143ce0920 2024-11-10)
binary: rustc
commit-hash: 143ce0920a2307b19831160a01f06f107610f1b2
commit-date: 2024-11-10
host: aarch64-unknown-linux-gnu
release: 1.84.0-nightly
LLVM version: 19.1.3
@ehuss ehuss added A-edition-2024 Area: The 2024 edition C-bug Category: This is a bug. D-edition Diagnostics: An error or lint that should account for edition differences. D-invalid-suggestion Diagnostics: A structured suggestion resulting in incorrect code. L-unsafe_attr_outside_unsafe Lint: unsafe_attr_outside_unsafe labels Nov 11, 2024
@rustbot rustbot added the needs-triage This issue may need triage. Remove it if it has been sufficiently triaged. label Nov 11, 2024
@jieyouxu jieyouxu added A-macros Area: All kinds of macros (custom derive, macro_rules!, proc macros, ..) A-lint Area: Lints (warnings about flaws in source code) such as unused_mut. A-diagnostics Area: Messages for errors, warnings, and lints T-compiler Relevant to the compiler team, which will review and decide on the PR/issue. A-suggestion-diagnostics Area: Suggestions generated by the compiler applied by `cargo fix` and removed needs-triage This issue may need triage. Remove it if it has been sufficiently triaged. labels Nov 11, 2024
@traviscross
Copy link
Contributor

cc @carbotaniuman

@ehuss
Copy link
Contributor Author
ehuss commented Nov 20, 2024 •
edited
Loading

I'm pretty sure this is related to this line:

rust/compiler/rustc_parse/src/validate_attr.rs

Line 172 in bfe809d

attr.span.with_lo(attr.span.lo() + BytePos(2)).with_hi(attr.span.hi() - BytePos(1))

This is assuming that the span is around #[no_mangle], and the offsets are to go inside the #[ and ]. However, with cfg_attr, that's not a valid assumption.

I'm looking at this in relation to #132906, as I would like to figure out a way to prevent the lint from firing on macro_rules generated from a proc-macro. However, I haven't cracked that problem. Thought I would note that I see where the problem here is.

EDIT: Correction: This only happens from a macro, so I'm not sure why it is different between the two.

@ehuss ehuss linked a pull request Nov 21, 2024 that will close this issue
Open
@ehuss
Copy link
Contributor Author
ehuss commented Nov 21, 2024

Posted a fix in #133270.

bors added a commit to rust-lang-ci/rust that referenced this issue Nov 22, 2024
@bors
Auto merge of rust-lang#133270 - ehuss:fix-attr-item-span, r=<try>
46c9e3a 
Fix span of unsafe attribute diagnostic

This fixes the span of the `unsafe_attr_outside_unsafe` diagnostic when the attribute uses `cfg_attr` and comes from a macro. Previously the span it was pointing to was in the wrong place (offset by 2 bytes in the start, and 1 byte in the end), causing a corrupt suggestion.

The problem is that the lint was trying to do manual byte manipulation of the `Attribute` span to get within the `#[` and `]` tokens. However, when the attribute comes from `cfg_attr`, that span starts from the attribute path (like `no_mangle`), not the `#[` of the `cfg_attr`.

The solution here is to store the span of the `AttrItem` while parsing, so that we know for sure that it covers the correct range (the path and all args). We could not use `AttrItem::span()` (which is removed in this PR), because that function did not correctly account for the path and arguments coming from separate expansion contexts. For example, in the macro expansion of `#[$p = $a]`, the span would be `$p = ` because you are not allowed to generate a span across expansion contexts.

Fixes rust-lang#132908
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees
No one assigned
Labels
A-diagnostics Area: Messages for errors, warnings, and lints A-edition-2024 Area: The 2024 edition A-lint Area: Lints (warnings about flaws in source code) such as unused_mut. A-macros Area: All kinds of macros (custom derive, macro_rules!, proc macros, ..) A-suggestion-diagnostics Area: Suggestions generated by the compiler applied by `cargo fix` C-bug Category: This is a bug. D-edition Diagnostics: An error or lint that should account for edition differences. D-invalid-suggestion Diagnostics: A structured suggestion resulting in incorrect code. L-unsafe_attr_outside_unsafe Lint: unsafe_attr_outside_unsafe T-compiler Relevant to the compiler team, which will review and decide on the PR/issue.
Projects
None yet
Milestone
No milestone
Development

Successfully merging a pull request may close this issue.

Fix span of unsafe attribute diagnostic ehuss/rust
4 participants
@ehuss @traviscross @jieyouxu @rustbot