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

Indicate in non_local_defs lint that the macro needs to change #125722

Merged
merged 2 commits into from
Jun 15, 2024
Merged

Indicate in non_local_defs lint that the macro needs to change #125722

merged 2 commits into from
Jun 15, 2024

Conversation

Urgau
Copy link
Member

@Urgau Urgau commented May 29, 2024 •
edited
Loading

This PR adds a note to indicate that the macro needs to change in the non_local_definitions lint output.

Address #125089 (comment)
Fixes #125681
r? @estebank

@rustbot rustbot added S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. T-compiler Relevant to the compiler team, which will review and decide on the PR/issue. labels May 29, 2024
@Urgau Urgau added the L-non_local_definitions Lint: non_local_definitions label May 29, 2024
@Urgau Urgau force-pushed the non_local_defs-macro-to-change branch from 6f48a1e to b5d4867 Compare May 29, 2024 15:05
@Urgau Urgau mentioned this pull request Jun 8, 2024
Closed
@estebank
Copy link
Contributor

@bors r+

@bors
Copy link
Contributor

bors commented Jun 13, 2024

📌 Commit b5d4867 has been approved by estebank

It is now in the queue for this repository.

@bors
Copy link
Contributor

bors commented Jun 13, 2024

🌲 The tree is currently closed for pull requests below priority 13. This pull request will be tested once the tree is reopened.

@bors bors added S-waiting-on-bors Status: Waiting on bors to run and complete tests. Bors will change the label on completion. and removed S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. labels Jun 13, 2024
workingjubilee added a commit to workingjubilee/rustc that referenced this pull request Jun 13, 2024
@workingjubilee
Rollup merge of rust-lang#125722 - Urgau:non_local_defs-macro-to-chan…
1e12128 
…ge, r=estebank

Indicate in `non_local_defs` lint that the macro needs to change

This PR adds a note to indicate that the macro needs to change in the `non_local_definitions` lint output.

Address rust-lang#125089 (comment)
Fixes rust-lang#125681
r? `@estebank`
@workingjubilee workingjubilee mentioned this pull request Jun 13, 2024
Closed
bors added a commit to rust-lang-ci/rust that referenced this pull request Jun 13, 2024
@bors
Auto merge of rust-lang#126446 - workingjubilee:rollup-esdbgdw, r=wor…
bdef683 
…kingjubilee

Rollup of 8 pull requests

Successful merges:

 - rust-lang#121216 (Always emit `native-static-libs` note, even if it is empty)
 - rust-lang#123726 (Clarify `Command::new` behavior for programs with arguments)
 - rust-lang#125722 (Indicate in `non_local_defs` lint that the macro needs to change)
 - rust-lang#126088 ([1/2] clean-up / general improvements)
 - rust-lang#126390 (Fix wording in {checked_}next_power_of_two)
 - rust-lang#126392 (Small style improvement in `gvn.rs`)
 - rust-lang#126402 (Fix wrong `assert_unsafe_precondition` message for `core::ptr::copy`)
 - rust-lang#126445 (Remove failing GUI test to stop blocking CI until it is fixed)

r? `@ghost`
`@rustbot` modify labels: rollup
bors added a commit to rust-lang-ci/rust that referenced this pull request Jun 13, 2024
@bors
Auto merge of rust-lang#126446 - workingjubilee:rollup-esdbgdw, r=wor…
2165cf6 
…kingjubilee

Rollup of 8 pull requests

Successful merges:

 - rust-lang#121216 (Always emit `native-static-libs` note, even if it is empty)
 - rust-lang#123726 (Clarify `Command::new` behavior for programs with arguments)
 - rust-lang#125722 (Indicate in `non_local_defs` lint that the macro needs to change)
 - rust-lang#126088 ([1/2] clean-up / general improvements)
 - rust-lang#126390 (Fix wording in {checked_}next_power_of_two)
 - rust-lang#126392 (Small style improvement in `gvn.rs`)
 - rust-lang#126402 (Fix wrong `assert_unsafe_precondition` message for `core::ptr::copy`)
 - rust-lang#126445 (Remove failing GUI test to stop blocking CI until it is fixed)

r? `@ghost`
`@rustbot` modify labels: rollup
matthiaskrgr added a commit to matthiaskrgr/rust that referenced this pull request Jun 14, 2024
@matthiaskrgr
Rollup merge of rust-lang#125722 - Urgau:non_local_defs-macro-to-chan…
3e8c6e5 
…ge, r=estebank

Indicate in `non_local_defs` lint that the macro needs to change

This PR adds a note to indicate that the macro needs to change in the `non_local_definitions` lint output.

Address rust-lang#125089 (comment)
Fixes rust-lang#125681
r? ``@estebank``
@matthiaskrgr matthiaskrgr mentioned this pull request Jun 14, 2024
Closed
bors added a commit to rust-lang-ci/rust that referenced this pull request Jun 14, 2024
@bors
Auto merge of rust-lang#126461 - matthiaskrgr:rollup-tepitxf, r=matth…
46fb648 
…iaskrgr

Rollup of 6 pull requests

Successful merges:

 - rust-lang#125293 (Place tail expression behind terminating scope)
 - rust-lang#125722 (Indicate in `non_local_defs` lint that the macro needs to change)
 - rust-lang#126192 (Various Redox OS fixes and add i686 Redox OS target)
 - rust-lang#126285 (`UniqueRc`: support allocators and `T: ?Sized`.)
 - rust-lang#126352 (ci: Update centos:7 to use vault repos)
 - rust-lang#126399 (extend the check for LLVM build)

r? `@ghost`
`@rustbot` modify labels: rollup
matthiaskrgr added a commit to matthiaskrgr/rust that referenced this pull request Jun 14, 2024
@matthiaskrgr
Rollup merge of rust-lang#125722 - Urgau:non_local_defs-macro-to-chan…
56e204b 
…ge, r=estebank

Indicate in `non_local_defs` lint that the macro needs to change

This PR adds a note to indicate that the macro needs to change in the `non_local_definitions` lint output.

Address rust-lang#125089 (comment)
Fixes rust-lang#125681
r? ```@estebank```
@matthiaskrgr matthiaskrgr mentioned this pull request Jun 14, 2024
Closed
matthiaskrgr added a commit to matthiaskrgr/rust that referenced this pull request Jun 14, 2024
@matthiaskrgr
Rollup merge of rust-lang#125722 - Urgau:non_local_defs-macro-to-chan…
73f769e 
…ge, r=estebank

Indicate in `non_local_defs` lint that the macro needs to change

This PR adds a note to indicate that the macro needs to change in the `non_local_definitions` lint output.

Address rust-lang#125089 (comment)
Fixes rust-lang#125681
r? ````@estebank````
@matthiaskrgr matthiaskrgr mentioned this pull request Jun 14, 2024
Closed
bors added a commit to rust-lang-ci/rust that referenced this pull request Jun 14, 2024
@bors
Auto merge of rust-lang#126480 - matthiaskrgr:rollup-w2jcybu, r=matth…
3e64975 
…iaskrgr

Rollup of 8 pull requests

Successful merges:

 - rust-lang#125722 (Indicate in `non_local_defs` lint that the macro needs to change)
 - rust-lang#125829 (rustc_span: Add conveniences for working with span formats)
 - rust-lang#126128 (Consistently use subtyping in method resolution)
 - rust-lang#126192 (Various Redox OS fixes and add i686 Redox OS target)
 - rust-lang#126352 (ci: Update centos:7 to use vault repos)
 - rust-lang#126354 (Use `Variance` glob imported variants everywhere)
 - rust-lang#126469 (MIR Shl/Shr: the offset can be computed with rem_euclid)
 - rust-lang#126472 (build `libcxx-version` only when it doesn't exist)

r? `@ghost`
`@rustbot` modify labels: rollup
matthiaskrgr added a commit to matthiaskrgr/rust that referenced this pull request Jun 14, 2024
@matthiaskrgr
Rollup merge of rust-lang#125722 - Urgau:non_local_defs-macro-to-chan…
b81fa04 
…ge, r=estebank

Indicate in `non_local_defs` lint that the macro needs to change

This PR adds a note to indicate that the macro needs to change in the `non_local_definitions` lint output.

Address rust-lang#125089 (comment)
Fixes rust-lang#125681
r? `````@estebank`````
@matthiaskrgr matthiaskrgr mentioned this pull request Jun 14, 2024
Closed
bors added a commit to rust-lang-ci/rust that referenced this pull request Jun 14, 2024
@bors
Auto merge of rust-lang#126486 - matthiaskrgr:rollup-yrcxb34, r=matth…
93b52c9 
…iaskrgr

Rollup of 8 pull requests

Successful merges:

 - rust-lang#125722 (Indicate in `non_local_defs` lint that the macro needs to change)
 - rust-lang#125829 (rustc_span: Add conveniences for working with span formats)
 - rust-lang#126192 (Various Redox OS fixes and add i686 Redox OS target)
 - rust-lang#126352 (ci: Update centos:7 to use vault repos)
 - rust-lang#126354 (Use `Variance` glob imported variants everywhere)
 - rust-lang#126469 (MIR Shl/Shr: the offset can be computed with rem_euclid)
 - rust-lang#126472 (build `libcxx-version` only when it doesn't exist)
 - rust-lang#126476 (Fix running bootstrap tests with a local Rust toolchain as the stage0)

r? `@ghost`
`@rustbot` modify labels: rollup
bors added a commit to rust-lang-ci/rust that referenced this pull request Jun 14, 2024
@bors
Auto merge of rust-lang#126486 - matthiaskrgr:rollup-yrcxb34, r=matth…
e2f0eac 
…iaskrgr

Rollup of 8 pull requests

Successful merges:

 - rust-lang#125722 (Indicate in `non_local_defs` lint that the macro needs to change)
 - rust-lang#125829 (rustc_span: Add conveniences for working with span formats)
 - rust-lang#126192 (Various Redox OS fixes and add i686 Redox OS target)
 - rust-lang#126352 (ci: Update centos:7 to use vault repos)
 - rust-lang#126354 (Use `Variance` glob imported variants everywhere)
 - rust-lang#126469 (MIR Shl/Shr: the offset can be computed with rem_euclid)
 - rust-lang#126472 (build `libcxx-version` only when it doesn't exist)
 - rust-lang#126476 (Fix running bootstrap tests with a local Rust toolchain as the stage0)

r? `@ghost`
`@rustbot` modify labels: rollup
matthiaskrgr added a commit to matthiaskrgr/rust that referenced this pull request Jun 15, 2024
@matthiaskrgr
Rollup merge of rust-lang#125722 - Urgau:non_local_defs-macro-to-chan…
f817f03 
…ge, r=estebank

Indicate in `non_local_defs` lint that the macro needs to change

This PR adds a note to indicate that the macro needs to change in the `non_local_definitions` lint output.

Address rust-lang#125089 (comment)
Fixes rust-lang#125681
r? ``````@estebank``````
@matthiaskrgr matthiaskrgr mentioned this pull request Jun 15, 2024
Closed
bors added a commit to rust-lang-ci/rust that referenced this pull request Jun 15, 2024
@bors
Auto merge of rust-lang#126503 - matthiaskrgr:rollup-nwvz24q, r=matth…
75f541e 
…iaskrgr

Rollup of 7 pull requests

Successful merges:

 - rust-lang#125722 (Indicate in `non_local_defs` lint that the macro needs to change)
 - rust-lang#125829 (rustc_span: Add conveniences for working with span formats)
 - rust-lang#126192 (Various Redox OS fixes and add i686 Redox OS target)
 - rust-lang#126352 (ci: Update centos:7 to use vault repos)
 - rust-lang#126354 (Use `Variance` glob imported variants everywhere)
 - rust-lang#126469 (MIR Shl/Shr: the offset can be computed with rem_euclid)
 - rust-lang#126472 (build `libcxx-version` only when it doesn't exist)

r? `@ghost`
`@rustbot` modify labels: rollup
@bors
Copy link
Contributor

bors commented Jun 15, 2024

⌛ Testing commit b5d4867 with merge 1d1356d...

@bors
Copy link
Contributor

bors commented Jun 15, 2024

☀️ Test successful - checks-actions
Approved by: estebank
Pushing 1d1356d to master...

@bors bors added the merged-by-bors This PR was explicitly merged by bors. label Jun 15, 2024
@bors bors merged commit 1d1356d into rust-lang:master Jun 15, 2024
7 checks passed
@rustbot rustbot added this to the 1.81.0 milestone Jun 15, 2024
@Urgau Urgau deleted the non_local_defs-macro-to-change branch June 15, 2024 11:09
@rust-timer
Copy link
Collaborator

Finished benchmarking commit (1d1356d): comparison URL.

Overall result: ❌ regressions - ACTION NEEDED

Next Steps: If you can justify the regressions found in this perf run, please indicate this with @rustbot label: +perf-regression-triaged along with sufficient written justification. If you cannot justify the regressions please open an issue or create a new PR that fixes the regressions, add a comment linking to the newly created issue or PR, and then add the perf-regression-triaged label to this PR.

@rustbot label: +perf-regression
cc @rust-lang/wg-compiler-performance

Instruction count

This is a highly reliable metric that was used to determine the overall result at the top of this comment.

mean range count
Regressions ❌
(primary)		 0.2% [0.2%, 0.2%] 6
Regressions ❌
(secondary)		 - - 0
Improvements ✅
(primary)		 - - 0
Improvements ✅
(secondary)		 - - 0
All ❌✅ (primary) 0.2% [0.2%, 0.2%] 6

Max RSS (memory usage)

This benchmark run did not return any relevant results for this metric.

Cycles

Results (secondary -6.6%)

This is a less reliable metric that may be of interest but was not used to determine the overall result at the top of this comment.

mean range count
Regressions ❌
(primary)		 - - 0
Regressions ❌
(secondary)		 - - 0
Improvements ✅
(primary)		 - - 0
Improvements ✅
(secondary)		 -6.6% [-7.1%, -5.8%] 3
All ❌✅ (primary) - - 0

Binary size

This benchmark run did not return any relevant results for this metric.

Bootstrap: 671.15s -> 672.08s (0.14%)
Artifact size: 319.77 MiB -> 319.79 MiB (0.01%)

@rustbot rustbot added the perf-regression Performance regression. label Jun 15, 2024
@Urgau
Copy link
Member Author

Urgau commented Jun 15, 2024

Only the diesel benchmark is affected, which is somewhat expected as it triggers the lint many many times (over 150 times). No other benchmarks are affected, which is expected as they don't trigger the lint. Given the very small regression compared to the number of times the lint is triggered and the fact that we ruled in the past that this is okay. I'm marking this as triaged.

@rustbot label: +perf-regression-triaged

@rustbot rustbot added the perf-regression-triaged The performance regression has been triaged. label Jun 15, 2024
@Urgau Urgau mentioned this pull request Sep 24, 2024
Merged
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@compiler-errors compiler-errors compiler-errors left review comments

Assignees

@estebank estebank

Labels
L-non_local_definitions Lint: non_local_definitions merged-by-bors This PR was explicitly merged by bors. perf-regression Performance regression. perf-regression-triaged The performance regression has been triaged. S-waiting-on-bors Status: Waiting on bors to run and complete tests. Bors will change the label on completion. T-compiler Relevant to the compiler team, which will review and decide on the PR/issue.
Projects
None yet
Milestone
1.81.0
Development

Successfully merging this pull request may close these issues.

non_local_definitions triggers on macro-generated code
6 participants
@Urgau @estebank @bors @rust-timer @compiler-errors @rustbot