Skip to content

Commit

Permalink
Auto merge of #125289 - WaffleLapkin:never-obligations, r=compiler-er…
Browse files Browse the repository at this point in the history
…rors

Implement lint for obligations broken by never type fallback change

This is the second (and probably last major?) lint required for the never type fallback change.

The idea is to check if the code errors with `fallback = ()` and if it errors with `fallback = !` and if it went from "ok" to "error", lint.

I'm not happy with the diagnostic, ideally we'd highlight what bound is the problem. But I'm really unsure how to do that  (cc `@jackh726,` iirc you had some ideas?)

r? `@compiler-errors`

Thanks `@BoxyUwU` with helping with trait solver stuff when I was implementing the initial version of this lint.

Tracking:
- #123748
  • Loading branch information
bors committed Jun 13, 2024
2 parents 9fdbfe1 + ea98e42 commit b6e5e3f
Show file tree
Hide file tree
Showing 27 changed files with 344 additions and 29 deletions.
3 changes: 3 additions & 0 deletions compiler/rustc_hir_typeck/messages.ftl
Original file line number Diff line number Diff line change
Expand Up @@ -44,6 +44,9 @@ hir_typeck_convert_using_method = try using `{$sugg}` to convert `{$found}` to `
hir_typeck_ctor_is_private = tuple struct constructor `{$def}` is private
hir_typeck_dependency_on_unit_never_type_fallback = this function depends on never type fallback being `()`
.help = specify the types explicitly
hir_typeck_deref_is_empty = this expression `Deref`s to `{$deref_ty}` which implements `is_empty`
hir_typeck_expected_default_return_type = expected `()` because of default return type
Expand Down
5 changes: 5 additions & 0 deletions compiler/rustc_hir_typeck/src/errors.rs
Original file line number Diff line number Diff line change
Expand Up @@ -183,6 +183,11 @@ pub enum NeverTypeFallbackFlowingIntoUnsafe {
Deref,
}

#[derive(LintDiagnostic)]
#[help]
#[diag(hir_typeck_dependency_on_unit_never_type_fallback)]
pub struct DependencyOnUnitNeverTypeFallback {}

#[derive(Subdiagnostic)]
#[multipart_suggestion(
hir_typeck_add_missing_parentheses_in_range,
Expand Down
50 changes: 50 additions & 0 deletions compiler/rustc_hir_typeck/src/fallback.rs
Original file line number Diff line number Diff line change
Expand Up @@ -14,6 +14,7 @@ use rustc_middle::ty::{self, Ty, TyCtxt, TypeSuperVisitable, TypeVisitable};
use rustc_session::lint;
use rustc_span::DUMMY_SP;
use rustc_span::{def_id::LocalDefId, Span};
use rustc_trait_selection::traits::{ObligationCause, ObligationCtxt};

#[derive(Copy, Clone)]
pub enum DivergingFallbackBehavior {
Expand Down Expand Up @@ -344,6 +345,9 @@ impl<'tcx> FnCtxt<'_, 'tcx> {
// `!`.
let mut diverging_fallback = UnordMap::with_capacity(diverging_vids.len());
let unsafe_infer_vars = OnceCell::new();

self.lint_obligations_broken_by_never_type_fallback_change(behavior, &diverging_vids);

for &diverging_vid in &diverging_vids {
let diverging_ty = Ty::new_var(self.tcx, diverging_vid);
let root_vid = self.root_var(diverging_vid);
Expand Down Expand Up @@ -468,6 +472,52 @@ impl<'tcx> FnCtxt<'_, 'tcx> {
}
}

fn lint_obligations_broken_by_never_type_fallback_change(
&self,
behavior: DivergingFallbackBehavior,
diverging_vids: &[ty::TyVid],
) {
let DivergingFallbackBehavior::ToUnit = behavior else { return };

// Fallback happens if and only if there are diverging variables
if diverging_vids.is_empty() {
return;
}

// Returns errors which happen if fallback is set to `fallback`
let remaining_errors_if_fallback_to = |fallback| {
self.probe(|_| {
let obligations = self.fulfillment_cx.borrow().pending_obligations();
let ocx = ObligationCtxt::new(&self.infcx);
ocx.register_obligations(obligations.iter().cloned());

for &diverging_vid in diverging_vids {
let diverging_ty = Ty::new_var(self.tcx, diverging_vid);

ocx.eq(&ObligationCause::dummy(), self.param_env, diverging_ty, fallback)
.expect("expected diverging var to be unconstrained");
}

ocx.select_where_possible()
})
};

// If we have no errors with `fallback = ()`, but *do* have errors with `fallback = !`,
// then this code will be broken by the never type fallback change.qba
let unit_errors = remaining_errors_if_fallback_to(self.tcx.types.unit);
if unit_errors.is_empty()
&& let never_errors = remaining_errors_if_fallback_to(self.tcx.types.never)
&& !never_errors.is_empty()
{
self.tcx.emit_node_span_lint(
lint::builtin::DEPENDENCY_ON_UNIT_NEVER_TYPE_FALLBACK,
self.tcx.local_def_id_to_hir_id(self.body_id),
self.tcx.def_span(self.body_id),
errors::DependencyOnUnitNeverTypeFallback {},
)
}
}

/// Returns a graph whose nodes are (unresolved) inference variables and where
/// an edge `?A -> ?B` indicates that the variable `?A` is coerced to `?B`.
fn create_coercion_graph(&self) -> VecGraph<ty::TyVid, true> {
Expand Down
54 changes: 54 additions & 0 deletions compiler/rustc_lint_defs/src/builtin.rs
Original file line number Diff line number Diff line change
Expand Up @@ -34,6 +34,7 @@ declare_lint_pass! {
CONST_EVALUATABLE_UNCHECKED,
CONST_ITEM_MUTATION,
DEAD_CODE,
DEPENDENCY_ON_UNIT_NEVER_TYPE_FALLBACK,
DEPRECATED,
DEPRECATED_CFG_ATTR_CRATE_TYPE_NAME,
DEPRECATED_IN_FUTURE,
Expand Down Expand Up @@ -4199,6 +4200,59 @@ declare_lint! {
report_in_external_macro
}

declare_lint! {
/// The `dependency_on_unit_never_type_fallback` lint detects cases where code compiles with
/// [never type fallback] being [`()`], but will stop compiling with fallback being [`!`].
///
/// [never type fallback]: https://doc.rust-lang.org/nightly/core/primitive.never.html#never-type-fallback
/// [`!`]: https://doc.rust-lang.org/core/primitive.never.html
/// [`()`]: https://doc.rust-lang.org/core/primitive.unit.html
///
/// ### Example
///
/// ```rust,compile_fail
/// #![deny(dependency_on_unit_never_type_fallback)]
/// fn main() {
/// if true {
/// // return has type `!` which, is some cases, causes never type fallback
/// return
/// } else {
/// // the type produced by this call is not specified explicitly,
/// // so it will be inferred from the previous branch
/// Default::default()
/// };
/// // depending on the fallback, this may compile (because `()` implements `Default`),
/// // or it may not (because `!` does not implement `Default`)
/// }
/// ```
///
/// {{produces}}
///
/// ### Explanation
///
/// Due to historic reasons never type fallback was `()`, meaning that `!` got spontaneously
/// coerced to `()`. There are plans to change that, but they may make the code such as above
/// not compile. Instead of depending on the fallback, you should specify the type explicitly:
/// ```
/// if true {
/// return
/// } else {
/// // type is explicitly specified, fallback can't hurt us no more
/// <() as Default>::default()
/// };
/// ```
///
/// See [Tracking Issue for making `!` fall back to `!`](https://github.com/rust-lang/rust/issues/123748).
pub DEPENDENCY_ON_UNIT_NEVER_TYPE_FALLBACK,
Warn,
"never type fallback affecting unsafe function calls",
@future_incompatible = FutureIncompatibleInfo {
reason: FutureIncompatibilityReason::FutureReleaseErrorDontReportInDeps,
reference: "issue #123748 <https://github.com/rust-lang/rust/issues/123748>",
};
report_in_external_macro
}

declare_lint! {
/// The `byte_slice_in_packed_struct_with_derive` lint detects cases where a byte slice field
/// (`[u8]`) or string slice field (`str`) is used in a `packed` struct that derives one or
Expand Down
5 changes: 1 addition & 4 deletions src/tools/clippy/tests/ui/new_ret_no_self.rs
Original file line number Diff line number Diff line change
Expand Up @@ -390,9 +390,7 @@ mod issue7344 {

impl<T> RetImplTraitSelf2<T> {
// should not trigger lint
fn new(t: T) -> impl Trait2<(), Self> {
unimplemented!()
}
fn new(t: T) -> impl Trait2<(), Self> {}
}

struct RetImplTraitNoSelf2<T>(T);
Expand All @@ -401,7 +399,6 @@ mod issue7344 {
// should trigger lint
fn new(t: T) -> impl Trait2<(), i32> {
//~^ ERROR: methods called `new` usually return `Self`
unimplemented!()
}
}

Expand Down
3 changes: 1 addition & 2 deletions src/tools/clippy/tests/ui/new_ret_no_self.stderr
Original file line number Diff line number Diff line change
Expand Up @@ -96,11 +96,10 @@ LL | | }
| |_________^

error: methods called `new` usually return `Self`
--> tests/ui/new_ret_no_self.rs:402:9
--> tests/ui/new_ret_no_self.rs:400:9
|
LL | / fn new(t: T) -> impl Trait2<(), i32> {
LL | |
LL | | unimplemented!()
LL | | }
| |_________^

Expand Down
4 changes: 4 additions & 0 deletions tests/ui/delegation/not-supported.rs
Original file line number Diff line number Diff line change
Expand Up @@ -70,12 +70,16 @@ mod opaque {

pub fn opaque_arg(_: impl Trait) -> i32 { 0 }
pub fn opaque_ret() -> impl Trait { unimplemented!() }
//~^ warn: this function depends on never type fallback being `()`
//~| warn: this was previously accepted by the compiler but is being phased out; it will become a hard error in a future release!
}
reuse to_reuse::opaque_arg;
//~^ ERROR delegation with early bound generics is not supported yet

trait ToReuse {
fn opaque_ret() -> impl Trait { unimplemented!() }
//~^ warn: this function depends on never type fallback being `()`
//~| warn: this was previously accepted by the compiler but is being phased out; it will become a hard error in a future release!
}

// FIXME: Inherited `impl Trait`s create query cycles when used inside trait impls.
Expand Down
51 changes: 36 additions & 15 deletions tests/ui/delegation/not-supported.stderr
Original file line number Diff line number Diff line change
Expand Up @@ -107,62 +107,83 @@ LL | reuse Trait::foo2 { &self.0 }
| ^^^^

error: delegation with early bound generics is not supported yet
--> $DIR/not-supported.rs:74:21
--> $DIR/not-supported.rs:76:21
|
LL | pub fn opaque_arg(_: impl Trait) -> i32 { 0 }
| --------------------------------------- callee defined here
...
LL | reuse to_reuse::opaque_arg;
| ^^^^^^^^^^

error[E0391]: cycle detected when computing type of `opaque::<impl at $DIR/not-supported.rs:82:5: 82:24>::{synthetic#0}`
--> $DIR/not-supported.rs:83:25
warning: this function depends on never type fallback being `()`
--> $DIR/not-supported.rs:80:9
|
LL | fn opaque_ret() -> impl Trait { unimplemented!() }
| ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
|
= warning: this was previously accepted by the compiler but is being phased out; it will become a hard error in a future release!
= note: for more information, see issue #123748 <https://github.com/rust-lang/rust/issues/123748>
= help: specify the types explicitly
= note: `#[warn(dependency_on_unit_never_type_fallback)]` on by default

error[E0391]: cycle detected when computing type of `opaque::<impl at $DIR/not-supported.rs:86:5: 86:24>::{synthetic#0}`
--> $DIR/not-supported.rs:87:25
|
LL | reuse to_reuse::opaque_ret;
| ^^^^^^^^^^
|
note: ...which requires comparing an impl and trait method signature, inferring any hidden `impl Trait` types in the process...
--> $DIR/not-supported.rs:83:25
--> $DIR/not-supported.rs:87:25
|
LL | reuse to_reuse::opaque_ret;
| ^^^^^^^^^^
= note: ...which again requires computing type of `opaque::<impl at $DIR/not-supported.rs:82:5: 82:24>::{synthetic#0}`, completing the cycle
note: cycle used when checking that `opaque::<impl at $DIR/not-supported.rs:82:5: 82:24>` is well-formed
--> $DIR/not-supported.rs:82:5
= note: ...which again requires computing type of `opaque::<impl at $DIR/not-supported.rs:86:5: 86:24>::{synthetic#0}`, completing the cycle
note: cycle used when checking that `opaque::<impl at $DIR/not-supported.rs:86:5: 86:24>` is well-formed
--> $DIR/not-supported.rs:86:5
|
LL | impl ToReuse for u8 {
| ^^^^^^^^^^^^^^^^^^^
= note: see https://rustc-dev-guide.rust-lang.org/overview.html#queries and https://rustc-dev-guide.rust-lang.org/query.html for more information

error[E0391]: cycle detected when computing type of `opaque::<impl at $DIR/not-supported.rs:85:5: 85:25>::{synthetic#0}`
--> $DIR/not-supported.rs:86:24
warning: this function depends on never type fallback being `()`
--> $DIR/not-supported.rs:72:9
|
LL | pub fn opaque_ret() -> impl Trait { unimplemented!() }
| ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
|
= warning: this was previously accepted by the compiler but is being phased out; it will become a hard error in a future release!
= note: for more information, see issue #123748 <https://github.com/rust-lang/rust/issues/123748>
= help: specify the types explicitly

error[E0391]: cycle detected when computing type of `opaque::<impl at $DIR/not-supported.rs:89:5: 89:25>::{synthetic#0}`
--> $DIR/not-supported.rs:90:24
|
LL | reuse ToReuse::opaque_ret;
| ^^^^^^^^^^
|
note: ...which requires comparing an impl and trait method signature, inferring any hidden `impl Trait` types in the process...
--> $DIR/not-supported.rs:86:24
--> $DIR/not-supported.rs:90:24
|
LL | reuse ToReuse::opaque_ret;
| ^^^^^^^^^^
= note: ...which again requires computing type of `opaque::<impl at $DIR/not-supported.rs:85:5: 85:25>::{synthetic#0}`, completing the cycle
note: cycle used when checking that `opaque::<impl at $DIR/not-supported.rs:85:5: 85:25>` is well-formed
--> $DIR/not-supported.rs:85:5
= note: ...which again requires computing type of `opaque::<impl at $DIR/not-supported.rs:89:5: 89:25>::{synthetic#0}`, completing the cycle
note: cycle used when checking that `opaque::<impl at $DIR/not-supported.rs:89:5: 89:25>` is well-formed
--> $DIR/not-supported.rs:89:5
|
LL | impl ToReuse for u16 {
| ^^^^^^^^^^^^^^^^^^^^
= note: see https://rustc-dev-guide.rust-lang.org/overview.html#queries and https://rustc-dev-guide.rust-lang.org/query.html for more information

error: recursive delegation is not supported yet
--> $DIR/not-supported.rs:99:22
--> $DIR/not-supported.rs:103:22
|
LL | pub reuse to_reuse2::foo;
| --- callee defined here
...
LL | reuse to_reuse1::foo;
| ^^^

error: aborting due to 16 previous errors
error: aborting due to 16 previous errors; 2 warnings emitted

Some errors have detailed explanations: E0049, E0195, E0391.
For more information about an error, try `rustc --explain E0049`.
23 changes: 23 additions & 0 deletions tests/ui/editions/never-type-fallback-breaking.e2021.stderr
Original file line number Diff line number Diff line change
@@ -0,0 +1,23 @@
warning: this function depends on never type fallback being `()`
--> $DIR/never-type-fallback-breaking.rs:15:1
|
LL | fn m() {
| ^^^^^^
|
= warning: this was previously accepted by the compiler but is being phased out; it will become a hard error in a future release!
= note: for more information, see issue #123748 <https://github.com/rust-lang/rust/issues/123748>
= help: specify the types explicitly
= note: `#[warn(dependency_on_unit_never_type_fallback)]` on by default

warning: this function depends on never type fallback being `()`
--> $DIR/never-type-fallback-breaking.rs:27:1
|
LL | fn q() -> Option<()> {
| ^^^^^^^^^^^^^^^^^^^^
|
= warning: this was previously accepted by the compiler but is being phased out; it will become a hard error in a future release!
= note: for more information, see issue #123748 <https://github.com/rust-lang/rust/issues/123748>
= help: specify the types explicitly

warning: 2 warnings emitted

6 changes: 3 additions & 3 deletions tests/ui/editions/never-type-fallback-breaking.e2024.stderr
Original file line number Diff line number Diff line change
@@ -1,5 +1,5 @@
error[E0277]: the trait bound `!: Default` is not satisfied
--> $DIR/never-type-fallback-breaking.rs:17:17
--> $DIR/never-type-fallback-breaking.rs:19:17
|
LL | true => Default::default(),
| ^^^^^^^^^^^^^^^^^^ the trait `Default` is not implemented for `!`
Expand All @@ -8,15 +8,15 @@ LL | true => Default::default(),
= help: did you intend to use the type `()` here instead?

error[E0277]: the trait bound `!: Default` is not satisfied
--> $DIR/never-type-fallback-breaking.rs:30:5
--> $DIR/never-type-fallback-breaking.rs:34:5
|
LL | deserialize()?;
| ^^^^^^^^^^^^^ the trait `Default` is not implemented for `!`
|
= note: this error might have been caused by changes to Rust's type-inference algorithm (see issue #48950 <https://github.com/rust-lang/rust/issues/48950> for more information)
= help: did you intend to use the type `()` here instead?
note: required by a bound in `deserialize`
--> $DIR/never-type-fallback-breaking.rs:26:23
--> $DIR/never-type-fallback-breaking.rs:30:23
|
LL | fn deserialize<T: Default>() -> Option<T> {
| ^^^^^^^ required by this bound in `deserialize`
Expand Down
4 changes: 4 additions & 0 deletions tests/ui/editions/never-type-fallback-breaking.rs
Original file line number Diff line number Diff line change
Expand Up @@ -13,6 +13,8 @@ fn main() {
}

fn m() {
//[e2021]~^ this function depends on never type fallback being `()`
//[e2021]~| this was previously accepted by the compiler but is being phased out; it will become a hard error in a future release!
let x = match true {
true => Default::default(),
//[e2024]~^ error: the trait bound `!: Default` is not satisfied
Expand All @@ -23,6 +25,8 @@ fn m() {
}

fn q() -> Option<()> {
//[e2021]~^ this function depends on never type fallback being `()`
//[e2021]~| this was previously accepted by the compiler but is being phased out; it will become a hard error in a future release!
fn deserialize<T: Default>() -> Option<T> {
Some(T::default())
}
Expand Down
2 changes: 1 addition & 1 deletion tests/ui/never_type/defaulted-never-note.fallback.stderr
Original file line number Diff line number Diff line change
@@ -1,5 +1,5 @@
error[E0277]: the trait bound `!: ImplementedForUnitButNotNever` is not satisfied
--> $DIR/defaulted-never-note.rs:30:9
--> $DIR/defaulted-never-note.rs:32:9
|
LL | foo(_x);
| --- ^^ the trait `ImplementedForUnitButNotNever` is not implemented for `!`
Expand Down
13 changes: 13 additions & 0 deletions tests/ui/never_type/defaulted-never-note.nofallback.stderr
Original file line number Diff line number Diff line change
@@ -0,0 +1,13 @@
warning: this function depends on never type fallback being `()`
--> $DIR/defaulted-never-note.rs:28:1
|
LL | fn smeg() {
| ^^^^^^^^^
|
= warning: this was previously accepted by the compiler but is being phased out; it will become a hard error in a future release!
= note: for more information, see issue #123748 <https://github.com/rust-lang/rust/issues/123748>
= help: specify the types explicitly
= note: `#[warn(dependency_on_unit_never_type_fallback)]` on by default

warning: 1 warning emitted

Loading

0 comments on commit b6e5e3f

Please sign in to comment.