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

Add detection of [Partial]Ord methods in the ambiguous_wide_pointer_comparisons lint #121268

Merged
merged 2 commits into from
Mar 29, 2024

Conversation

Urgau
Copy link
Member

@Urgau Urgau commented Feb 18, 2024

Partially addresses #121264 by adding diagnostics items for PartialOrd and Ord methods, detecting such diagnostics items as "binary operation" and suggesting the correct replacement.

I also took the opportunity to change the suggestion to use new methods .cast() on *mut T an d *const T.

@rustbot
Copy link
Collaborator

rustbot commented Feb 18, 2024

r? @estebank

rustbot has assigned @estebank.
They will have a look at your PR within the next two weeks and either review your PR or reassign to another reviewer.

Use r? to explicitly pick a reviewer

@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. T-libs Relevant to the library team, which will review and decide on the PR/issue. labels Feb 18, 2024
@bors
Copy link
Contributor

bors commented Feb 18, 2024

☔ The latest upstream changes (presumably #119673) made this pull request unmergeable. Please resolve the merge conflicts.

@Urgau Urgau force-pushed the improve_ambi_wide_ptr_cmps branch from 5a613c9 to 6ebca3c Compare February 19, 2024 07:01
@Urgau
Copy link
Member Author

Urgau commented Mar 11, 2024

@estebank ping.

@Urgau
Copy link
Member Author

Urgau commented Mar 15, 2024

r? compiler

@rustbot rustbot assigned cjgillot and unassigned estebank Mar 15, 2024
@Urgau
Copy link
Member Author

Urgau commented Mar 26, 2024

r? @Nadrieril (as discussed privately)

@rustbot rustbot assigned Nadrieril and unassigned cjgillot Mar 26, 2024
compiler/rustc_lint/src/types.rs Outdated Show resolved Hide resolved
compiler/rustc_lint/src/types.rs Outdated Show resolved Hide resolved
@rustbot rustbot added S-waiting-on-author Status: This is awaiting some action (such as code changes or more information) from the author. and removed S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. labels Mar 29, 2024
@Urgau Urgau force-pushed the improve_ambi_wide_ptr_cmps branch from 6ebca3c to d4b514f Compare March 29, 2024 15:37
@Urgau
Copy link
Member Author

Urgau commented Mar 29, 2024

I've addressed all the review comments and CI passes; this is ready.

@rustbot ready

@rustbot rustbot added S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. and removed S-waiting-on-author Status: This is awaiting some action (such as code changes or more information) from the author. labels Mar 29, 2024
@Nadrieril
Copy link
Member

Thanks!

@bors r+

@bors
Copy link
Contributor

bors commented Mar 29, 2024

📌 Commit d4b514f has been approved by Nadrieril

It is now in the queue for this repository.

@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 Mar 29, 2024
@bors
Copy link
Contributor

bors commented Mar 29, 2024

⌛ Testing commit d4b514f with merge af4a5a1...

@bors
Copy link
Contributor

bors commented Mar 29, 2024

☀️ Test successful - checks-actions
Approved by: Nadrieril
Pushing af4a5a1 to master...

@bors bors added the merged-by-bors This PR was explicitly merged by bors. label Mar 29, 2024
@bors bors merged commit af4a5a1 into rust-lang:master Mar 29, 2024
12 checks passed
@rustbot rustbot added this to the 1.79.0 milestone Mar 29, 2024
@rust-timer
Copy link
Collaborator

Finished benchmarking commit (af4a5a1): comparison URL.

Overall result: ❌✅ regressions and improvements - 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
Regressions ❌
(secondary)
2.2% [2.2%, 2.2%] 1
Improvements ✅
(primary)
-0.5% [-0.7%, -0.3%] 2
Improvements ✅
(secondary)
-1.4% [-3.5%, -0.2%] 3
All ❌✅ (primary) -0.5% [-0.7%, -0.3%] 2

Max RSS (memory usage)

Results

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)
2.3% [2.3%, 2.3%] 1
Improvements ✅
(primary)
- - 0
Improvements ✅
(secondary)
- - 0
All ❌✅ (primary) - - 0

Cycles

Results

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)
3.4% [2.3%, 4.2%] 3
Improvements ✅
(primary)
- - 0
Improvements ✅
(secondary)
- - 0
All ❌✅ (primary) - - 0

Binary size

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

Bootstrap: 669.534s -> 667.985s (-0.23%)
Artifact size: 315.85 MiB -> 315.84 MiB (-0.00%)

@rustbot rustbot added the perf-regression Performance regression. label Mar 29, 2024
@Nadrieril
Copy link
Member

One secondary regression on incr alongside some improvements even on the same bench, nothing to worry about

@Nadrieril Nadrieril added the perf-regression-triaged The performance regression has been triaged. label Mar 29, 2024
bors added a commit to rust-lang-ci/rust that referenced this pull request Mar 30, 2024
…eril

Add support for `NonNull`s in the `ambiguous_wide_ptr_comparisions` lint

This PR add support for `NonNull` pointers in the `ambiguous_wide_ptr_comparisions` lint.

Fixes rust-lang#121264
r? `@Nadrieril` (since you just reviewed rust-lang#121268, feel free to reassign)
bors added a commit to rust-lang-ci/rust that referenced this pull request Mar 30, 2024
…eril

Add support for `NonNull`s in the `ambiguous_wide_ptr_comparisions` lint

This PR add support for `NonNull` pointers in the `ambiguous_wide_ptr_comparisions` lint.

Fixes rust-lang#121264
r? `@Nadrieril` (since you just reviewed rust-lang#121268, feel free to reassign)
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
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. T-libs Relevant to the library team, which will review and decide on the PR/issue.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

7 participants