Deny-by-default never type lints

[WaffleLapkin]

In Rust 1.89.0 we started emitting these lints in dependencies. I discussed the future steps with @lcnr and we think that before stabilizing the never type (and doing the breaking changes) we should deny the lints for ~4 releases.

This PR marks never_type_fallback_flowing_into_unsafe and dependency_on_unit_never_type_fallback lints as deny-by-default.

Tracking:

• Tracking issue for promoting ! to a type (RFC 1216) #35121

Related:

• Report never type lints in dependencies #141937

[rustbot]

r? @petrochenkov

rustbot has assigned @petrochenkov.
They will have a look at your PR within the next two weeks and either review your PR or reassign to another reviewer.

Use r? to explicitly pick a reviewer

[WaffleLapkin]

@bors try

[WaffleLapkin]

T-lang nomination

This PR marks never_type_fallback_flowing_into_unsafe and dependency_on_unit_never_type_fallback lints as deny-by-default. I think this is the next step towards never type stabilization.

[rust-bors]

☀️ Try build successful (CI)
Build commit: e90f1ff (e90f1ff63e694965281f833819a486da29772bdf, parent: 51ff895062ba60a7cba53f57af928c3fb7b0f2f4)

[WaffleLapkin]

@craterbot check

[craterbot]

👌 Experiment pr-146167 created and queued.
🤖 Automatically detected try build e90f1ff
🔍 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-146167 is now running

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

[lcnr]

I discussed the future steps with @lcnr and we think that before stabilizing the never type (and doing the breaking changes) we should deny the lints for ~4 releases.

I personally would convert the lint to a hard error for a few versions :> Users can still ignore denied lints (e.g. if it's in a dependency)

[craterbot]

🎉 Experiment pr-146167 is completed!
📊 474 regressed and 3 fixed (693868 total)
📊 1781 spurious results on the retry-regessed-list.txt, consider a retry¹ if this is a significant amount.
📰 Open the summary 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

Footnotes

1. re-run the experiment with crates=https://crater-reports.s3.amazonaws.com/pr-146167/retry-regressed-list.txt ↩

[tmandry]

@rfcbot reviewed

[nikomatsakis]

@rfcbot reviewed

This seems good! Thanks for your continued pushing on this, @WaffleLapkin -- and the crater results look encouraging, seems like mostly minor stuff.

[rust-rfcbot]

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

[scottmcm]

Yay more ! progress!
@rfcbot reviewed

[rust-rfcbot]

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.

[bors]

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

[rustbot]

This PR was rebased onto a different master commit. Here's a range-diff highlighting what actually changed.

Rebasing is a normal part of keeping PRs up to date, so no action is needed—this note is just to help reviewers.

[WaffleLapkin]

@nikomatsakis (and to the language team in general I guess) can you explain your assessment of the crater run?

I don't see how ~470 unique crater breakages can be considered "minor stuff". 470 unique breakages feels like way too much, especially considering how not everything is tested by crater.

I'm really concerned by this, because I'm not sure how we can proceed with the current plan, given that it requires so much breaking changes.

[traviscross]

Thanks for the question. You're right it's a non-trivial number, and clearly stability is important to us all. Some items we consider in our analysis:

One, we're not breaking any code, under our normal definitions for these things, with this PR. Even at deny-by-default, we're only currently linting against this. Someone can allow it or use --cap-lints, for now, to continue compiling the program. While we do consider carefully the effect of all lint level changes on users, we do not treat the behavior of lints as being part of our stability guarantee.

Since it's only a lint, it only stops compilation for the project with the affected code. Dependent crates are not affected except being offered the report about dependencies emitting FCWs.

Since we want people to stop relying on this, this is what we want. We want to ratchet up the lints in a way that is directed at the people whose code will need to change.

Two, we've been issuing this FCW for a long time. After a suitably long interval, when changing a behavior we consider to be part of our underspecified language semantics (see RFC 1122), we have historically considered ourselves "within our rights" (after considering all other factors) to make the ("minor", per RFC 1122) breaking change. This is part of what we'll be considering when it is later proposed for this to become a hard error.

Three, when looking at a crater report for this kind of change where we've been issuing an FCW for a long time, we keep in mind that crater checks many pieces of code that are abandoned and will never be updated. If we see evidence that such abandoned code would still create widespread disruption to users, such as because it continues to be relied upon by many actively maintained dependent crates, then we would ask for other mitigating actions to be taken. However, we do not treat abandoned code relying on these areas of underspecified language semantics to be a permanent blocker to making minor breaking changes that we consider valuable.

Four, we look at what we're getting in return for the breakage. Here, the eventual breakage we'll take will allow us to stabilize the never type without the sort of compromises that had stalled prior attempts. That has a lot of value to us, and so we're willing -- carefully and with due consideration for the stability of Rust -- to take the steps necessary to get there.

All together, in this case, that was enough for us to conclude that we can change the lint level, as proposed here, and as confirmed in our FCP.

(Perhaps others on lang have further thoughts.)