Bump IMPLIED_BOUNDS_ENTAILMENT to Deny + ReportNow

[compiler-errors]

#105575 (comment)

and then later in the same cycle increase the lint to deny and change it to FutureCompatReportNow in this nightly cycle.

r? @lcnr when they're back from holiday 😄

[jackh726]

Agh, I think I missed the exact wording in the other PR.

I think we should change the lint to ReportNow at the end of this cycle, but I'm wary of bumping to deny.

Regardless, if (/when) we do bump to deny, we need to run crater to assess the impact.

[lcnr]

still have 10 days until the next beta branch. I believe that this PR is the correct approach here as even if we still have the same impact as in #105483 (comment) we should quickly move this towards a hard error.

I do feel like it should be possible to get more accurate spans for the lint though 😅 it would be awesome if we're able to print a signature with the correct types '^^ not sure how easy that is though :/

[jackh726]

@bors try

[bors]

⌛ Trying commit 7369c52cf1bc76f73f3f9643144030271cf57056 with merge 3ac4ab8283c0bed583d0a33ab92c6a77c486aa98...

[bors]

☀️ Try build successful - checks-actions
Build commit: 3ac4ab8283c0bed583d0a33ab92c6a77c486aa98 (3ac4ab8283c0bed583d0a33ab92c6a77c486aa98)

[jackh726]

@craterbot check

[craterbot]

👌 Experiment pr-106465 created and queued.
🤖 Automatically detected try build 3ac4ab8283c0bed583d0a33ab92c6a77c486aa98
🔍 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]

🚧 Experiment pr-106465 is now running

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

[lcnr]

I've talked about this PR with @jackh726 yesterday and our thoughts were that to land this we have to improve the diagnostic to be more actionable for users.

I think the easiest change would be to look at the errors in CheckImpliedWfMode and get back the method argument which was responsible for the error. Then point at that argument in the error (you could emit this lint separately for each argument involved) and also print the "expected argument", i.e. the argument you would get by substituting the argument of the trait method with the impl substs.

We should also file PRs to the affected crates where possible. Going to do that myself rn.

[lqd]

It'd then also be great to release a fixed version of rustc-serialize, somehow notify users (with a rustsec advisory maybe) that a simple cargo update would fix the future-incompat report they'll be encountering, etc.

Fixing the method signature applies cleanly, and apart from logistics about un-archiving the repo and a crates.io release, this all sounds very doable and I'd like to help if t-types agrees we should do this.

[lcnr]

• imxrt-dma fix implied_bounds_entailment lint imxrt-rs/imxrt-dma#2 fixed
• schema_analysis fix the implied_bounds_entailment future compatibility lint. QuartzLibrary/schema_analysis#5 we actually rely on the implied bounds here, so that impl was unsound.
• syn already fixed in the most recent version
• biogarden fix implied_bounds_entailment future compatibility lint robsndr/biogarden#1 not unsound
• gdrust fix implied_bounds_entailment future compatibility lint wyattjsmith1/gdrust#32 not unsound
• castle_game fix implied_bounds_entailment future compat lint JunkuiZhang/castle_game#1 I think this would be exploitable for unsoundness
• assertor fix implied_bound_entailment future compat lint google/assertor#1 can't be bothered to deal with the google contributor license agreement though 😁
• spair fix implied_bounds_entailment future compat lint aclueless/spair#14 that one's hard :/ I don't really know how to change that right now. Have to get back to that later. Sadly the current impl relies on unsoundness by extending a lifetime in the return type. edit: thanks @BoxyUwU for implementing the fix :3

[lcnr]

It'd then also be great to release a fixed version of rustc-serialize, somehow notify users (with a rustsec advisory maybe) that a simple cargo update would fix the future-incompat report they'll be encountering, etc.

Fixing the method signature applies cleanly, and apart from logistics about un-archiving the repo and a crates.io release, this all sounds very doable and I'd like to help if t-types agrees we should do this.

yes, but also. Having a future compat report now lint for something we've stopped maintaining for 6 years now might be a good way to get people to finally switch to a newer crate 😁

we should definitely do that before making this lint a hard error though ^^

[craterbot]

🎉 Experiment pr-106465 is completed!
📊 16 regressed and 20 fixed (252288 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

[compiler-errors]

I pushed some logic to make the spans better, + a best effort suggestion for what to replace a type with.

[lcnr]

that's wrong in subtle ways because impls aren't required to order their bound vars in the same way. That seems fine considering that this lint will only exist for a few releases and works correctly for all the issues we've found via crater.

[lcnr]

nice

@bors r+ rollup

[bors]

📌 Commit eaa7cc8 has been approved by lcnr

It is now in the queue for this repository.