Generalize the non-freeze and needs_drop handling.

compiler/rustc_metadata/src/rmeta/mod.rs

@@ -24,12 +24,13 @@ use rustc_macros::{
 use rustc_middle::metadata::ModChild;
 use rustc_middle::middle::codegen_fn_attrs::CodegenFnAttrs;
 use rustc_middle::middle::debugger_visualizer::DebuggerVisualizerFile;
+use rustc_middle::middle::deduced_param_attrs::DeducedParamAttrs;
 use rustc_middle::middle::exported_symbols::{ExportedSymbol, SymbolExportInfo};
 use rustc_middle::middle::lib_features::FeatureStability;
 use rustc_middle::middle::resolve_bound_vars::ObjectLifetimeDefault;
 use rustc_middle::mir;
 use rustc_middle::ty::fast_reject::SimplifiedType;
-use rustc_middle::ty::{self, DeducedParamAttrs, Ty, TyCtxt, UnusedGenericParams};
+use rustc_middle::ty::{self, Ty, TyCtxt, UnusedGenericParams};
 use rustc_middle::util::Providers;
 use rustc_serialize::opaque::FileEncoder;
 use rustc_session::config::{SymbolManglingVersion, TargetModifier};

compiler/rustc_metadata/src/rmeta/parameterized.rs

@@ -97,6 +97,7 @@ trivially_parameterized_over_tcx! {
     rustc_middle::metadata::ModChild,
     rustc_middle::middle::codegen_fn_attrs::CodegenFnAttrs,
     rustc_middle::middle::debugger_visualizer::DebuggerVisualizerFile,
+    rustc_middle::middle::deduced_param_attrs::DeducedParamAttrs,
     rustc_middle::middle::exported_symbols::SymbolExportInfo,
     rustc_middle::middle::lib_features::FeatureStability,
     rustc_middle::middle::resolve_bound_vars::ObjectLifetimeDefault,
@@ -105,7 +106,6 @@ trivially_parameterized_over_tcx! {
     rustc_middle::ty::AssocContainer,
     rustc_middle::ty::AsyncDestructor,
     rustc_middle::ty::Asyncness,
-    rustc_middle::ty::DeducedParamAttrs,
     rustc_middle::ty::Destructor,
     rustc_middle::ty::Generics,
     rustc_middle::ty::ImplTraitInTraitData,

compiler/rustc_middle/src/middle/deduced_param_attrs.rs

@@ -0,0 +1,61 @@
+use rustc_macros::{Decodable, Encodable, HashStable};
+
+use crate::ty::{Ty, TyCtxt, TypingEnv};
+
+/// Flags that dictate how a parameter is mutated. If the flags are empty, the param is
+/// read-only. If non-empty, it is read-only with conditions.
+#[derive(Clone, Copy, PartialEq, Debug, Decodable, Encodable, HashStable)]
+pub struct DeducedReadOnlyParam(u8);
+
+bitflags::bitflags! {
+    impl DeducedReadOnlyParam: u8 {
+        /// This parameter is dropped. It is read-only if `!needs_drop`.
+        const IF_NO_DROP = 1 << 0;
+        /// This parameter is borrowed. It is read-only if `Freeze`.
+        const IF_FREEZE   = 1 << 1;
+        /// This parameter is mutated. It is never read-only.
+        const MUTATED     = 1 << 2;
+    }
+}
+
+/// Parameter attributes that can only be determined by examining the body of a function instead
+/// of just its signature.
+///
+/// These can be useful for optimization purposes when a function is directly called. We compute
+/// them and store them into the crate metadata so that downstream crates can make use of them.
+///
+/// Right now, we only have `read_only`, but `no_capture` and `no_alias` might be useful in the
+/// future.
+#[derive(Clone, Copy, PartialEq, Debug, Decodable, Encodable, HashStable)]
+pub struct DeducedParamAttrs {
+    /// The parameter is marked immutable in the function.
+    pub read_only: DeducedReadOnlyParam,
+}
+
+// By default, consider the parameters to be mutated.
+impl Default for DeducedParamAttrs {
+    fn default() -> DeducedParamAttrs {
+        DeducedParamAttrs { read_only: DeducedReadOnlyParam::MUTATED }
+    }
+}
+
+impl DeducedParamAttrs {
+    pub fn read_only<'tcx>(
+        &self,
+        tcx: TyCtxt<'tcx>,
+        typing_env: TypingEnv<'tcx>,
+        ty: Ty<'tcx>,
+    ) -> bool {
+        let read_only = self.read_only;
+        if read_only.contains(DeducedReadOnlyParam::MUTATED) {
+            return false;
+        }
+        if read_only.contains(DeducedReadOnlyParam::IF_NO_DROP) && ty.needs_drop(tcx, typing_env) {
+            return false;
+        }
+        if read_only.contains(DeducedReadOnlyParam::IF_FREEZE) && !ty.is_freeze(tcx, typing_env) {
+            return false;
+        }
+        true
+    }
+}

compiler/rustc_middle/src/middle/mod.rs

@@ -1,5 +1,6 @@
 pub mod codegen_fn_attrs;
 pub mod debugger_visualizer;
+pub mod deduced_param_attrs;
 pub mod dependency_format;
 pub mod exported_symbols;
 pub mod lang_items;

compiler/rustc_middle/src/query/erase.rs

@@ -313,6 +313,7 @@ trivial! {
     rustc_hir::Stability,
     rustc_hir::Upvar,
     rustc_index::bit_set::FiniteBitSet<u32>,
+    rustc_middle::middle::deduced_param_attrs::DeducedParamAttrs,
     rustc_middle::middle::dependency_format::Linkage,
     rustc_middle::middle::exported_symbols::SymbolExportInfo,
     rustc_middle::middle::resolve_bound_vars::ObjectLifetimeDefault,
@@ -336,7 +337,6 @@ trivial! {
     rustc_middle::ty::AsyncDestructor,
     rustc_middle::ty::BoundVariableKind,
     rustc_middle::ty::AnonConstKind,
-    rustc_middle::ty::DeducedParamAttrs,
     rustc_middle::ty::Destructor,
     rustc_middle::ty::fast_reject::SimplifiedType,
     rustc_middle::ty::ImplPolarity,

compiler/rustc_middle/src/query/mod.rs

@@ -107,6 +107,7 @@ use crate::lint::LintExpectation;
 use crate::metadata::ModChild;
 use crate::middle::codegen_fn_attrs::CodegenFnAttrs;
 use crate::middle::debugger_visualizer::DebuggerVisualizerFile;
+use crate::middle::deduced_param_attrs::DeducedParamAttrs;
 use crate::middle::exported_symbols::{ExportedSymbol, SymbolExportInfo};
 use crate::middle::lib_features::LibFeatures;
 use crate::middle::privacy::EffectiveVisibilities;
@@ -2657,7 +2658,7 @@ rustc_queries! {
         return_result_from_ensure_ok
     }
 
-    query deduced_param_attrs(def_id: DefId) -> &'tcx [ty::DeducedParamAttrs] {
+    query deduced_param_attrs(def_id: DefId) -> &'tcx [DeducedParamAttrs] {
         desc { |tcx| "deducing parameter attributes for {}", tcx.def_path_str(def_id) }
         separate_provide_extern
     }

compiler/rustc_middle/src/query/on_disk_cache.rs

@@ -799,7 +799,7 @@ impl_ref_decoder! {<'tcx>
     rustc_span::def_id::DefId,
     rustc_span::def_id::LocalDefId,
     (rustc_middle::middle::exported_symbols::ExportedSymbol<'tcx>, rustc_middle::middle::exported_symbols::SymbolExportInfo),
-    ty::DeducedParamAttrs,
+    rustc_middle::middle::deduced_param_attrs::DeducedParamAttrs,
 }
 
 //- ENCODING -------------------------------------------------------------------

compiler/rustc_middle/src/ty/codec.rs

@@ -577,7 +577,7 @@ impl_arena_copy_decoder! {<'tcx>
     rustc_span::def_id::DefId,
     rustc_span::def_id::LocalDefId,
     (rustc_middle::middle::exported_symbols::ExportedSymbol<'tcx>, rustc_middle::middle::exported_symbols::SymbolExportInfo),
-    ty::DeducedParamAttrs,
+    rustc_middle::middle::deduced_param_attrs::DeducedParamAttrs,
 }
 
 #[macro_export]

compiler/rustc_middle/src/ty/context.rs

@@ -41,7 +41,6 @@ use rustc_hir::lang_items::LangItem;
 use rustc_hir::limit::Limit;
 use rustc_hir::{self as hir, Attribute, HirId, Node, TraitCandidate, find_attr};
 use rustc_index::IndexVec;
-use rustc_macros::{HashStable, TyDecodable, TyEncodable};
 use rustc_query_system::cache::WithDepNode;
 use rustc_query_system::dep_graph::DepNodeIndex;
 use rustc_query_system::ich::StableHashingContext;
@@ -3567,21 +3566,6 @@ impl<'tcx> TyCtxt<'tcx> {
     }
 }
 
-/// Parameter attributes that can only be determined by examining the body of a function instead
-/// of just its signature.
-///
-/// These can be useful for optimization purposes when a function is directly called. We compute
-/// them and store them into the crate metadata so that downstream crates can make use of them.
-///
-/// Right now, we only have `read_only`, but `no_capture` and `no_alias` might be useful in the
-/// future.
-#[derive(Clone, Copy, PartialEq, Debug, Default, TyDecodable, TyEncodable, HashStable)]
-pub struct DeducedParamAttrs {
-    /// The parameter is marked immutable in the function and contains no `UnsafeCell` (i.e. its
-    /// type is freeze).
-    pub read_only: bool,
-}
-
 pub fn provide(providers: &mut Providers) {
     providers.is_panic_runtime =
         |tcx, LocalCrate| contains_name(tcx.hir_krate_attrs(), sym::panic_runtime);

compiler/rustc_middle/src/ty/mod.rs

@@ -78,8 +78,7 @@ pub use self::consts::{
     ExprKind, ScalarInt, UnevaluatedConst, ValTree, ValTreeKind, Value,
 };
 pub use self::context::{
-    CtxtInterners, CurrentGcx, DeducedParamAttrs, Feed, FreeRegionInfo, GlobalCtxt, Lift, TyCtxt,
-    TyCtxtFeed, tls,
+    CtxtInterners, CurrentGcx, Feed, FreeRegionInfo, GlobalCtxt, Lift, TyCtxt, TyCtxtFeed, tls,
 };
 pub use self::fold::*;
 pub use self::instance::{Instance, InstanceKind, ReifyReason, UnusedGenericParams};

compiler/rustc_mir_transform/src/deduce_param_attrs.rs

@@ -6,10 +6,11 @@
 //! dependent crates can use them.
 
 use rustc_hir::def_id::LocalDefId;
-use rustc_index::bit_set::DenseBitSet;
-use rustc_middle::mir::visit::{NonMutatingUseContext, PlaceContext, Visitor};
+use rustc_index::IndexVec;
+use rustc_middle::middle::deduced_param_attrs::{DeducedParamAttrs, DeducedReadOnlyParam};
+use rustc_middle::mir::visit::{MutatingUseContext, NonMutatingUseContext, PlaceContext, Visitor};
 use rustc_middle::mir::*;
-use rustc_middle::ty::{self, DeducedParamAttrs, Ty, TyCtxt};
+use rustc_middle::ty::{self, Ty, TyCtxt};
 use rustc_session::config::OptLevel;
 
 /// A visitor that determines which arguments have been mutated. We can't use the mutability field
@@ -18,20 +19,20 @@ struct DeduceReadOnly {
     /// Each bit is indexed by argument number, starting at zero (so 0 corresponds to local decl
     /// 1). The bit is false if the argument may have been mutated or true if we know it hasn't
     /// been up to the point we're at.
-    read_only: DenseBitSet<usize>,
+    read_only: IndexVec<usize, DeducedReadOnlyParam>,
 }
 
 impl DeduceReadOnly {
     /// Returns a new DeduceReadOnly instance.
     fn new(arg_count: usize) -> Self {
-        Self { read_only: DenseBitSet::new_filled(arg_count) }
+        Self { read_only: IndexVec::from_elem_n(DeducedReadOnlyParam::empty(), arg_count) }
     }
 
     /// Returns whether the given local is a parameter and its index.
     fn as_param(&self, local: Local) -> Option<usize> {
         // Locals and parameters are shifted by `RETURN_PLACE`.
         let param_index = local.as_usize().checked_sub(1)?;
-        if param_index < self.read_only.domain_size() { Some(param_index) } else { None }
+        if param_index < self.read_only.len() { Some(param_index) } else { None }
     }
 }
 
@@ -40,24 +41,28 @@ impl<'tcx> Visitor<'tcx> for DeduceReadOnly {
         // We're only interested in arguments.
         let Some(param_index) = self.as_param(place.local) else { return };
 
-        let mark_as_mutable = match context {
+        match context {
             // Not mutating, so it's fine.
-            PlaceContext::NonUse(..) => false,
+            PlaceContext::NonUse(..) => {}
             // Dereference is not a mutation.
-            _ if place.is_indirect_first_projection() => false,
+            _ if place.is_indirect_first_projection() => {}
+            // This is a `Drop`. It could disappear at monomorphization, so mark it specially.
+            PlaceContext::MutatingUse(MutatingUseContext::Drop) => {
+                self.read_only[param_index] |= DeducedReadOnlyParam::IF_NO_DROP;
+            }
             // This is a mutation, so mark it as such.
-            PlaceContext::MutatingUse(..) => true,
+            PlaceContext::MutatingUse(..)
             // Whether mutating though a `&raw const` is allowed is still undecided, so we
-            // disable any sketchy `readonly` optimizations for now. But we only need to do
-            // this if the pointer would point into the argument. IOW: for indirect places,
-            // like `&raw (*local).field`, this surely cannot mutate `local`.
-            PlaceContext::NonMutatingUse(NonMutatingUseContext::RawBorrow) => true,
+            // disable any sketchy `readonly` optimizations for now.
+            | PlaceContext::NonMutatingUse(NonMutatingUseContext::RawBorrow) => {
+                self.read_only[param_index] |= DeducedReadOnlyParam::MUTATED;
+            }
+            // Not mutating if the parameter is `Freeze`.
+            PlaceContext::NonMutatingUse(NonMutatingUseContext::SharedBorrow) => {
+                self.read_only[param_index] |= DeducedReadOnlyParam::IF_FREEZE;
+            }
             // Not mutating, so it's fine.
-            PlaceContext::NonMutatingUse(..) => false,
-        };
-
-        if mark_as_mutable {
-            self.read_only.remove(param_index);
+            PlaceContext::NonMutatingUse(..) => {}
         }
     }
 
@@ -90,7 +95,7 @@ impl<'tcx> Visitor<'tcx> for DeduceReadOnly {
                     && let Some(param_index) = self.as_param(place.local)
                     && !place.is_indirect_first_projection()
                 {
-                    self.read_only.remove(param_index);
+                    self.read_only[param_index] |= DeducedReadOnlyParam::MUTATED;
                 }
             }
         };
@@ -120,6 +125,7 @@ fn type_will_always_be_passed_directly(ty: Ty<'_>) -> bool {
 /// body of the function instead of just the signature. These can be useful for optimization
 /// purposes on a best-effort basis. We compute them here and store them into the crate metadata so
 /// dependent crates can use them.
+#[tracing::instrument(level = "trace", skip(tcx), ret)]
 pub(super) fn deduced_param_attrs<'tcx>(
     tcx: TyCtxt<'tcx>,
     def_id: LocalDefId,
@@ -159,18 +165,11 @@ pub(super) fn deduced_param_attrs<'tcx>(
     let body: &Body<'tcx> = tcx.optimized_mir(def_id);
     let mut deduce_read_only = DeduceReadOnly::new(body.arg_count);
     deduce_read_only.visit_body(body);
+    tracing::trace!(?deduce_read_only.read_only);
 
-    // Set the `readonly` attribute for every argument that we concluded is immutable and that
-    // contains no UnsafeCells.
-    //
-    // FIXME: This is overly conservative around generic parameters: `is_freeze()` will always
-    // return false for them. For a description of alternatives that could do a better job here,
-    // see [1].
-    //
-    // [1]: https://github.com/rust-lang/rust/pull/103172#discussion_r999139997
-    let mut deduced_param_attrs = tcx.arena.alloc_from_iter((0..body.arg_count).map(|arg_index| {
-        DeducedParamAttrs { read_only: deduce_read_only.read_only.contains(arg_index) }
-    }));
+    let mut deduced_param_attrs = tcx.arena.alloc_from_iter(
+        deduce_read_only.read_only.into_iter().map(|read_only| DeducedParamAttrs { read_only }),
+    );
 
     // Trailing parameters past the size of the `deduced_param_attrs` array are assumed to have the
     // default set of attributes, so we don't have to store them explicitly. Pop them off to save a

compiler/rustc_ty_utils/src/abi.rs

@@ -640,11 +640,10 @@ fn fn_abi_adjust_for_abi<'tcx>(
                 // we can't deduce any parameters for, so make sure the argument index is in
                 // bounds.
                 if let Some(deduced_param_attrs) = deduced_param_attrs.get(arg_idx)
-                    && deduced_param_attrs.read_only
-                    && arg.layout.ty.is_freeze(tcx, cx.typing_env)
+                    && deduced_param_attrs.read_only(tcx, cx.typing_env, arg.layout.ty)
                 {
-                    attrs.regular.insert(ArgAttribute::ReadOnly);
                     debug!("added deduced read-only attribute");
+                    attrs.regular.insert(ArgAttribute::ReadOnly);
                 }
             }
         }

tests/codegen-llvm/deduced-param-attrs.rs

@@ -68,10 +68,7 @@ pub fn use_something<T>(something: T) {
 // CHECK-SAME: %x)
 #[no_mangle]
 #[inline(never)]
-pub fn use_something_freeze<T>(x: T) {
-    // `Drop` counts as a mutable use.
-    std::mem::forget(x)
-}
+pub fn use_something_freeze<T>(x: T) {}
 
 #[no_mangle]
 pub fn forward_big_cell_container(big_cell_container: BigCellContainer) {