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

Remove the box_pointers lint. #126018

Merged
merged 1 commit into from
Jun 30, 2024
Merged

Remove the box_pointers lint. #126018

merged 1 commit into from
Jun 30, 2024

Conversation

nnethercote
Copy link
Contributor

As the comment says, this lint "is mostly historical, and not particularly useful". It's not worth keeping it around.

r? @estebank

@rustbot
Copy link
Collaborator

rustbot commented Jun 5, 2024

rust-analyzer is developed in its own repository. If possible, consider making this change to rust-lang/rust-analyzer instead.

cc @rust-lang/rust-analyzer

@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 Jun 5, 2024
@oli-obk oli-obk added the needs-fcp This change is insta-stable, so needs a completed FCP to proceed. label Jun 5, 2024
lcnr
lcnr reviewed Jun 5, 2024
View reviewed changes
Copy link
Contributor

@lcnr lcnr left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

there's a list of removed/renamed lints somewhere to adjust the error message when using a renamed lint. Please add it there

r=me on the impl changes, please start a compiler FCP for this

@lcnr
Copy link
Contributor

lcnr commented Jun 5, 2024

r? lcnr

@rustbot rustbot assigned lcnr and unassigned estebank Jun 5, 2024
@nnethercote
Copy link
Contributor Author

I have added a call to register_removed.

@rust-log-analyzer

This comment has been minimized.

Sign in to view
@nnethercote
Copy link
Contributor Author

Removing this old, useless lint that tells you if you are using Box. (Yes, really. Perhaps it made more sense in pre-1.0 days.)

@rfcbot fcp merge

@nnethercote
Copy link
Contributor Author

How do I start an FCP? Or does it have to be started by a compiler team member? I'm only a contributor.

@lcnr
Copy link
Contributor

lcnr commented Jun 5, 2024

@rfcbot fcp merge

lcnr
lcnr reviewed Jun 5, 2024
View reviewed changes
compiler/rustc_lint/src/lib.rs Outdated Show resolved Hide resolved
@lcnr
Copy link
Contributor

lcnr commented Jun 5, 2024

@rfcbot ping

@lcnr lcnr closed this Jun 5, 2024
@lcnr lcnr reopened this Jun 5, 2024
@lcnr
Copy link
Contributor

lcnr commented Jun 5, 2024

@rfcbot fcp merge

@rust-log-analyzer

This comment has been minimized.

Sign in to view
@nnethercote
Copy link
Contributor Author

Does anyone know how to trigger an FCP?

@kpreid
Copy link
Contributor

kpreid commented Jun 6, 2024 •
edited
Loading

rfcbot was having some trouble, which has been fixed-for-now, and it sounds like you should be able to just retry.

@nnethercote
Copy link
Contributor Author

@rfcbot fcp

@nnethercote
Copy link
Contributor Author

@rfcbot fcp merge

1 similar comment
@lcnr
Copy link
Contributor

lcnr commented Jun 6, 2024

@rfcbot fcp merge

@nnethercote
Copy link
Contributor Author

I rebased.

@scottmcm
Copy link
Member

(Lang hat on; not speaking for the team) This makes good sense to me. A restriction lint like this isn't something I think we'd accept for rustc these days. So 👍 to removing it, assuming the correct incantation happened so peoples builds don't break if they were (inexplicably) mentioning it (which I think has already been done in the PR).

@nnethercote
Copy link
Contributor Author

So 👍 to removing it, assuming the correct incantation happened so peoples builds don't break if they were (inexplicably) mentioning it (which I think has already been done in the PR).

Yep, the register_removed call takes care of that -- people will get a warning about the lint having been removed.

@est31
Copy link
Member

est31 commented Jun 12, 2024

Even without register_removed, nonexistent lints only trigger unknown_lints, which is a warning lint of its own. so it's affected by cargo's lint capping etc.

@nnethercote
Copy link
Contributor Author

Only one approval short of the n-2 threshold!

@rfcbot rfcbot added final-comment-period In the final comment period and will be merged soon unless new substantive objections are raised. and removed proposed-final-comment-period Proposed to merge/close by relevant subteam, see T-<team> label. Will enter FCP once signed off. labels Jun 19, 2024
@rfcbot
Copy link

rfcbot commented Jun 19, 2024

🔔 This is now entering its final comment period, as per the review above. 🔔

@bors
Copy link
Contributor

bors commented Jun 26, 2024

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

@nnethercote
Remove the box_pointers lint.
c053e89 
As the comment says, this lint "is mostly historical, and not
particularly useful". It's not worth keeping it around.
@rfcbot rfcbot added finished-final-comment-period The final comment period is finished for this PR / Issue. and removed final-comment-period In the final comment period and will be merged soon unless new substantive objections are raised. labels Jun 29, 2024
@rfcbot
Copy link

rfcbot commented Jun 29, 2024

The final comment period, with a disposition to merge, as per the review above, is now complete.

As the automated representative of the governance process, I would like to thank the author for their work and everyone else who contributed.

This will be merged soon.

@rfcbot rfcbot added the to-announce Announce this issue on triage meeting label Jun 29, 2024
@nnethercote
Copy link
Contributor Author

@bors r=lcnr

@bors
Copy link
Contributor

bors commented Jun 30, 2024

📌 Commit c053e89 has been approved by lcnr

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 Jun 30, 2024
matthiaskrgr added a commit to matthiaskrgr/rust that referenced this pull request Jun 30, 2024
@matthiaskrgr
Rollup merge of rust-lang#126018 - nnethercote:rm-box_pointers-lint, …
fb021c1 
…r=lcnr

Remove the `box_pointers` lint.

As the comment says, this lint "is mostly historical, and not particularly useful". It's not worth keeping it around.

r? `@estebank`
@matthiaskrgr matthiaskrgr mentioned this pull request Jun 30, 2024
Closed
bors added a commit to rust-lang-ci/rust that referenced this pull request Jun 30, 2024
@bors
Auto merge of rust-lang#127169 - matthiaskrgr:rollup-18hpjuh, r=matth…
4196674 
…iaskrgr

Rollup of 8 pull requests

Successful merges:

 - rust-lang#126018 (Remove the `box_pointers` lint.)
 - rust-lang#126895 (Fix simd_gather documentation)
 - rust-lang#126981 (Replace some magic booleans in match-lowering with enums)
 - rust-lang#127069 (small correction to fmt::Pointer impl)
 - rust-lang#127157 (coverage: Avoid getting extra unexpansion info when we don't need it)
 - rust-lang#127160 (Add a regression test for rust-lang#123630)
 - rust-lang#127161 (Improve `run-make-support` library `args` API)
 - rust-lang#127162 (Subtree sync for rustc_codegen_cranelift)

r? `@ghost`
`@rustbot` modify labels: rollup
@matthiaskrgr matthiaskrgr mentioned this pull request Jun 30, 2024
Merged
bors added a commit to rust-lang-ci/rust that referenced this pull request Jun 30, 2024
@bors
Auto merge of rust-lang#127174 - matthiaskrgr:rollup-q87j6cn, r=matth…
804746e 
…iaskrgr

Rollup of 9 pull requests

Successful merges:

 - rust-lang#126018 (Remove the `box_pointers` lint.)
 - rust-lang#126895 (Fix simd_gather documentation)
 - rust-lang#126981 (Replace some magic booleans in match-lowering with enums)
 - rust-lang#127038 (Update test comment)
 - rust-lang#127053 (Update the LoongArch target documentation)
 - rust-lang#127069 (small correction to fmt::Pointer impl)
 - rust-lang#127157 (coverage: Avoid getting extra unexpansion info when we don't need it)
 - rust-lang#127160 (Add a regression test for rust-lang#123630)
 - rust-lang#127161 (Improve `run-make-support` library `args` API)

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

Rollup of 9 pull requests

Successful merges:

 - rust-lang#126018 (Remove the `box_pointers` lint.)
 - rust-lang#126895 (Fix simd_gather documentation)
 - rust-lang#126981 (Replace some magic booleans in match-lowering with enums)
 - rust-lang#127038 (Update test comment)
 - rust-lang#127053 (Update the LoongArch target documentation)
 - rust-lang#127069 (small correction to fmt::Pointer impl)
 - rust-lang#127157 (coverage: Avoid getting extra unexpansion info when we don't need it)
 - rust-lang#127160 (Add a regression test for rust-lang#123630)
 - rust-lang#127161 (Improve `run-make-support` library `args` API)

r? `@ghost`
`@rustbot` modify labels: rollup
@bors bors merged commit 6f57655 into rust-lang:master Jun 30, 2024
6 checks passed
@rustbot rustbot added this to the 1.81.0 milestone Jun 30, 2024
rust-timer added a commit to rust-lang-ci/rust that referenced this pull request Jun 30, 2024
@rust-timer
Unrolled build for rust-lang#126018
a9c0b1b 
Rollup merge of rust-lang#126018 - nnethercote:rm-box_pointers-lint, r=lcnr

Remove the `box_pointers` lint.

As the comment says, this lint "is mostly historical, and not particularly useful". It's not worth keeping it around.

r? ``@estebank``
@nnethercote nnethercote deleted the rm-box_pointers-lint branch June 30, 2024 23:04
@apiraino apiraino removed the to-announce Announce this issue on triage meeting label Jul 4, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@lcnr lcnr lcnr left review comments

@estebank estebank estebank approved these changes

@est31 est31 est31 approved these changes

Assignees

@lcnr lcnr

Labels
disposition-merge This issue / PR is in PFCP or FCP with a disposition to merge it. finished-final-comment-period The final comment period is finished for this PR / Issue. needs-fcp This change is insta-stable, so needs a completed FCP to proceed. 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.

None yet