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

Regression from new lint out_of_scope_macro_calls #126984

Closed
tgross35 opened this issue Jun 26, 2024 · 6 comments · Fixed by #126987
Closed

Regression from new lint out_of_scope_macro_calls #126984

tgross35 opened this issue Jun 26, 2024 · 6 comments · Fixed by #126987
Assignees
Labels
A-lint Area: Lints (warnings about flaws in source code) such as unused_mut. C-bug Category: This is a bug. L-out_of_scope_macro_calls Lint: out_of_scope_macro_calls regression-from-stable-to-nightly Performance or correctness regression from stable to nightly. T-compiler Relevant to the compiler team, which will review and decide on the PR/issue.

Comments

@tgross35
Copy link
Contributor

tgross35 commented Jun 26, 2024

compiler_builtins is now getting:

 error: cannot find macro `define_rust_probestack` in this scope
   --> src/probestack.rs:134:5
    |
137 |     define_rust_probestack!(
    |     ^^^^^^^^^^^^^^^^^^^^^^
    |
    = warning: this was previously accepted by the compiler but is being phased out; it will become a hard error in a future release!
    = note: for more information, see issue #124535 <https://github.com/rust-lang/rust/issues/124535>
    = help: import `macro_rules` with `use` to make it callable above its definition
    = note: `-D out-of-scope-macro-calls` implied by `-D warnings`
    = help: to override `-D warnings` add `#[allow(out_of_scope_macro_calls)]`

But this doesn't seem right, unless I am misreading something. The global_asm! invocation that is enabled on x86_64 (https://github.com/rust-lang/compiler-builtins/blob/7a84ce4a90cd2d22494daead50f9026b5ca52140/src/probestack.rs#L130-L134) should find the define_rustc_probestack definition that is enabled on everything except apple or uefi (https://github.com/rust-lang/compiler-builtins/blob/7a84ce4a90cd2d22494daead50f9026b5ca52140/src/probestack.rs#L61-L79).

CI: https://github.com/rust-lang/compiler-builtins/actions/runs/9675571819/job/26693340492?pr=636#step:5:24

Unfortunately I haven't been successful in finding a mcve, https://play.rust-lang.org/?version=nightly&mode=debug&edition=2021&gist=f299db7ba55daaa5b7721323bf72cfb8 works fine.

@tgross35 tgross35 added the C-bug Category: This is a bug. label Jun 26, 2024
@rustbot rustbot added the needs-triage This issue may need triage. Remove it if it has been sufficiently triaged. label Jun 26, 2024
@tgross35
Copy link
Contributor Author

tgross35 commented Jun 26, 2024

I can reproduce locally with rustc 1.81.0-nightly (fda509e 2024-06-25)

@rustbot label +E-needs-mcve +A-lint +T-compiler +regression-from-stable-to-nightly -needs-triage

Cc @petrochenkov since you added the lint in #125741

@rustbot rustbot added A-lint Area: Lints (warnings about flaws in source code) such as unused_mut. E-needs-mcve Call for participation: This issue has a repro, but needs a Minimal Complete and Verifiable Example T-compiler Relevant to the compiler team, which will review and decide on the PR/issue. regression-from-stable-to-nightly Performance or correctness regression from stable to nightly. I-prioritize Issue: Indicates that prioritization has been requested for this issue. and removed needs-triage This issue may need triage. Remove it if it has been sufficiently triaged. labels Jun 26, 2024
@petrochenkov
Copy link
Contributor

Yes, looks like a false positive caused by global_asm! needing a special treatment.

@petrochenkov
Copy link
Contributor

It's not about global asm, it's about any eager expansion in macro call items.
Fixed in #126987.

@tgross35
Copy link
Contributor Author

Awesome, thanks for the quick fix

@apiraino apiraino removed E-needs-mcve Call for participation: This issue has a repro, but needs a Minimal Complete and Verifiable Example I-prioritize Issue: Indicates that prioritization has been requested for this issue. labels Jun 26, 2024
@jieyouxu jieyouxu added the L-out_of_scope_macro_calls Lint: out_of_scope_macro_calls label Jun 29, 2024
@tgross35
Copy link
Contributor Author

There might be another bug with this lint, seems like it might not be recognized as an inner attribute. Trying to ignore it gives contradictory messages:

error: unknown lint: `out_of_scope_macro_calls`
  --> src/probestack.rs:52:10
   |
52 | #![allow(out_of_scope_macro_calls)]
   |          ^^^^^^^^^^^^^^^^^^^^^^^^
   |
   = note: `-D unknown-lints` implied by `-D warnings`
   = help: to override `-D warnings` add `#[allow(unknown_lints)]`

error: cannot find macro `define_rust_probestack` in this scope
   --> src/probestack.rs:265:5
    |
265 |     define_rust_probestack!(
    |     ^^^^^^^^^^^^^^^^^^^^^^
    |
    = warning: this was previously accepted by the compiler but is being phased out; it will become a hard error in a future release!
    = note: for more information, see issue #124535 <https://github.com/rust-lang/rust/issues/124535>
    = help: import `macro_rules` with `use` to make it callable above its definition
    = note: `-D out-of-scope-macro-calls` implied by `-D warnings`
    = help: to override `-D warnings` add `#[allow(out_of_scope_macro_calls)]`

From trying to silence the error in compiler_builtins, https://github.com/rust-lang/compiler-builtins/actions/runs/9735325154/job/26864417423#step:13:2321

@beetrees
Copy link
Contributor

I don't think the lint is ever actually being registered anywhere; more specifically it needs to be added to the HardwiredLints lint pass at the top of rustc_lint_defs/src/builtin.rs.

matthiaskrgr added a commit to matthiaskrgr/rust that referenced this issue Jul 1, 2024
…ro-calls, r=compiler-errors

Ensure `out_of_scope_macro_calls` lint is registered

Fixes part of rust-lang#126984 (comment).
matthiaskrgr added a commit to matthiaskrgr/rust that referenced this issue Jul 1, 2024
…ro-calls, r=compiler-errors

Ensure `out_of_scope_macro_calls` lint is registered

Fixes part of rust-lang#126984 (comment).
rust-timer added a commit to rust-lang-ci/rust that referenced this issue Jul 1, 2024
Rollup merge of rust-lang#127191 - beetrees:register-out-of-scope-macro-calls, r=compiler-errors

Ensure `out_of_scope_macro_calls` lint is registered

Fixes part of rust-lang#126984 (comment).
workingjubilee added a commit to workingjubilee/rustc that referenced this issue Jul 5, 2024
out_of_scope_macro_calls: Detect calls inside attributes more precisely

Fixes rust-lang#126984.
@bors bors closed this as completed in 6ba80a9 Jul 7, 2024
tgross35 added a commit to tgross35/compiler-builtins that referenced this issue Sep 28, 2024
<rust-lang/rust#126984> has been resolved.
Remove the workaround that was introduced to suppress it.

This reverts commit 254edbc.
tgross35 added a commit to tgross35/compiler-builtins that referenced this issue Sep 28, 2024
<rust-lang/rust#126984> has been resolved.
Remove the workaround that was introduced to suppress it.

This reverts commit 254edbc.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
A-lint Area: Lints (warnings about flaws in source code) such as unused_mut. C-bug Category: This is a bug. L-out_of_scope_macro_calls Lint: out_of_scope_macro_calls regression-from-stable-to-nightly Performance or correctness regression from stable to nightly. T-compiler Relevant to the compiler team, which will review and decide on the PR/issue.
Projects
None yet
Development

Successfully merging a pull request may close this issue.

6 participants