Implement an internal lint encouraging use of Span::eq_ctxt
#116787
Merged
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
rustbot
added
S-waiting-on-review
|
Some changes occurred to MIR optimizations cc @rust-lang/wg-mir-opt |
This comment has been minimized.
This comment has been minimized.
|
impl lgtm (just needs to fix the r? @compiler-errors do you remember details? |
|
PR to fix the lint violation in Clippy is up: rust-lang/rust-clippy#11679 |
|
There's nothing technically wrong with using |
Oh 😅 you could've just done it in this PR. |
|
Yeah 😅 I found that out in the Clippy PR. Good to know though, I'll update this PR with those changes. Edit: updated |
|
Some changes occurred in src/tools/clippy cc @rust-lang/clippy |
|
☔ The latest upstream changes (presumably #116820) made this pull request unmergeable. Please resolve the merge conflicts. |
a-lafrance
force-pushed
the
span-internal-lint
branch
from
faacc3b to
e89d4d4
Compare
|
@bors r+ rollup |
bors
added
S-waiting-on-bors
matthiaskrgr
added a commit
to matthiaskrgr/rust
that referenced
this pull request
Oct 17, 2023
…li-obk Implement an internal lint encouraging use of `Span::eq_ctxt` Adds a new Rustc internal lint that forbids use of `.ctxt() == .ctxt()` for spans, encouraging use of `.eq_ctxt()` instead (see rust-lang#49509). Also fixed a few violations of the lint in the Rustc codebase (a fun additional way I could test my code). Edit: MIR opt folks I believe that's why you're CC'ed, just a heads up. Two things I'm not sure about: 1. The way I chose to detect calls to `Span::ctxt`. I know adding diagnostic items to methods is generally discouraged, but after some searching and experimenting I couldn't find anything else that worked, so I went with it. That said, I'm happy to implement something different if there's a better way out there. (For what it's worth, if there is a better way, it might be worth documenting in the rustc-dev-guide, which I'm happy to take care of) 2. The error message for the lint. Ideally it would probably be good to give some context as to why the suggestion is made (e.g. `rustc::default_hash_types` tells the user that it's because of performance), but I don't have that context so I couldn't put it in the error message. Happy to iterate on the error message based on feedback during review. r? `@oli-obk`
bors
added a commit
to rust-lang-ci/rust
that referenced
this pull request
Oct 17, 2023
…iaskrgr Rollup of 5 pull requests Successful merges: - rust-lang#116717 (Special case iterator chain checks for suggestion) - rust-lang#116719 (Add MonoItems and Instance to stable_mir) - rust-lang#116787 (Implement an internal lint encouraging use of `Span::eq_ctxt`) - rust-lang#116827 (Make `handle_options` public again.) - rust-lang#116838 (Fix duplicate labels emitted in `render_multispan_macro_backtrace()`) r? `@ghost` `@rustbot` modify labels: rollup
bors
added a commit
to rust-lang-ci/rust
that referenced
this pull request
Oct 17, 2023
…iaskrgr Rollup of 5 pull requests Successful merges: - rust-lang#111072 (Add new simpler and more explicit syntax for check-cfg) - rust-lang#116717 (Special case iterator chain checks for suggestion) - rust-lang#116719 (Add MonoItems and Instance to stable_mir) - rust-lang#116787 (Implement an internal lint encouraging use of `Span::eq_ctxt`) - rust-lang#116827 (Make `handle_options` public again.) r? `@ghost` `@rustbot` modify labels: rollup
rust-timer
added a commit
to rust-lang-ci/rust
that referenced
this pull request
Oct 17, 2023
Rollup merge of rust-lang#116787 - a-lafrance:span-internal-lint, r=oli-obk Implement an internal lint encouraging use of `Span::eq_ctxt` Adds a new Rustc internal lint that forbids use of `.ctxt() == .ctxt()` for spans, encouraging use of `.eq_ctxt()` instead (see rust-lang#49509). Also fixed a few violations of the lint in the Rustc codebase (a fun additional way I could test my code). Edit: MIR opt folks I believe that's why you're CC'ed, just a heads up. Two things I'm not sure about: 1. The way I chose to detect calls to `Span::ctxt`. I know adding diagnostic items to methods is generally discouraged, but after some searching and experimenting I couldn't find anything else that worked, so I went with it. That said, I'm happy to implement something different if there's a better way out there. (For what it's worth, if there is a better way, it might be worth documenting in the rustc-dev-guide, which I'm happy to take care of) 2. The error message for the lint. Ideally it would probably be good to give some context as to why the suggestion is made (e.g. `rustc::default_hash_types` tells the user that it's because of performance), but I don't have that context so I couldn't put it in the error message. Happy to iterate on the error message based on feedback during review. r? ``@oli-obk``
bors-ferrocene bot
added a commit
to ferrocene/ferrocene
that referenced
this pull request
Oct 18, 2023
57: Pull upstream master 2023 10 18 r=pietroalbini a=Veykril * rust-lang/rust#116505 * rust-lang/rust#116840 * rust-lang/rust#116767 * rust-lang/rust#116855 * rust-lang/rust#116827 * rust-lang/rust#116787 * rust-lang/rust#116719 * rust-lang/rust#116717 * rust-lang/rust#111072 * rust-lang/rust#116844 * rust-lang/rust#115577 * rust-lang/rust#116756 * rust-lang/rust#116518 Co-authored-by: Urgau <urgau@numericable.fr> Co-authored-by: Esteban Küber <esteban@kuber.com.ar> Co-authored-by: Deadbeef <ent3rm4n@gmail.com> Co-authored-by: Ralf Jung <post@ralfj.de> Co-authored-by: Camille GILLOT <gillot.camille@gmail.com> Co-authored-by: Celina G. Val <celinval@amazon.com> Co-authored-by: Nicholas Nethercote <n.nethercote@gmail.com> Co-authored-by: Arthur Lafrance <lafrancearthur@gmail.com> Co-authored-by: Nikolay Arhipov <n@arhipov.net> Co-authored-by: Nikita Popov <npopov@redhat.com> Co-authored-by: bors <bors@rust-lang.org>
12 tasks
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Adds a new Rustc internal lint that forbids use of
.ctxt() == .ctxt()for spans, encouraging use of.eq_ctxt()instead (see #49509).Also fixed a few violations of the lint in the Rustc codebase (a fun additional way I could test my code). Edit: MIR opt folks I believe that's why you're CC'ed, just a heads up.
Two things I'm not sure about:
Span::ctxt. I know adding diagnostic items to methods is generally discouraged, but after some searching and experimenting I couldn't find anything else that worked, so I went with it. That said, I'm happy to implement something different if there's a better way out there. (For what it's worth, if there is a better way, it might be worth documenting in the rustc-dev-guide, which I'm happy to take care of)rustc::default_hash_typestells the user that it's because of performance), but I don't have that context so I couldn't put it in the error message. Happy to iterate on the error message based on feedback during review.r? @oli-obk