Auto merge of #94357 - matthiaskrgr:rollup-xrjaof3, r=matthiaskrgr

Rollup of 7 pull requests

Successful merges:

 - #93845 (Remove in band lifetimes)
 - #94155 (Extend toggle GUI test a bit)
 - #94252 (don't special case `DefKind::Ctor` in encoding)
 - #94305 (Remove an unnecessary restriction in `dest_prop`)
 - #94343 (Miri fn ptr check: don't use conservative null check)
 - #94344 (diagnostic: suggest parens when users want logical ops, but get closures)
 - #94352 (Fix SGX docs build)

Failed merges:

r? `@ghost`
`@rustbot` modify labels: rollup

Author: bors

Diff for: compiler/rustc_ast_lowering/src/lib.rs

@@ -142,13 +142,9 @@ struct LoweringContext<'a, 'hir: 'a> {
     /// indicate whether or not we're in a place where new lifetimes will result
     /// in in-band lifetime definitions, such a function or an impl header,
     /// including implicit lifetimes from `impl_header_lifetime_elision`.
-    is_collecting_in_band_lifetimes: bool,
+    is_collecting_anonymous_lifetimes: bool,
 
     /// Currently in-scope lifetimes defined in impl headers, fn headers, or HRTB.
-    /// When `is_collecting_in_band_lifetimes` is true, each lifetime is checked
-    /// against this list to see if it is already in-scope, or if a definition
-    /// needs to be created for it.
-    ///
     /// We always store a `normalize_to_macros_2_0()` version of the param-name in this
     /// vector.
     in_scope_lifetimes: Vec<ParamName>,
@@ -379,7 +375,7 @@ pub fn lower_crate<'a, 'hir>(
         task_context: None,
         current_item: None,
         lifetimes_to_define: Vec::new(),
-        is_collecting_in_band_lifetimes: false,
+        is_collecting_anonymous_lifetimes: false,
         in_scope_lifetimes: Vec::new(),
         allow_try_trait: Some([sym::try_trait_v2][..].into()),
         allow_gen_future: Some([sym::gen_future][..].into()),
@@ -726,13 +722,13 @@ impl<'a, 'hir> LoweringContext<'a, 'hir> {
         &mut self,
         f: impl FnOnce(&mut Self) -> T,
     ) -> (Vec<(Span, ParamName)>, T) {
-        let was_collecting = std::mem::replace(&mut self.is_collecting_in_band_lifetimes, true);
+        let was_collecting = std::mem::replace(&mut self.is_collecting_anonymous_lifetimes, true);
         let len = self.lifetimes_to_define.len();
 
         let res = f(self);
 
         let lifetimes_to_define = self.lifetimes_to_define.split_off(len);
-        self.is_collecting_in_band_lifetimes = was_collecting;
+        self.is_collecting_anonymous_lifetimes = was_collecting;
         (lifetimes_to_define, res)
     }
 
@@ -749,7 +745,7 @@ impl<'a, 'hir> LoweringContext<'a, 'hir> {
         // that collisions are ok here and this shouldn't
         // really show up for end-user.
         let (str_name, kind) = match hir_name {
-            ParamName::Plain(ident) => (ident.name, hir::LifetimeParamKind::InBand),
+            ParamName::Plain(ident) => (ident.name, hir::LifetimeParamKind::Explicit),
             ParamName::Fresh(_) => (kw::UnderscoreLifetime, hir::LifetimeParamKind::Elided),
             ParamName::Error => (kw::UnderscoreLifetime, hir::LifetimeParamKind::Error),
         };
@@ -773,38 +769,10 @@ impl<'a, 'hir> LoweringContext<'a, 'hir> {
         }
     }
 
-    /// When there is a reference to some lifetime `'a`, and in-band
-    /// lifetimes are enabled, then we want to push that lifetime into
-    /// the vector of names to define later. In that case, it will get
-    /// added to the appropriate generics.
-    fn maybe_collect_in_band_lifetime(&mut self, ident: Ident) {
-        if !self.is_collecting_in_band_lifetimes {
-            return;
-        }
-
-        if !self.sess.features_untracked().in_band_lifetimes {
-            return;
-        }
-
-        if self.in_scope_lifetimes.contains(&ParamName::Plain(ident.normalize_to_macros_2_0())) {
-            return;
-        }
-
-        let hir_name = ParamName::Plain(ident);
-
-        if self.lifetimes_to_define.iter().any(|(_, lt_name)| {
-            lt_name.normalize_to_macros_2_0() == hir_name.normalize_to_macros_2_0()
-        }) {
-            return;
-        }
-
-        self.lifetimes_to_define.push((ident.span, hir_name));
-    }
-
     /// When we have either an elided or `'_` lifetime in an impl
     /// header, we convert it to an in-band lifetime.
-    fn collect_fresh_in_band_lifetime(&mut self, span: Span) -> ParamName {
-        assert!(self.is_collecting_in_band_lifetimes);
+    fn collect_fresh_anonymous_lifetime(&mut self, span: Span) -> ParamName {
+        assert!(self.is_collecting_anonymous_lifetimes);
         let index = self.lifetimes_to_define.len() + self.in_scope_lifetimes.len();
         let hir_name = ParamName::Fresh(index);
         self.lifetimes_to_define.push((span, hir_name));
@@ -1946,7 +1914,7 @@ impl<'a, 'hir> LoweringContext<'a, 'hir> {
             }
             ident if ident.name == kw::UnderscoreLifetime => match self.anonymous_lifetime_mode {
                 AnonymousLifetimeMode::CreateParameter => {
-                    let fresh_name = self.collect_fresh_in_band_lifetime(span);
+                    let fresh_name = self.collect_fresh_anonymous_lifetime(span);
                     self.new_named_lifetime(l.id, span, hir::LifetimeName::Param(fresh_name))
                 }
 
@@ -1957,7 +1925,6 @@ impl<'a, 'hir> LoweringContext<'a, 'hir> {
                 AnonymousLifetimeMode::ReportError => self.new_error_lifetime(Some(l.id), span),
             },
             ident => {
-                self.maybe_collect_in_band_lifetime(ident);
                 let param_name = ParamName::Plain(self.lower_ident(ident));
                 self.new_named_lifetime(l.id, span, hir::LifetimeName::Param(param_name))
             }
@@ -2001,8 +1968,8 @@ impl<'a, 'hir> LoweringContext<'a, 'hir> {
 
         let (name, kind) = match param.kind {
             GenericParamKind::Lifetime => {
-                let was_collecting_in_band = self.is_collecting_in_band_lifetimes;
-                self.is_collecting_in_band_lifetimes = false;
+                let was_collecting_in_band = self.is_collecting_anonymous_lifetimes;
+                self.is_collecting_anonymous_lifetimes = false;
 
                 let lt = self
                     .with_anonymous_lifetime_mode(AnonymousLifetimeMode::ReportError, |this| {
@@ -2025,7 +1992,7 @@ impl<'a, 'hir> LoweringContext<'a, 'hir> {
                 let kind =
                     hir::GenericParamKind::Lifetime { kind: hir::LifetimeParamKind::Explicit };
 
-                self.is_collecting_in_band_lifetimes = was_collecting_in_band;
+                self.is_collecting_anonymous_lifetimes = was_collecting_in_band;
 
                 (param_name, kind)
             }
@@ -2384,7 +2351,7 @@ impl<'a, 'hir> LoweringContext<'a, 'hir> {
             // Hence `impl Foo for &u32` becomes `impl<'f> Foo for &'f u32` for some fresh
             // `'f`.
             AnonymousLifetimeMode::CreateParameter => {
-                let fresh_name = self.collect_fresh_in_band_lifetime(span);
+                let fresh_name = self.collect_fresh_anonymous_lifetime(span);
                 hir::Lifetime {
                     hir_id: self.next_id(),
                     span: self.lower_span(span),

Diff for: compiler/rustc_const_eval/src/const_eval/machine.rs

@@ -217,8 +217,9 @@ impl<'mir, 'tcx: 'mir> CompileTimeEvalContext<'mir, 'tcx> {
             // Comparisons of abstract pointers with null pointers are known if the pointer
             // is in bounds, because if they are in bounds, the pointer can't be null.
             // Inequality with integers other than null can never be known for sure.
-            (Scalar::Int(int), Scalar::Ptr(ptr, _)) | (Scalar::Ptr(ptr, _), Scalar::Int(int)) => {
-                int.is_null() && !self.memory.ptr_may_be_null(ptr.into())
+            (Scalar::Int(int), ptr @ Scalar::Ptr(..))
+            | (ptr @ Scalar::Ptr(..), Scalar::Int(int)) => {
+                int.is_null() && !self.scalar_may_be_null(ptr)
             }
             // FIXME: return `true` for at least some comparisons where we can reliably
             // determine the result of runtime inequality tests at compile-time.

Diff for: compiler/rustc_const_eval/src/interpret/eval_context.rs

@@ -22,9 +22,9 @@ use rustc_span::{Pos, Span};
 use rustc_target::abi::{call::FnAbi, Align, HasDataLayout, Size, TargetDataLayout};
 
 use super::{
-    AllocId, GlobalId, Immediate, InterpErrorInfo, InterpResult, MPlaceTy, Machine, MemPlace,
-    MemPlaceMeta, Memory, MemoryKind, Operand, Place, PlaceTy, Pointer, Provenance, Scalar,
-    ScalarMaybeUninit, StackPopJump,
+    AllocCheck, AllocId, GlobalId, Immediate, InterpErrorInfo, InterpResult, MPlaceTy, Machine,
+    MemPlace, MemPlaceMeta, Memory, MemoryKind, Operand, Place, PlaceTy, Pointer, Provenance,
+    Scalar, ScalarMaybeUninit, StackPopJump,
 };
 use crate::transform::validate::equal_up_to_regions;
 
@@ -440,6 +440,29 @@ impl<'mir, 'tcx: 'mir, M: Machine<'mir, 'tcx>> InterpCx<'mir, 'tcx, M> {
         self.memory.scalar_to_ptr(scalar)
     }
 
+    /// Test if this value might be null.
+    /// If the machine does not support ptr-to-int casts, this is conservative.
+    pub fn scalar_may_be_null(&self, scalar: Scalar<M::PointerTag>) -> bool {
+        match scalar.try_to_int() {
+            Ok(int) => int.is_null(),
+            Err(_) => {
+                let ptr = self.scalar_to_ptr(scalar);
+                match self.memory.ptr_try_get_alloc(ptr) {
+                    Ok((alloc_id, offset, _)) => {
+                        let (size, _align) = self
+                            .memory
+                            .get_size_and_align(alloc_id, AllocCheck::MaybeDead)
+                            .expect("alloc info with MaybeDead cannot fail");
+                        // If the pointer is out-of-bounds, it may be null.
+                        // Note that one-past-the-end (offset == size) is still inbounds, and never null.
+                        offset > size
+                    }
+                    Err(offset) => offset == 0,
+                }
+            }
+        }
+    }
+
     /// Call this to turn untagged "global" pointers (obtained via `tcx`) into
     /// the machine pointer to the allocation.  Must never be used
     /// for any other pointers, nor for TLS statics.

Diff for: compiler/rustc_const_eval/src/interpret/memory.rs

@@ -483,21 +483,6 @@ impl<'mir, 'tcx, M: Machine<'mir, 'tcx>> Memory<'mir, 'tcx, M> {
             }
         })
     }
-
-    /// Test if the pointer might be null.
-    pub fn ptr_may_be_null(&self, ptr: Pointer<Option<M::PointerTag>>) -> bool {
-        match self.ptr_try_get_alloc(ptr) {
-            Ok((alloc_id, offset, _)) => {
-                let (size, _align) = self
-                    .get_size_and_align(alloc_id, AllocCheck::MaybeDead)
-                    .expect("alloc info with MaybeDead cannot fail");
-                // If the pointer is out-of-bounds, it may be null.
-                // Note that one-past-the-end (offset == size) is still inbounds, and never null.
-                offset > size
-            }
-            Err(offset) => offset == 0,
-        }
-    }
 }
 
 /// Allocation accessors

Diff for: compiler/rustc_const_eval/src/interpret/operand.rs

@@ -720,12 +720,11 @@ impl<'mir, 'tcx: 'mir, M: Machine<'mir, 'tcx>> InterpCx<'mir, 'tcx, M> {
                     Err(dbg_val) => {
                         // So this is a pointer then, and casting to an int failed.
                         // Can only happen during CTFE.
-                        let ptr = self.scalar_to_ptr(tag_val);
                         // The niche must be just 0, and the ptr not null, then we know this is
                         // okay. Everything else, we conservatively reject.
                         let ptr_valid = niche_start == 0
                             && variants_start == variants_end
-                            && !self.memory.ptr_may_be_null(ptr);
+                            && !self.scalar_may_be_null(tag_val);
                         if !ptr_valid {
                             throw_ub!(InvalidTag(dbg_val))
                         }

Diff for: compiler/rustc_const_eval/src/interpret/validity.rs

@@ -572,21 +572,25 @@ impl<'rt, 'mir, 'tcx: 'mir, M: Machine<'mir, 'tcx>> ValidityVisitor<'rt, 'mir, '
                     err_unsup!(ReadPointerAsBytes) => { "part of a pointer" } expected { "a proper pointer or integer value" },
                     err_ub!(InvalidUninitBytes(None)) => { "uninitialized bytes" } expected { "a proper pointer or integer value" },
                 );
-                let ptr = self.ecx.scalar_to_ptr(value);
-                // Ensure the pointer is non-null.
-                if self.ecx.memory.ptr_may_be_null(ptr) {
-                    throw_validation_failure!(self.path, { "a potentially null function pointer" });
-                }
+
                 // If we check references recursively, also check that this points to a function.
                 if let Some(_) = self.ref_tracking {
+                    let ptr = self.ecx.scalar_to_ptr(value);
                     let _fn = try_validation!(
                         self.ecx.memory.get_fn(ptr),
                         self.path,
+                        err_ub!(DanglingIntPointer(0, _)) =>
+                            { "a null function pointer" },
                         err_ub!(DanglingIntPointer(..)) |
                         err_ub!(InvalidFunctionPointer(..)) =>
                             { "{:x}", value } expected { "a function pointer" },
                     );
                     // FIXME: Check if the signature matches
+                } else {
+                    // Otherwise (for standalone Miri), we have to still check it to be non-null.
+                    if self.ecx.scalar_may_be_null(value) {
+                        throw_validation_failure!(self.path, { "a null function pointer" });
+                    }
                 }
                 Ok(true)
             }
@@ -644,10 +648,9 @@ impl<'rt, 'mir, 'tcx: 'mir, M: Machine<'mir, 'tcx>> ValidityVisitor<'rt, 'mir, '
             Err(_) => {
                 // So this is a pointer then, and casting to an int failed.
                 // Can only happen during CTFE.
-                let ptr = self.ecx.scalar_to_ptr(value);
                 if start == 1 && end == max_value {
                     // Only null is the niche.  So make sure the ptr is NOT null.
-                    if self.ecx.memory.ptr_may_be_null(ptr) {
+                    if self.ecx.scalar_may_be_null(value) {
                         throw_validation_failure!(self.path,
                             { "a potentially null pointer" }
                             expected {
@@ -758,7 +761,7 @@ impl<'rt, 'mir, 'tcx: 'mir, M: Machine<'mir, 'tcx>> ValueVisitor<'mir, 'tcx, M>
     fn visit_value(&mut self, op: &OpTy<'tcx, M::PointerTag>) -> InterpResult<'tcx> {
         trace!("visit_value: {:?}, {:?}", *op, op.layout);
 
-        // Check primitive types -- the leafs of our recursive descend.
+        // Check primitive types -- the leaves of our recursive descent.
         if self.try_visit_primitive(op)? {
             return Ok(());
         }

Diff for: compiler/rustc_error_codes/src/error_codes/E0687.md

@@ -1,8 +1,10 @@
+#### Note: this error code is no longer emitted by the compiler.
+
 In-band lifetimes cannot be used in `fn`/`Fn` syntax.
 
 Erroneous code examples:
 
-```compile_fail,E0687
+```ignore (feature got removed)
 #![feature(in_band_lifetimes)]
 
 fn foo(x: fn(&'a u32)) {} // error!

Diff for: compiler/rustc_error_codes/src/error_codes/E0688.md

@@ -1,8 +1,10 @@
+#### Note: this error code is no longer emitted by the compiler.
+
 In-band lifetimes were mixed with explicit lifetime binders.
 
 Erroneous code example:
 
-```compile_fail,E0688
+```ignore (feature got removed)
 #![feature(in_band_lifetimes)]
 
 fn foo<'a>(x: &'a u32, y: &'b u32) {}   // error!

Diff for: compiler/rustc_feature/src/active.rs

@@ -400,8 +400,6 @@ declare_features! (
     (active, if_let_guard, "1.47.0", Some(51114), None),
     /// Allows using imported `main` function
     (active, imported_main, "1.53.0", Some(28937), None),
-    /// Allows in-band quantification of lifetime bindings (e.g., `fn foo(x: &'a u8) -> &'a u8`).
-    (active, in_band_lifetimes, "1.23.0", Some(44524), None),
     /// Allows inferring `'static` outlives requirements (RFC 2093).
     (active, infer_static_outlives_requirements, "1.26.0", Some(54185), None),
     /// Allows associated types in inherent impls.

Diff for: compiler/rustc_feature/src/removed.rs

@@ -104,6 +104,9 @@ declare_features! (
     (removed, impl_trait_in_bindings, "1.55.0", Some(63065), None,
      Some("the implementation was not maintainable, the feature may get reintroduced once the current refactorings are done")),
     (removed, import_shadowing, "1.0.0", None, None, None),
+    /// Allows in-band quantification of lifetime bindings (e.g., `fn foo(x: &'a u8) -> &'a u8`).
+    (removed, in_band_lifetimes, "1.23.0", Some(44524), None,
+     Some("removed due to unsolved ergonomic questions and added lifetime resolution complexity")),
     /// Lazily evaluate constants. This allows constants to depend on type parameters.
     (removed, lazy_normalization_consts, "1.46.0", Some(72219), None, Some("superseded by `generic_const_exprs`")),
     /// Allows using the `#[link_args]` attribute.

Diff for: compiler/rustc_hir/src/hir.rs

@@ -471,11 +471,6 @@ pub enum LifetimeParamKind {
     // `fn foo<'a>(x: &'a u8) -> &'a u8 { x }`).
     Explicit,
 
-    // Indicates that the lifetime definition was synthetically added
-    // as a result of an in-band lifetime usage (e.g., in
-    // `fn foo(x: &'a u8) -> &'a u8 { x }`).
-    InBand,
-
     // Indication that the lifetime was elided (e.g., in both cases in
     // `fn foo(x: &u8) -> &'_ u8 { x }`).
     Elided,

Diff for: compiler/rustc_infer/src/infer/error_reporting/nice_region_error/find_anon_type.rs

@@ -125,7 +125,7 @@ impl<'tcx> Visitor<'tcx> for FindNestedTypeVisitor<'tcx> {
                     // Find the index of the named region that was part of the
                     // error. We will then search the function parameters for a bound
                     // region at the right depth with the same index
-                    (Some(rl::Region::EarlyBound(_, id, _)), ty::BrNamed(def_id, _)) => {
+                    (Some(rl::Region::EarlyBound(_, id)), ty::BrNamed(def_id, _)) => {
                         debug!("EarlyBound id={:?} def_id={:?}", id, def_id);
                         if id == def_id {
                             self.found_type = Some(arg);
@@ -137,7 +137,7 @@ impl<'tcx> Visitor<'tcx> for FindNestedTypeVisitor<'tcx> {
                     // error. We will then search the function parameters for a bound
                     // region at the right depth with the same index
                     (
-                        Some(rl::Region::LateBound(debruijn_index, _, id, _)),
+                        Some(rl::Region::LateBound(debruijn_index, _, id)),
                         ty::BrNamed(def_id, _),
                     ) => {
                         debug!(
@@ -155,8 +155,8 @@ impl<'tcx> Visitor<'tcx> for FindNestedTypeVisitor<'tcx> {
                         Some(
                             rl::Region::Static
                             | rl::Region::Free(_, _)
-                            | rl::Region::EarlyBound(_, _, _)
-                            | rl::Region::LateBound(_, _, _, _)
+                            | rl::Region::EarlyBound(_, _)
+                            | rl::Region::LateBound(_, _, _)
                             | rl::Region::LateBoundAnon(_, _, _),
                         )
                         | None,
@@ -221,15 +221,15 @@ impl<'tcx> Visitor<'tcx> for TyPathVisitor<'tcx> {
                 }
             }
 
-            (Some(rl::Region::EarlyBound(_, id, _)), ty::BrNamed(def_id, _)) => {
+            (Some(rl::Region::EarlyBound(_, id)), ty::BrNamed(def_id, _)) => {
                 debug!("EarlyBound id={:?} def_id={:?}", id, def_id);
                 if id == def_id {
                     self.found_it = true;
                     return; // we can stop visiting now
                 }
             }
 
-            (Some(rl::Region::LateBound(debruijn_index, _, id, _)), ty::BrNamed(def_id, _)) => {
+            (Some(rl::Region::LateBound(debruijn_index, _, id)), ty::BrNamed(def_id, _)) => {
                 debug!("FindNestedTypeVisitor::visit_ty: LateBound depth = {:?}", debruijn_index,);
                 debug!("id={:?}", id);
                 debug!("def_id={:?}", def_id);
@@ -242,8 +242,8 @@ impl<'tcx> Visitor<'tcx> for TyPathVisitor<'tcx> {
             (
                 Some(
                     rl::Region::Static
-                    | rl::Region::EarlyBound(_, _, _)
-                    | rl::Region::LateBound(_, _, _, _)
+                    | rl::Region::EarlyBound(_, _)
+                    | rl::Region::LateBound(_, _, _)
                     | rl::Region::LateBoundAnon(_, _, _)
                     | rl::Region::Free(_, _),
                 )