Auto merge of rust-lang#125289 - WaffleLapkin:never-obligations, r=compiler-errors

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:
- rust-lang#123748

Author: bors

compiler/rustc_hir_typeck/messages.ftl

@@ -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

compiler/rustc_hir_typeck/src/errors.rs

@@ -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,

compiler/rustc_hir_typeck/src/fallback.rs

@@ -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 {
@@ -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);
@@ -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::FallbackToUnit = 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> {

compiler/rustc_lint_defs/src/builtin.rs

@@ -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,
@@ -4195,6 +4196,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

src/tools/clippy/tests/ui/new_ret_no_self.rs

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

src/tools/clippy/tests/ui/new_ret_no_self.stderr

@@ -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 | |         }
    | |_________^
 

tests/ui/delegation/not-supported.rs

@@ -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.

tests/ui/delegation/not-supported.stderr

@@ -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`.

tests/ui/never_type/defaulted-never-note.fallback.stderr

@@ -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 `!`

tests/ui/never_type/defaulted-never-note.nofallback.stderr

@@ -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
+

tests/ui/never_type/defaulted-never-note.rs

@@ -26,6 +26,8 @@ fn foo<T: ImplementedForUnitButNotNever>(_t: T) {}
 //[fallback]~^ NOTE required by this bound in `foo`
 //[fallback]~| NOTE required by a bound in `foo`
 fn smeg() {
+    //[nofallback]~^ warn: this function depends on never type fallback being `()`
+    //[nofallback]~| warn: this was previously accepted by the compiler but is being phased out; it will become a hard error in a future release!
     let _x = return;
     foo(_x);
     //[fallback]~^ ERROR the trait bound

tests/ui/never_type/dependency-on-fallback-to-unit.rs

@@ -0,0 +1,28 @@
+//@ check-pass
+
+fn main() {
+    def();
+    _ = question_mark();
+}
+
+fn def() {
+    //~^ 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!
+    match true {
+        false => <_>::default(),
+        true => return,
+    };
+}
+
+// <https://github.com/rust-lang/rust/issues/51125>
+// <https://github.com/rust-lang/rust/issues/39216>
+fn question_mark() -> Result<(), ()> {
+    //~^ 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!
+    deserialize()?;
+    Ok(())
+}
+
+fn deserialize<T: Default>() -> Result<T, ()> {
+    Ok(T::default())
+}

tests/ui/never_type/dependency-on-fallback-to-unit.stderr

@@ -0,0 +1,23 @@
+warning: this function depends on never type fallback being `()`
+  --> $DIR/dependency-on-fallback-to-unit.rs:8:1
+   |
+LL | fn def() {
+   | ^^^^^^^^
+   |
+   = 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/dependency-on-fallback-to-unit.rs:19:1
+   |
+LL | fn question_mark() -> Result<(), ()> {
+   | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
+   |
+   = 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
+