Auto merge of #141255 - matthiaskrgr:rollup-ravsgen, r=matthiaskrgr

Rollup of 6 pull requests

Successful merges:

 - #131200 (Handle `rustc_query_system` cases of `rustc::potential_query_instability` lint)
 - #141244 (windows: document that we rely on an undocumented property of GetUserProfileDirectoryW)
 - #141247 (skip compiler tools sanity checks on certain commands)
 - #141248 (fix data race in ReentrantLock fallback for targets without 64bit atomics)
 - #141249 (introduce common macro for `MutVisitor` and `Visitor` to dedup code)
 - #141253 (Warning added when dependency crate has async drop types, and the feature is disabled)

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

Author: bors

compiler/rustc_ast/src/lib.rs

@@ -15,6 +15,7 @@
 #![feature(associated_type_defaults)]
 #![feature(box_patterns)]
 #![feature(if_let_guard)]
+#![feature(macro_metavar_expr)]
 #![feature(negative_impls)]
 #![feature(never_type)]
 #![feature(rustdoc_internals)]

compiler/rustc_ast/src/mut_visit.rs

@@ -20,7 +20,7 @@ use thin_vec::ThinVec;
 use crate::ast::*;
 use crate::ptr::P;
 use crate::tokenstream::*;
-use crate::visit::{AssocCtxt, BoundKind, FnCtxt};
+use crate::visit::{AssocCtxt, BoundKind, FnCtxt, try_visit};
 
 pub trait ExpectOne<A: Array> {
     fn expect_one(self, err: &'static str) -> A::Item;
@@ -388,6 +388,8 @@ pub trait MutVisitor: Sized {
     }
 }
 
+super::common_visitor_and_walkers!((mut) MutVisitor);
+
 /// Use a map-style function (`FnOnce(T) -> T`) to overwrite a `&mut T`. Useful
 /// when using a `flat_map_*` or `filter_map_*` method within a `visit_`
 /// method.
@@ -777,15 +779,6 @@ fn visit_defaultness<T: MutVisitor>(vis: &mut T, defaultness: &mut Defaultness)
     }
 }
 
-// No `noop_` prefix because there isn't a corresponding method in `MutVisitor`.
-fn visit_safety<T: MutVisitor>(vis: &mut T, safety: &mut Safety) {
-    match safety {
-        Safety::Unsafe(span) => vis.visit_span(span),
-        Safety::Safe(span) => vis.visit_span(span),
-        Safety::Default => {}
-    }
-}
-
 // No `noop_` prefix because there isn't a corresponding method in `MutVisitor`.
 fn visit_polarity<T: MutVisitor>(vis: &mut T, polarity: &mut ImplPolarity) {
     match polarity {
@@ -794,14 +787,6 @@ fn visit_polarity<T: MutVisitor>(vis: &mut T, polarity: &mut ImplPolarity) {
     }
 }
 
-// No `noop_` prefix because there isn't a corresponding method in `MutVisitor`.
-fn visit_constness<T: MutVisitor>(vis: &mut T, constness: &mut Const) {
-    match constness {
-        Const::Yes(span) => vis.visit_span(span),
-        Const::No => {}
-    }
-}
-
 fn walk_closure_binder<T: MutVisitor>(vis: &mut T, binder: &mut ClosureBinder) {
     match binder {
         ClosureBinder::NotPresent => {}
@@ -940,15 +925,6 @@ pub fn walk_flat_map_generic_param<T: MutVisitor>(
     smallvec![param]
 }
 
-fn walk_label<T: MutVisitor>(vis: &mut T, Label { ident }: &mut Label) {
-    vis.visit_ident(ident);
-}
-
-fn walk_lifetime<T: MutVisitor>(vis: &mut T, Lifetime { id, ident }: &mut Lifetime) {
-    vis.visit_id(id);
-    vis.visit_ident(ident);
-}
-
 fn walk_generics<T: MutVisitor>(vis: &mut T, generics: &mut Generics) {
     let Generics { params, where_clause, span } = generics;
     params.flat_map_in_place(|param| vis.flat_map_generic_param(param));
@@ -1340,13 +1316,6 @@ fn walk_const_item<T: MutVisitor>(vis: &mut T, item: &mut ConstItem) {
     walk_define_opaques(vis, define_opaque);
 }
 
-fn walk_fn_header<T: MutVisitor>(vis: &mut T, header: &mut FnHeader) {
-    let FnHeader { safety, coroutine_kind, constness, ext: _ } = header;
-    visit_constness(vis, constness);
-    coroutine_kind.as_mut().map(|coroutine_kind| vis.visit_coroutine_kind(coroutine_kind));
-    visit_safety(vis, safety);
-}
-
 pub fn walk_crate<T: MutVisitor>(vis: &mut T, krate: &mut Crate) {
     let Crate { attrs, items, spans, id, is_placeholder: _ } = krate;
     vis.visit_id(id);

compiler/rustc_ast/src/visit.rs

@@ -315,6 +315,75 @@ pub trait Visitor<'ast>: Sized {
     }
 }
 
+#[macro_export]
+macro_rules! common_visitor_and_walkers {
+    ($(($mut: ident))? $Visitor:ident$(<$lt:lifetime>)?) => {
+        // this is only used by the MutVisitor. We include this symmetry here to make writing other functions easier
+        $(${ignore($lt)}
+            #[expect(unused, rustc::pass_by_value)]
+            #[inline]
+        )?
+        fn visit_span<$($lt,)? V: $Visitor$(<$lt>)?>(visitor: &mut V, span: &$($lt)? $($mut)? Span) $(-> <V as Visitor<$lt>>::Result)? {
+            $(
+                let _ = stringify!($mut);
+                visitor.visit_span(span);
+            )?
+            $(${ignore($lt)}V::Result::output())?
+        }
+
+        // this is only used by the MutVisitor. We include this symmetry here to make writing other functions easier
+        $(${ignore($lt)}
+            #[expect(unused, rustc::pass_by_value)]
+            #[inline]
+        )?
+        fn visit_id<$($lt,)? V: $Visitor$(<$lt>)?>(visitor: &mut V, id: &$($lt)? $($mut)? NodeId) $(-> <V as Visitor<$lt>>::Result)? {
+            $(
+                let _ = stringify!($mut);
+                visitor.visit_id(id);
+            )?
+            $(${ignore($lt)}V::Result::output())?
+        }
+
+        // this is only used by the MutVisitor. We include this symmetry here to make writing other functions easier
+        fn visit_safety<$($lt,)? V: $Visitor$(<$lt>)?>(vis: &mut V, safety: &$($lt)? $($mut)? Safety) $(-> <V as Visitor<$lt>>::Result)? {
+            match safety {
+                Safety::Unsafe(span) => visit_span(vis, span),
+                Safety::Safe(span) => visit_span(vis, span),
+                Safety::Default => { $(${ignore($lt)}V::Result::output())? }
+            }
+        }
+
+        fn visit_constness<$($lt,)? V: $Visitor$(<$lt>)?>(vis: &mut V, constness: &$($lt)? $($mut)? Const) $(-> <V as Visitor<$lt>>::Result)? {
+            match constness {
+                Const::Yes(span) => visit_span(vis, span),
+                Const::No => {
+                    $(<V as Visitor<$lt>>::Result::output())?
+                }
+            }
+        }
+
+        pub fn walk_label<$($lt,)? V: $Visitor$(<$lt>)?>(visitor: &mut V, Label { ident }: &$($lt)? $($mut)? Label) $(-> <V as Visitor<$lt>>::Result)? {
+            visitor.visit_ident(ident)
+        }
+
+        pub fn walk_fn_header<$($lt,)? V: $Visitor$(<$lt>)?>(visitor: &mut V, header: &$($lt)? $($mut)? FnHeader) $(-> <V as Visitor<$lt>>::Result)? {
+            let FnHeader { safety, coroutine_kind, constness, ext: _ } = header;
+            try_visit!(visit_constness(visitor, constness));
+            if let Some(coroutine_kind) = coroutine_kind {
+                try_visit!(visitor.visit_coroutine_kind(coroutine_kind));
+            }
+            visit_safety(visitor, safety)
+        }
+
+        pub fn walk_lifetime<$($lt,)? V: $Visitor$(<$lt>)?>(visitor: &mut V, Lifetime { id, ident }: &$($lt)? $($mut)? Lifetime) $(-> <V as Visitor<$lt>>::Result)? {
+            try_visit!(visit_id(visitor, id));
+            visitor.visit_ident(ident)
+        }
+    };
+}
+
+common_visitor_and_walkers!(Visitor<'a>);
+
 pub fn walk_crate<'a, V: Visitor<'a>>(visitor: &mut V, krate: &'a Crate) -> V::Result {
     let Crate { attrs, items, spans: _, id: _, is_placeholder: _ } = krate;
     walk_list!(visitor, visit_attribute, attrs);
@@ -334,15 +403,6 @@ pub fn walk_local<'a, V: Visitor<'a>>(visitor: &mut V, local: &'a Local) -> V::R
     V::Result::output()
 }
 
-pub fn walk_label<'a, V: Visitor<'a>>(visitor: &mut V, Label { ident }: &'a Label) -> V::Result {
-    visitor.visit_ident(ident)
-}
-
-pub fn walk_lifetime<'a, V: Visitor<'a>>(visitor: &mut V, lifetime: &'a Lifetime) -> V::Result {
-    let Lifetime { id: _, ident } = lifetime;
-    visitor.visit_ident(ident)
-}
-
 pub fn walk_poly_trait_ref<'a, V>(visitor: &mut V, trait_ref: &'a PolyTraitRef) -> V::Result
 where
     V: Visitor<'a>,
@@ -926,12 +986,6 @@ pub fn walk_fn_ret_ty<'a, V: Visitor<'a>>(visitor: &mut V, ret_ty: &'a FnRetTy)
     V::Result::output()
 }
 
-pub fn walk_fn_header<'a, V: Visitor<'a>>(visitor: &mut V, fn_header: &'a FnHeader) -> V::Result {
-    let FnHeader { safety: _, coroutine_kind, constness: _, ext: _ } = fn_header;
-    visit_opt!(visitor, visit_coroutine_kind, coroutine_kind.as_ref());
-    V::Result::output()
-}
-
 pub fn walk_fn_decl<'a, V: Visitor<'a>>(
     visitor: &mut V,
     FnDecl { inputs, output }: &'a FnDecl,

compiler/rustc_interface/src/passes.rs

@@ -282,6 +282,7 @@ fn configure_and_expand(
     resolver.resolve_crate(&krate);
 
     CStore::from_tcx(tcx).report_incompatible_target_modifiers(tcx, &krate);
+    CStore::from_tcx(tcx).report_incompatible_async_drop_feature(tcx, &krate);
     krate
 }
 

compiler/rustc_metadata/messages.ftl

@@ -1,6 +1,10 @@
 metadata_as_needed_compatibility =
     linking modifier `as-needed` is only compatible with `dylib` and `framework` linking kinds
 
+metadata_async_drop_types_in_dependency =
+    found async drop types in dependecy `{$extern_crate}`, but async_drop feature is disabled for `{$local_crate}`
+    .help = if async drop type will be dropped in a crate without `feature(async_drop)`, sync Drop will be used
+
 metadata_bad_panic_strategy =
     the linked panic runtime `{$runtime}` is not compiled with this crate's panic strategy `{$strategy}`
 

compiler/rustc_metadata/src/creader.rs

@@ -473,6 +473,27 @@ impl CStore {
         }
     }
 
+    // Report about async drop types in dependency if async drop feature is disabled
+    pub fn report_incompatible_async_drop_feature(&self, tcx: TyCtxt<'_>, krate: &Crate) {
+        if tcx.features().async_drop() {
+            return;
+        }
+        for (_cnum, data) in self.iter_crate_data() {
+            if data.is_proc_macro_crate() {
+                continue;
+            }
+            if data.has_async_drops() {
+                let extern_crate = data.name();
+                let local_crate = tcx.crate_name(LOCAL_CRATE);
+                tcx.dcx().emit_warn(errors::AsyncDropTypesInDependency {
+                    span: krate.spans.inner_span.shrink_to_lo(),
+                    extern_crate,
+                    local_crate,
+                });
+            }
+        }
+    }
+
     pub fn new(metadata_loader: Box<MetadataLoaderDyn>) -> CStore {
         CStore {
             metadata_loader,

compiler/rustc_metadata/src/errors.rs

@@ -811,3 +811,13 @@ pub struct UnknownTargetModifierUnsafeAllowed {
     pub span: Span,
     pub flag_name: String,
 }
+
+#[derive(Diagnostic)]
+#[diag(metadata_async_drop_types_in_dependency)]
+#[help]
+pub struct AsyncDropTypesInDependency {
+    #[primary_span]
+    pub span: Span,
+    pub extern_crate: Symbol,
+    pub local_crate: Symbol,
+}

compiler/rustc_metadata/src/rmeta/decoder.rs

@@ -1984,6 +1984,10 @@ impl CrateMetadata {
         self.root.header.hash
     }
 
+    pub(crate) fn has_async_drops(&self) -> bool {
+        self.root.tables.adt_async_destructor.len > 0
+    }
+
     fn num_def_ids(&self) -> usize {
         self.root.tables.def_keys.size()
     }

compiler/rustc_query_system/src/dep_graph/graph.rs

@@ -1433,6 +1433,8 @@ fn panic_on_forbidden_read<D: Deps>(data: &DepGraphData<D>, dep_node_index: DepN
         && let Some(nodes) = &data.current.nodes_in_current_session
     {
         // Try to find it among the nodes allocated so far in this session
+        // This is OK, there's only ever one node result possible so this is deterministic.
+        #[allow(rustc::potential_query_instability)]
         if let Some((node, _)) = nodes.lock().iter().find(|&(_, index)| *index == dep_node_index) {
             dep_node = Some(*node);
         }

compiler/rustc_query_system/src/dep_graph/serialized.rs

@@ -784,6 +784,8 @@ impl<D: Deps> EncoderState<D> {
     ) {
         if let Some(record_stats) = &self.stats {
             let record_stats = record_stats.lock();
+            // `stats` is sorted below so we can allow this lint here.
+            #[allow(rustc::potential_query_instability)]
             let mut stats: Vec<_> = record_stats.values().collect();
             stats.sort_by_key(|s| -(s.node_counter as i64));
 

compiler/rustc_query_system/src/lib.rs

@@ -1,5 +1,5 @@
 // tidy-alphabetical-start
-#![allow(rustc::potential_query_instability, internal_features)]
+#![allow(internal_features)]
 #![feature(assert_matches)]
 #![feature(core_intrinsics)]
 #![feature(dropck_eyepatch)]

compiler/rustc_query_system/src/query/job.rs

@@ -510,6 +510,10 @@ pub fn break_query_cycles<I: Clone + Debug>(
     registry: &rayon_core::Registry,
 ) {
     let mut wakelist = Vec::new();
+    // It is OK per the comments:
+    // - https://github.com/rust-lang/rust/pull/131200#issuecomment-2798854932
+    // - https://github.com/rust-lang/rust/pull/131200#issuecomment-2798866392
+    #[allow(rustc::potential_query_instability)]
     let mut jobs: Vec<QueryJobId> = query_map.keys().cloned().collect();
 
     let mut found_cycle = false;

library/std/src/sync/reentrant_lock.rs

@@ -136,7 +136,7 @@ cfg_if!(
             // we only ever read from the tid if `tls_addr` matches the current
             // TLS address. In that case, either the tid has been set by
             // the current thread, or by a thread that has terminated before
-            // the current thread was created. In either case, no further
+            // the current thread's `tls_addr` was allocated. In either case, no further
             // synchronization is needed (as per <https://github.com/rust-lang/miri/issues/3450>)
             tls_addr: Atomic<usize>,
             tid: UnsafeCell<u64>,
@@ -154,8 +154,12 @@ cfg_if!(
             // NOTE: This assumes that `owner` is the ID of the current
             // thread, and may spuriously return `false` if that's not the case.
             fn contains(&self, owner: ThreadId) -> bool {
+                // We must call `tls_addr()` *before* doing the load to ensure that if we reuse an
+                // earlier thread's address, the `tls_addr.load()` below happens-after everything
+                // that thread did.
+                let tls_addr = tls_addr();
                 // SAFETY: See the comments in the struct definition.
-                self.tls_addr.load(Ordering::Relaxed) == tls_addr()
+                self.tls_addr.load(Ordering::Relaxed) == tls_addr
                     && unsafe { *self.tid.get() } == owner.as_u64().get()
             }
 

library/std/src/sys/pal/windows/os.rs

@@ -202,6 +202,8 @@ fn home_dir_crt() -> Option<PathBuf> {
             |buf, mut sz| {
                 // GetUserProfileDirectoryW does not quite use the usual protocol for
                 // negotiating the buffer size, so we have to translate.
+                // FIXME(#141254): We rely on the *undocumented* property that this function will
+                // always set the size, not just on failure.
                 match c::GetUserProfileDirectoryW(
                     ptr::without_provenance_mut(CURRENT_PROCESS_TOKEN),
                     buf,

src/bootstrap/src/core/sanity.rs

@@ -13,14 +13,14 @@ use std::ffi::{OsStr, OsString};
 use std::path::PathBuf;
 use std::{env, fs};
 
-use crate::Build;
 #[cfg(not(test))]
 use crate::builder::Builder;
 use crate::builder::Kind;
 #[cfg(not(test))]
 use crate::core::build_steps::tool;
 use crate::core::config::Target;
 use crate::utils::exec::command;
+use crate::{Build, Subcommand};
 
 pub struct Finder {
     cache: HashMap<OsString, Option<PathBuf>>,
@@ -205,6 +205,20 @@ than building it.
     .map(|s| s.to_string())
     .collect();
 
+    // Compiler tools like `cc` and `ar` are not configured for cross-targets on certain subcommands
+    // because they are not needed.
+    //
+    // See `cc_detect::find` for more details.
+    let skip_tools_checks = build.config.dry_run()
+        || matches!(
+            build.config.cmd,
+            Subcommand::Clean { .. }
+                | Subcommand::Check { .. }
+                | Subcommand::Suggest { .. }
+                | Subcommand::Format { .. }
+                | Subcommand::Setup { .. }
+        );
+
     // We're gonna build some custom C code here and there, host triples
     // also build some C++ shims for LLVM so we need a C++ compiler.
     for target in &build.targets {
@@ -278,15 +292,15 @@ than building it.
             }
         }
 
-        if !build.config.dry_run() {
+        if !skip_tools_checks {
             cmd_finder.must_have(build.cc(*target));
             if let Some(ar) = build.ar(*target) {
                 cmd_finder.must_have(ar);
             }
         }
     }
 
-    if !build.config.dry_run() {
+    if !skip_tools_checks {
         for host in &build.hosts {
             cmd_finder.must_have(build.cxx(*host).unwrap());
 

src/tools/miri/README.md

@@ -580,6 +580,7 @@ Definite bugs found:
 * [Weak-memory-induced memory leak in Windows thread-local storage](https://github.com/rust-lang/rust/pull/124281)
 * [A bug in the new `RwLock::downgrade` implementation](https://rust-lang.zulipchat.com/#narrow/channel/269128-miri/topic/Miri.20error.20library.20test) (caught by Miri before it landed in the Rust repo)
 * [Mockall reading unintialized memory when mocking `std::io::Read::read`, even if all expectations are satisfied](https://github.com/asomers/mockall/issues/647) (caught by Miri running Tokio's test suite)
+* [`ReentrantLock` not correctly dealing with reuse of addresses for TLS storage of different threads](https://github.com/rust-lang/rust/pull/141248)
 
 Violations of [Stacked Borrows] found that are likely bugs (but Stacked Borrows is currently just an experiment):