From 279f2d1a98cd4bb58661f417526aab912119c34d Mon Sep 17 00:00:00 2001 From: Jacob Finkelman Date: Tue, 9 Jul 2024 17:24:25 +0000 Subject: [PATCH 1/2] make summary sync by using Arc not Rc --- src/cargo/core/dependency.rs | 38 ++++++++++++++++++------------------ src/cargo/core/summary.rs | 16 +++++++-------- 2 files changed, 27 insertions(+), 27 deletions(-) diff --git a/src/cargo/core/dependency.rs b/src/cargo/core/dependency.rs index ddd43e1f59d..12c48411983 100644 --- a/src/cargo/core/dependency.rs +++ b/src/cargo/core/dependency.rs @@ -5,7 +5,7 @@ use serde::Serialize; use std::borrow::Cow; use std::fmt; use std::path::PathBuf; -use std::rc::Rc; +use std::sync::Arc; use tracing::trace; use crate::core::compiler::{CompileKind, CompileTarget}; @@ -18,7 +18,7 @@ use crate::util::OptVersionReq; /// Cheap to copy. #[derive(PartialEq, Eq, Hash, Clone, Debug)] pub struct Dependency { - inner: Rc, + inner: Arc, } /// The data underlying a `Dependency`. @@ -152,7 +152,7 @@ impl Dependency { let mut ret = Dependency::new_override(name, source_id); { - let ptr = Rc::make_mut(&mut ret.inner); + let ptr = Arc::make_mut(&mut ret.inner); ptr.only_match_name = false; ptr.req = version_req; ptr.specified_req = specified_req; @@ -163,7 +163,7 @@ impl Dependency { pub fn new_override(name: InternedString, source_id: SourceId) -> Dependency { assert!(!name.is_empty()); Dependency { - inner: Rc::new(Inner { + inner: Arc::new(Inner { name, source_id, registry_id: None, @@ -241,7 +241,7 @@ impl Dependency { } pub fn set_registry_id(&mut self, registry_id: SourceId) -> &mut Dependency { - Rc::make_mut(&mut self.inner).registry_id = Some(registry_id); + Arc::make_mut(&mut self.inner).registry_id = Some(registry_id); self } @@ -259,7 +259,7 @@ impl Dependency { // Setting 'public' only makes sense for normal dependencies assert_eq!(self.kind(), DepKind::Normal); } - Rc::make_mut(&mut self.inner).public = public; + Arc::make_mut(&mut self.inner).public = public; self } @@ -286,7 +286,7 @@ impl Dependency { // Setting 'public' only makes sense for normal dependencies assert_eq!(kind, DepKind::Normal); } - Rc::make_mut(&mut self.inner).kind = kind; + Arc::make_mut(&mut self.inner).kind = kind; self } @@ -295,36 +295,36 @@ impl Dependency { &mut self, features: impl IntoIterator>, ) -> &mut Dependency { - Rc::make_mut(&mut self.inner).features = features.into_iter().map(|s| s.into()).collect(); + Arc::make_mut(&mut self.inner).features = features.into_iter().map(|s| s.into()).collect(); self } /// Sets whether the dependency requests default features of the package. pub fn set_default_features(&mut self, default_features: bool) -> &mut Dependency { - Rc::make_mut(&mut self.inner).default_features = default_features; + Arc::make_mut(&mut self.inner).default_features = default_features; self } /// Sets whether the dependency is optional. pub fn set_optional(&mut self, optional: bool) -> &mut Dependency { - Rc::make_mut(&mut self.inner).optional = optional; + Arc::make_mut(&mut self.inner).optional = optional; self } /// Sets the source ID for this dependency. pub fn set_source_id(&mut self, id: SourceId) -> &mut Dependency { - Rc::make_mut(&mut self.inner).source_id = id; + Arc::make_mut(&mut self.inner).source_id = id; self } /// Sets the version requirement for this dependency. pub fn set_version_req(&mut self, req: OptVersionReq) -> &mut Dependency { - Rc::make_mut(&mut self.inner).req = req; + Arc::make_mut(&mut self.inner).req = req; self } pub fn set_platform(&mut self, platform: Option) -> &mut Dependency { - Rc::make_mut(&mut self.inner).platform = platform; + Arc::make_mut(&mut self.inner).platform = platform; self } @@ -332,7 +332,7 @@ impl Dependency { &mut self, name: impl Into, ) -> &mut Dependency { - Rc::make_mut(&mut self.inner).explicit_name_in_toml = Some(name.into()); + Arc::make_mut(&mut self.inner).explicit_name_in_toml = Some(name.into()); self } @@ -346,7 +346,7 @@ impl Dependency { self.source_id(), id ); - let me = Rc::make_mut(&mut self.inner); + let me = Arc::make_mut(&mut self.inner); me.req.lock_to(id.version()); // Only update the `precise` of this source to preserve other @@ -361,7 +361,7 @@ impl Dependency { /// Mainly used in dependency patching like `[patch]` or `[replace]`, which /// doesn't need to lock the entire dependency to a specific [`PackageId`]. pub fn lock_version(&mut self, version: &semver::Version) -> &mut Dependency { - let me = Rc::make_mut(&mut self.inner); + let me = Arc::make_mut(&mut self.inner); me.req.lock_to(version); self } @@ -430,7 +430,7 @@ impl Dependency { } pub(crate) fn set_artifact(&mut self, artifact: Artifact) { - Rc::make_mut(&mut self.inner).artifact = Some(artifact); + Arc::make_mut(&mut self.inner).artifact = Some(artifact); } pub(crate) fn artifact(&self) -> Option<&Artifact> { @@ -453,7 +453,7 @@ impl Dependency { /// This information represents a requirement in the package this dependency refers to. #[derive(PartialEq, Eq, Hash, Clone, Debug)] pub struct Artifact { - inner: Rc>, + inner: Arc>, is_lib: bool, target: Option, } @@ -492,7 +492,7 @@ impl Artifact { .collect::, _>>()?, )?; Ok(Artifact { - inner: Rc::new(kinds), + inner: Arc::new(kinds), is_lib, target: target.map(ArtifactTarget::parse).transpose()?, }) diff --git a/src/cargo/core/summary.rs b/src/cargo/core/summary.rs index c551b05b7c8..d504a91c35e 100644 --- a/src/cargo/core/summary.rs +++ b/src/cargo/core/summary.rs @@ -9,7 +9,7 @@ use std::collections::{BTreeMap, HashMap, HashSet}; use std::fmt; use std::hash::{Hash, Hasher}; use std::mem; -use std::rc::Rc; +use std::sync::Arc; /// Subset of a `Manifest`. Contains only the most important information about /// a package. @@ -17,14 +17,14 @@ use std::rc::Rc; /// Summaries are cloned, and should not be mutated after creation #[derive(Debug, Clone)] pub struct Summary { - inner: Rc, + inner: Arc, } #[derive(Debug, Clone)] struct Inner { package_id: PackageId, dependencies: Vec, - features: Rc, + features: Arc, checksum: Option, links: Option, rust_version: Option, @@ -82,10 +82,10 @@ impl Summary { } let feature_map = build_feature_map(features, &dependencies)?; Ok(Summary { - inner: Rc::new(Inner { + inner: Arc::new(Inner { package_id: pkg_id, dependencies, - features: Rc::new(feature_map), + features: Arc::new(feature_map), checksum: None, links: links.map(|l| l.into()), rust_version, @@ -124,12 +124,12 @@ impl Summary { } pub fn override_id(mut self, id: PackageId) -> Summary { - Rc::make_mut(&mut self.inner).package_id = id; + Arc::make_mut(&mut self.inner).package_id = id; self } pub fn set_checksum(&mut self, cksum: String) { - Rc::make_mut(&mut self.inner).checksum = Some(cksum); + Arc::make_mut(&mut self.inner).checksum = Some(cksum); } pub fn map_dependencies(self, mut f: F) -> Summary @@ -144,7 +144,7 @@ impl Summary { F: FnMut(Dependency) -> CargoResult, { { - let slot = &mut Rc::make_mut(&mut self.inner).dependencies; + let slot = &mut Arc::make_mut(&mut self.inner).dependencies; *slot = mem::take(slot) .into_iter() .map(f) From c27579a584c4b2622260b81314a56997f394de0e Mon Sep 17 00:00:00 2001 From: Jacob Finkelman Date: Wed, 17 Jul 2024 14:34:34 +0000 Subject: [PATCH 2/2] add a check that Summary is Sync --- src/cargo/core/summary.rs | 6 ++++++ 1 file changed, 6 insertions(+) diff --git a/src/cargo/core/summary.rs b/src/cargo/core/summary.rs index d504a91c35e..424328fa6d1 100644 --- a/src/cargo/core/summary.rs +++ b/src/cargo/core/summary.rs @@ -178,6 +178,12 @@ impl Hash for Summary { } } +// A check that only compiles if Summary is Sync +const _: fn() = || { + fn is_sync() {} + is_sync::(); +}; + /// Checks features for errors, bailing out a CargoResult:Err if invalid, /// and creates FeatureValues for each feature. fn build_feature_map(