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

make incoherent_auto_trait_objects a hard error #102474

Closed
wants to merge 1 commit into from

Conversation

lcnr
Copy link
Contributor

@lcnr lcnr commented Sep 29, 2022

needs a crater run, this lint exists since nearly 4 years now and the affected crate is not maintained anymore.

The current implementation for the lint will break with #102472, which is fixes caching for the trait system. Keeping this lint working with the current implementation will be non-trivial and imo not worth the cost.

closes #56484

r? types

@rustbot rustbot added the T-compiler Relevant to the compiler team, which will review and decide on the PR/issue. label Sep 29, 2022
@rust-highfive rust-highfive added the S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. label Sep 29, 2022
@lcnr
Copy link
Contributor Author

lcnr commented Sep 29, 2022

@bors try

@bors
Copy link
Contributor

bors commented Sep 29, 2022

⌛ Trying commit 96ae278 with merge 4d7187a26f9eb7e5000e28e28662298d34d2508f...

@bors
Copy link
Contributor

bors commented Sep 29, 2022

☀️ Try build successful - checks-actions
Build commit: 4d7187a26f9eb7e5000e28e28662298d34d2508f (4d7187a26f9eb7e5000e28e28662298d34d2508f)

@lcnr
Copy link
Contributor Author

lcnr commented Sep 29, 2022

@craterbot check

@craterbot
Copy link
Collaborator

👌 Experiment pr-102474 created and queued.
🤖 Automatically detected try build 4d7187a26f9eb7e5000e28e28662298d34d2508f
🔍 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 Sep 29, 2022
@craterbot
Copy link
Collaborator

🚧 Experiment pr-102474 is now running

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

@craterbot
Copy link
Collaborator

🎉 Experiment pr-102474 is completed!
📊 1435 regressed and 5 fixed (244541 total)
📰 Open the full report.

⚠️ If you notice any spurious failure please add them to the blacklist!
ℹ️ 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 Sep 30, 2022
@lcnr
Copy link
Contributor Author

lcnr commented Oct 3, 2022

ok that looks wrong 🤔 cc @rust-lang/infra is that an issue with crater? i would have expected the traitobjects crate to be part of the regressed crates but i can't find it and there seem to be a surprising amount of unrelated failures

@jyn514
Copy link
Member

jyn514 commented Oct 3, 2022

@lcnr the newly failed builds are marked "build OOM"; I don't know the details of this change but it seems plausible to me it could be taking extra memory. They're marked as "spurious regressions"; you can see the root regressions (due to compiler errors) under "root results".

@jyn514
Copy link
Member

jyn514 commented Oct 3, 2022

ah, and traitobjects is under "regressed: dependencies"

@lcnr
Copy link
Contributor Author

lcnr commented Oct 3, 2022

hmm, interesting that it doesn't fail as a root cause 🤔 only looked at the first 3 entries of "regressed: dependencies" which were all spurious '^^ should have maybe looked a bit further down 😁

@lcnr
Copy link
Contributor Author

lcnr commented Oct 3, 2022

intend to add the lint to future-breakage reports for now as there are >1000 regressions: #102635

@lcnr lcnr closed this Oct 3, 2022
notriddle added a commit to notriddle/rust that referenced this pull request Oct 21, 2022
…, r=jackh726

make `order_dependent_trait_objects` show up in future-breakage reports

tried to change it to a hard error in rust-lang#102474 but breaking the more than 1000 dependents of `traitobject` doesn't feel great 😅

This lint has existed since more than 3 years now and the way this is currently implemented is buggy and will break with rust-lang#102472. imo we should upgrade it to also report for dependencies and maybe also backport this to beta. Then after maybe 2-3 stable versions I would like to finally convert this lint to a hard error.
matthiaskrgr added a commit to matthiaskrgr/rust that referenced this pull request Oct 21, 2022
…, r=jackh726

make `order_dependent_trait_objects` show up in future-breakage reports

tried to change it to a hard error in rust-lang#102474 but breaking the more than 1000 dependents of `traitobject` doesn't feel great 😅

This lint has existed since more than 3 years now and the way this is currently implemented is buggy and will break with rust-lang#102472. imo we should upgrade it to also report for dependencies and maybe also backport this to beta. Then after maybe 2-3 stable versions I would like to finally convert this lint to a hard error.
@lcnr lcnr deleted the lint-to-hard-err branch January 19, 2024 14:53
# for free to join this conversation on GitHub. Already have an account? # to comment
Labels
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.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Tracking issue for future-incompatbility lint order_dependent_trait_objects
7 participants