Skip to content
New issue

Have a question about this project? # for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “#”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? # to your account

Turn order dependent trait objects future incompat warning into a hard error #136968

Open
wants to merge 1 commit into
base: master
Choose a base branch
from

Conversation

oli-obk
Copy link
Contributor

@oli-obk oli-obk commented Feb 13, 2025

fixes #56484

r? @ghost

will FCP when we have a crater result

@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 Feb 13, 2025
@oli-obk
Copy link
Contributor Author

oli-obk commented Feb 13, 2025

@bors try

bors added a commit to rust-lang-ci/rust that referenced this pull request Feb 13, 2025
Turn order dependent trait objects future incompat warning into a hard error

fixes rust-lang#56484

r? `@ghost`

will FCP when we have a crater result
@bors
Copy link
Contributor

bors commented Feb 13, 2025

⌛ Trying commit 0ae0780 with merge 9d63f82...

@bors
Copy link
Contributor

bors commented Feb 13, 2025

☀️ Try build successful - checks-actions
Build commit: 9d63f82 (9d63f825c0d409b0e1743a2c84b33f86965e5929)

@oli-obk
Copy link
Contributor Author

oli-obk commented Feb 13, 2025

@craterbot check

@craterbot
Copy link
Collaborator

👌 Experiment pr-136968 created and queued.
🤖 Automatically detected try build 9d63f82
🔍 You can check out the queue and this experiment's details.

ℹ️ Crater is a tool to run experiments across parts of the Rust ecosystem. Learn more

@craterbot craterbot added S-waiting-on-crater Status: Waiting on a crater run to be completed. and removed S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. labels Feb 13, 2025
@craterbot
Copy link
Collaborator

🚧 Experiment pr-136968 is now running

ℹ️ Crater is a tool to run experiments across parts of the Rust ecosystem. Learn more

@craterbot
Copy link
Collaborator

🎉 Experiment pr-136968 is completed!
📊 506 regressed and 4 fixed (582562 total)
📰 Open the full report.

⚠️ If you notice any spurious failure please add them to the denylist!
ℹ️ Crater is a tool to run experiments across parts of the Rust ecosystem. Learn more

@craterbot craterbot added S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. and removed S-waiting-on-crater Status: Waiting on a crater run to be completed. labels Feb 14, 2025
@oli-obk
Copy link
Contributor Author

oli-obk commented Feb 14, 2025

🤔 hmm all the failures are because of crates with lockfiles for traitobject 0.1.0, even though traitobject 0.1.1 resolves the issues.

@oli-obk oli-obk marked this pull request as ready for review February 25, 2025 09:00
@oli-obk oli-obk added T-types Relevant to the types team, which will review and decide on the PR/issue. and removed T-compiler Relevant to the compiler team, which will review and decide on the PR/issue. labels Feb 25, 2025
@oli-obk
Copy link
Contributor Author

oli-obk commented Feb 25, 2025

cc @rust-lang/lang

We've had a future incompatibility warning for years for code like

trait Trait {}
impl Trait for dyn Send + Sync {}
impl Trait for dyn Sync + Send {}

We already hard error if the trait has methods, but due to some widespread breakage, we decided not to make it a hard error. All (yes all) of the breakage is due to crates depending (mostly transitively) on the traitobject. @reem recently gave me crates.io ownership and I published a patch update, so everyone can just cargo update -p traitobject without causing any other problems.

There are only 10 crates on crates.io that break from this PR, and all of them also were due to lockfiles. All but

haven't seen updates in 4 years and are usually depending on an old version of hyper that still had traitobject in its dependency graph at all.

We should turn this FCW into a hard error and thus be able to rip out all the code that was necessary to maintain it

@rfcbot merge

@rfcbot
Copy link

rfcbot commented Feb 25, 2025

Team member @oli-obk has proposed to merge this. The next step is review by the rest of the tagged team members:

No concerns currently listed.

Once a majority of reviewers approve (and at most 2 approvals are outstanding), this will enter its final comment period. If you spot a major issue that hasn't been raised at any point in this process, please speak up!

See this document for info about what commands tagged team members can give me.

@rfcbot rfcbot added proposed-final-comment-period Proposed to merge/close by relevant subteam, see T-<team> label. Will enter FCP once signed off. disposition-merge This issue / PR is in PFCP or FCP with a disposition to merge it. labels Feb 25, 2025
@oli-obk
Copy link
Contributor Author

oli-obk commented Feb 25, 2025

r? types

# for free to join this conversation on GitHub. Already have an account? # to comment
Labels
disposition-merge This issue / PR is in PFCP or FCP with a disposition to merge it. proposed-final-comment-period Proposed to merge/close by relevant subteam, see T-<team> label. Will enter FCP once signed off. S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. T-types Relevant to the types team, which will review and decide on the PR/issue.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Tracking issue for future-incompatbility lint order_dependent_trait_objects
6 participants