From cc204e874465815c09b02f9b3d4dcee296ef7412 Mon Sep 17 00:00:00 2001 From: Weihang Lo Date: Fri, 7 Oct 2022 09:26:06 +0100 Subject: [PATCH] refactor(features2): new variant `FeaturesFor::ArtifactDep` As the existence of `FeaturesFor` is acting as a key that discriminate activated features for the same package with different circumstances. Artifact dependencies deserve its own variant and it makes things clear to me. - `NormalOrDevOrArtifactTarget(None)` -> `NormalOrDev` - `NormalOrDevOrArtifactTarget(Some(Target))` -> `ArtifactDep(Target)` --- src/cargo/core/compiler/standard_lib.rs | 5 +-- src/cargo/core/resolver/features.rs | 45 ++++++++++++------------- src/cargo/ops/cargo_compile.rs | 2 +- src/cargo/ops/tree/graph.rs | 4 +-- 4 files changed, 25 insertions(+), 31 deletions(-) diff --git a/src/cargo/core/compiler/standard_lib.rs b/src/cargo/core/compiler/standard_lib.rs index e0baebd516b..d1ff7fbd343 100644 --- a/src/cargo/core/compiler/standard_lib.rs +++ b/src/cargo/core/compiler/standard_lib.rs @@ -193,10 +193,7 @@ pub fn generate_std_roots( // in time is minimal, and the difference in caching is // significant. let mode = CompileMode::Build; - let features = std_features.activated_features( - pkg.package_id(), - FeaturesFor::NormalOrDevOrArtifactTarget(None), - ); + let features = std_features.activated_features(pkg.package_id(), FeaturesFor::NormalOrDev); for kind in kinds { let list = ret.entry(*kind).or_insert_with(Vec::new); let unit_for = UnitFor::new_normal(*kind); diff --git a/src/cargo/core/resolver/features.rs b/src/cargo/core/resolver/features.rs index 25af117a508..85ba25453ff 100644 --- a/src/cargo/core/resolver/features.rs +++ b/src/cargo/core/resolver/features.rs @@ -95,30 +95,29 @@ pub enum ForceAllTargets { No, } -/// Flag to indicate if features are requested for a build dependency or not. -#[derive(Copy, Clone, Debug, PartialEq, Eq, Ord, PartialOrd, Hash)] +/// Flag to indicate if features are requested for a certain type of dependency. +/// +/// This is primarily used for constructing a [`PackageFeaturesKey`] to decouple +/// activated features of the same package with different types of dependency. +#[derive(Default, Copy, Clone, Debug, PartialEq, Eq, Ord, PartialOrd, Hash)] pub enum FeaturesFor { - /// If `Some(target)` is present, we represent an artifact target. - /// Otherwise any other normal or dev dependency. - NormalOrDevOrArtifactTarget(Option), + /// Normal or dev dependency. + #[default] + NormalOrDev, /// Build dependency or proc-macro. HostDep, -} - -impl Default for FeaturesFor { - fn default() -> Self { - FeaturesFor::NormalOrDevOrArtifactTarget(None) - } + /// Any dependency with both artifact and target specified. + /// + /// That is, `dep = { …, artifact = , target = }` + ArtifactDep(CompileTarget), } impl std::fmt::Display for FeaturesFor { fn fmt(&self, f: &mut std::fmt::Formatter<'_>) -> std::fmt::Result { match self { FeaturesFor::HostDep => f.write_str("host"), - FeaturesFor::NormalOrDevOrArtifactTarget(Some(target)) => { - f.write_str(&target.rustc_target()) - } - FeaturesFor::NormalOrDevOrArtifactTarget(None) => Ok(()), + FeaturesFor::ArtifactDep(target) => f.write_str(&target.rustc_target()), + FeaturesFor::NormalOrDev => Ok(()), } } } @@ -128,7 +127,7 @@ impl FeaturesFor { if for_host { FeaturesFor::HostDep } else { - FeaturesFor::NormalOrDevOrArtifactTarget(None) + FeaturesFor::NormalOrDev } } @@ -137,12 +136,12 @@ impl FeaturesFor { artifact_target: Option, ) -> FeaturesFor { match artifact_target { - Some(target) => FeaturesFor::NormalOrDevOrArtifactTarget(Some(target)), + Some(target) => FeaturesFor::ArtifactDep(target), None => { if for_host { FeaturesFor::HostDep } else { - FeaturesFor::NormalOrDevOrArtifactTarget(None) + FeaturesFor::NormalOrDev } } } @@ -769,11 +768,11 @@ impl<'a, 'cfg> FeatureResolver<'a, 'cfg> { self.target_data .dep_platform_activated(dep, CompileKind::Host) } - (_, FeaturesFor::NormalOrDevOrArtifactTarget(None)) => self + (_, FeaturesFor::NormalOrDev) => self .requested_targets .iter() .any(|kind| self.target_data.dep_platform_activated(dep, *kind)), - (_, FeaturesFor::NormalOrDevOrArtifactTarget(Some(target))) => self + (_, FeaturesFor::ArtifactDep(target)) => self .target_data .dep_platform_activated(dep, CompileKind::Target(target)), } @@ -832,7 +831,7 @@ impl<'a, 'cfg> FeatureResolver<'a, 'cfg> { artifact.is_lib(), artifact.target().map(|target| match target { ArtifactTarget::Force(target) => { - vec![FeaturesFor::NormalOrDevOrArtifactTarget(Some(target))] + vec![FeaturesFor::ArtifactDep(target)] } ArtifactTarget::BuildDependencyAssumeTarget => self .requested_targets @@ -840,9 +839,7 @@ impl<'a, 'cfg> FeatureResolver<'a, 'cfg> { .filter_map(|kind| match kind { CompileKind::Host => None, CompileKind::Target(target) => { - Some(FeaturesFor::NormalOrDevOrArtifactTarget( - Some(*target), - )) + Some(FeaturesFor::ArtifactDep(*target)) } }) .collect(), diff --git a/src/cargo/ops/cargo_compile.rs b/src/cargo/ops/cargo_compile.rs index 9cca2ee3099..6389434009f 100644 --- a/src/cargo/ops/cargo_compile.rs +++ b/src/cargo/ops/cargo_compile.rs @@ -1424,7 +1424,7 @@ pub fn resolve_all_features( package_id: PackageId, ) -> HashSet { let mut features: HashSet = resolved_features - .activated_features(package_id, FeaturesFor::NormalOrDevOrArtifactTarget(None)) + .activated_features(package_id, FeaturesFor::NormalOrDev) .iter() .map(|s| s.to_string()) .collect(); diff --git a/src/cargo/ops/tree/graph.rs b/src/cargo/ops/tree/graph.rs index ebe63f2a496..20a9ca0b657 100644 --- a/src/cargo/ops/tree/graph.rs +++ b/src/cargo/ops/tree/graph.rs @@ -325,8 +325,8 @@ fn add_pkg( let node_features = resolved_features.activated_features(package_id, features_for); let node_kind = match features_for { FeaturesFor::HostDep => CompileKind::Host, - FeaturesFor::NormalOrDevOrArtifactTarget(Some(target)) => CompileKind::Target(target), - FeaturesFor::NormalOrDevOrArtifactTarget(None) => requested_kind, + FeaturesFor::ArtifactDep(target) => CompileKind::Target(target), + FeaturesFor::NormalOrDev => requested_kind, }; let node = Node::Package { package_id,