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

Fix failure of new internal lint rustc::span_use_eq_ctxt #11679

Closed

Conversation

a-lafrance
Copy link
Contributor

I've implemented a new Rustc internal lint that forbids usage of .ctxt() == .ctxt() for spans, and suggests using Span::eq_ctxt directly instead (see rust-lang/rust#49509 and rust-lang/rust#116787).

Unfortunately the PR can't land yet because there's a violation of the lint in the Clippy codebase which (as it should) is causing CI failures, so this PR fixes that lint preemptively to unblock it from landing.

changelog: none

@rustbot
Copy link
Collaborator

rustbot commented Oct 16, 2023

Thanks for the pull request, and welcome! The Rust team is excited to review your changes, and you should hear from @llogiq (or someone else) soon.

Please see the contribution instructions for more information. Namely, in order to ensure the minimum review times lag, PR authors and assigned reviewers should ensure that the review label (S-waiting-on-review and S-waiting-on-author) stays updated, invoking these commands when appropriate:

  • @rustbot author: the review is finished, PR author should check the comments and take action accordingly
  • @rustbot review: the author is ready for a review, this PR will be queued again in the reviewer's queue

@rustbot rustbot added the S-waiting-on-review Status: Awaiting review from the assignee but also interested parties label Oct 16, 2023
@a-lafrance a-lafrance changed the title Fix failures of new internal lint rustc::span_use_eq_ctxt Fix failure of new internal lint rustc::span_use_eq_ctxt Oct 16, 2023
@flip1995
Copy link
Member

You can just include those changes in your PR towards rust-lang/rust. Otherwise you'll have to wait until Thursday, when I sync this PR to the Rust repo.

r? @flip1995

So the changes LGTM, but I'm holding off with merging, as I think that it's easier for everyone if you just apply those changes in the Rust repo (src/tools/clippy)

@rustbot rustbot assigned flip1995 and unassigned llogiq Oct 16, 2023
@a-lafrance
Copy link
Contributor Author

Ah whoops, thanks for the heads up. Didn't know I could update Clippy in-tree, that does seem much smoother.

Closing this now that I've updated the Rustc PR with the Clippy changes.

@a-lafrance a-lafrance closed this Oct 16, 2023
@a-lafrance a-lafrance deleted the span-internal-lint-fix branch October 16, 2023 20:54
@flip1995
Copy link
Member

Didn't know I could update Clippy in-tree,

Yes, this is encouraged for fixing up breaking changes in rustc. But functional changes like lint additions are not allowed in rustc and must be done in the Clippy repo.

@a-lafrance
Copy link
Contributor Author

Good to know, thanks for the info :)

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
S-waiting-on-review Status: Awaiting review from the assignee but also interested parties
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

4 participants