Auto merge of rust-lang#94634 - Dylan-DPC:rollup-8wx1yrj, r=Dylan-DPC

Rollup of 6 pull requests

Successful merges:

 - rust-lang#94446 (UNIX `remove_dir_all()`: Try recursing first on the slow path)
 - rust-lang#94460 (Reenable generator drop tracking tests and fix mutation handling)
 - rust-lang#94620 (Edit docs on consistency of `PartialOrd` and `PartialEq`)
 - rust-lang#94624 (Downgrade `#[test]` on macro call to warning)
 - rust-lang#94626 (Add known-bug directive to issue rust-lang#47511 test case)
 - rust-lang#94631 (Fix typo in c-variadic)

Failed merges:

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

Author: bors

compiler/rustc_builtin_macros/src/test.rs

@@ -105,14 +105,18 @@ pub fn expand_test_or_bench(
 
     // Note: non-associated fn items are already handled by `expand_test_or_bench`
     if !matches!(item.kind, ast::ItemKind::Fn(_)) {
-        cx.sess
-            .parse_sess
-            .span_diagnostic
-            .struct_span_err(
-                attr_sp,
-                "the `#[test]` attribute may only be used on a non-associated function",
-            )
-            .note("the `#[test]` macro causes a a function to be run on a test and has no effect on non-functions")
+        let diag = &cx.sess.parse_sess.span_diagnostic;
+        let msg = "the `#[test]` attribute may only be used on a non-associated function";
+        let mut err = match item.kind {
+            // These were a warning before #92959 and need to continue being that to avoid breaking
+            // stable user code (#94508).
+            ast::ItemKind::MacCall(_) => diag.struct_span_warn(attr_sp, msg),
+            // `.forget_guarantee()` needed to get these two arms to match types. Because of how
+            // locally close the `.emit()` call is I'm comfortable with it, but if it can be
+            // reworked in the future to not need it, it'd be nice.
+            _ => diag.struct_span_err(attr_sp, msg).forget_guarantee(),
+        };
+        err.span_label(attr_sp, "the `#[test]` macro causes a a function to be run on a test and has no effect on non-functions")
             .span_label(item.span, format!("expected a non-associated function, found {} {}", item.kind.article(), item.kind.descr()))
             .span_suggestion(attr_sp, "replace with conditional compilation to make the item only exist when tests are being run", String::from("#[cfg(test)]"), Applicability::MaybeIncorrect)
             .emit();

compiler/rustc_typeck/src/check/generator_interior/drop_ranges/record_consumed_borrow.rs

@@ -6,14 +6,14 @@ use crate::{
 use hir::{def_id::DefId, Body, HirId, HirIdMap};
 use rustc_data_structures::stable_set::FxHashSet;
 use rustc_hir as hir;
-use rustc_middle::hir::map::Map;
+use rustc_middle::ty::{ParamEnv, TyCtxt};
 
 pub(super) fn find_consumed_and_borrowed<'a, 'tcx>(
     fcx: &'a FnCtxt<'a, 'tcx>,
     def_id: DefId,
     body: &'tcx Body<'tcx>,
 ) -> ConsumedAndBorrowedPlaces {
-    let mut expr_use_visitor = ExprUseDelegate::new(fcx.tcx.hir());
+    let mut expr_use_visitor = ExprUseDelegate::new(fcx.tcx, fcx.param_env);
     expr_use_visitor.consume_body(fcx, def_id, body);
     expr_use_visitor.places
 }
@@ -36,14 +36,16 @@ pub(super) struct ConsumedAndBorrowedPlaces {
 /// Interesting values are those that are either dropped or borrowed. For dropped values, we also
 /// record the parent expression, which is the point where the drop actually takes place.
 struct ExprUseDelegate<'tcx> {
-    hir: Map<'tcx>,
+    tcx: TyCtxt<'tcx>,
+    param_env: ParamEnv<'tcx>,
     places: ConsumedAndBorrowedPlaces,
 }
 
 impl<'tcx> ExprUseDelegate<'tcx> {
-    fn new(hir: Map<'tcx>) -> Self {
+    fn new(tcx: TyCtxt<'tcx>, param_env: ParamEnv<'tcx>) -> Self {
         Self {
-            hir,
+            tcx,
+            param_env,
             places: ConsumedAndBorrowedPlaces {
                 consumed: <_>::default(),
                 borrowed: <_>::default(),
@@ -77,7 +79,7 @@ impl<'tcx> expr_use_visitor::Delegate<'tcx> for ExprUseDelegate<'tcx> {
         place_with_id: &expr_use_visitor::PlaceWithHirId<'tcx>,
         diag_expr_id: HirId,
     ) {
-        let parent = match self.hir.find_parent_node(place_with_id.hir_id) {
+        let parent = match self.tcx.hir().find_parent_node(place_with_id.hir_id) {
             Some(parent) => parent,
             None => place_with_id.hir_id,
         };
@@ -107,11 +109,22 @@ impl<'tcx> expr_use_visitor::Delegate<'tcx> for ExprUseDelegate<'tcx> {
         assignee_place: &expr_use_visitor::PlaceWithHirId<'tcx>,
         diag_expr_id: HirId,
     ) {
-        debug!("mutate {:?}; diag_expr_id={:?}", assignee_place, diag_expr_id);
-        // Count mutations as a borrow.
-        self.places
-            .borrowed
-            .insert(TrackedValue::from_place_with_projections_allowed(assignee_place));
+        debug!("mutate {assignee_place:?}; diag_expr_id={diag_expr_id:?}");
+        // If the type being assigned needs dropped, then the mutation counts as a borrow
+        // since it is essentially doing `Drop::drop(&mut x); x = new_value;`.
+        if assignee_place.place.base_ty.needs_drop(self.tcx, self.param_env) {
+            self.places
+                .borrowed
+                .insert(TrackedValue::from_place_with_projections_allowed(assignee_place));
+        }
+    }
+
+    fn bind(
+        &mut self,
+        binding_place: &expr_use_visitor::PlaceWithHirId<'tcx>,
+        diag_expr_id: HirId,
+    ) {
+        debug!("bind {binding_place:?}; diag_expr_id={diag_expr_id:?}");
     }
 
     fn fake_read(

compiler/rustc_typeck/src/expr_use_visitor.rs

@@ -51,6 +51,15 @@ pub trait Delegate<'tcx> {
     /// `diag_expr_id` is the id used for diagnostics (see `consume` for more details).
     fn mutate(&mut self, assignee_place: &PlaceWithHirId<'tcx>, diag_expr_id: hir::HirId);
 
+    /// The path at `binding_place` is a binding that is being initialized.
+    ///
+    /// This covers cases such as `let x = 42;`
+    fn bind(&mut self, binding_place: &PlaceWithHirId<'tcx>, diag_expr_id: hir::HirId) {
+        // Bindings can normally be treated as a regular assignment, so by default we
+        // forward this to the mutate callback.
+        self.mutate(binding_place, diag_expr_id)
+    }
+
     /// The `place` should be a fake read because of specified `cause`.
     fn fake_read(&mut self, place: Place<'tcx>, cause: FakeReadCause, diag_expr_id: hir::HirId);
 }
@@ -648,11 +657,9 @@ impl<'a, 'tcx> ExprUseVisitor<'a, 'tcx> {
                     let pat_ty = return_if_err!(mc.node_ty(pat.hir_id));
                     debug!("walk_pat: pat_ty={:?}", pat_ty);
 
-                    // Each match binding is effectively an assignment to the
-                    // binding being produced.
                     let def = Res::Local(canonical_id);
                     if let Ok(ref binding_place) = mc.cat_res(pat.hir_id, pat.span, pat_ty, def) {
-                        delegate.mutate(binding_place, binding_place.hir_id);
+                        delegate.bind(binding_place, binding_place.hir_id);
                     }
 
                     // It is also a borrow or copy/move of the value being matched.

library/core/src/cmp.rs

@@ -885,19 +885,18 @@ impl PartialOrd for Ordering {
 /// The `lt`, `le`, `gt`, and `ge` methods of this trait can be called using
 /// the `<`, `<=`, `>`, and `>=` operators, respectively.
 ///
-/// The methods of this trait must be consistent with each other and with those of `PartialEq` in
-/// the following sense:
-///
-/// - `a == b` if and only if `partial_cmp(a, b) == Some(Equal)`.
-/// - `a < b` if and only if `partial_cmp(a, b) == Some(Less)`
-///   (ensured by the default implementation).
-/// - `a > b` if and only if `partial_cmp(a, b) == Some(Greater)`
-///   (ensured by the default implementation).
-/// - `a <= b` if and only if `a < b || a == b`
-///   (ensured by the default implementation).
-/// - `a >= b` if and only if `a > b || a == b`
-///   (ensured by the default implementation).
-/// - `a != b` if and only if `!(a == b)` (already part of `PartialEq`).
+/// The methods of this trait must be consistent with each other and with those of [`PartialEq`].
+/// The following conditions must hold:
+///
+/// 1. `a == b` if and only if `partial_cmp(a, b) == Some(Equal)`.
+/// 2. `a < b` if and only if `partial_cmp(a, b) == Some(Less)`
+/// 3. `a > b` if and only if `partial_cmp(a, b) == Some(Greater)`
+/// 4. `a <= b` if and only if `a < b || a == b`
+/// 5. `a >= b` if and only if `a > b || a == b`
+/// 6. `a != b` if and only if `!(a == b)`.
+///
+/// Conditions 2–5 above are ensured by the default implementation.
+/// Condition 6 is already ensured by [`PartialEq`].
 ///
 /// If [`Ord`] is also implemented for `Self` and `Rhs`, it must also be consistent with
 /// `partial_cmp` (see the documentation of that trait for the exact requirements). It's