Rewrite new feature resolver to simplify DepKind handling.

Now that dev-dependencies are a global setting, only build-dependencies
matter. This is maybe a little simpler to understand?

Author: ehuss

src/cargo/core/compiler/context/compilation_files.rs

@@ -147,7 +147,7 @@ impl<'a, 'cfg: 'a> CompilationFiles<'a, 'cfg> {
     /// Returns `None` if the unit should not use a metadata data hash (like
     /// rustdoc, or some dylibs).
     pub fn metadata(&self, unit: &Unit<'a>) -> Option<Metadata> {
-        self.metas[unit].clone()
+        self.metas[unit]
     }
 
     /// Gets the short hash based only on the `PackageId`.
@@ -515,7 +515,7 @@ fn metadata_of<'a, 'cfg>(
             metadata_of(&dep.unit, cx, metas);
         }
     }
-    metas[unit].clone()
+    metas[unit]
 }
 
 fn compute_metadata<'a, 'cfg>(

src/cargo/core/compiler/standard_lib.rs

@@ -1,7 +1,6 @@
 //! Code for building the standard library.
 
 use crate::core::compiler::{BuildContext, CompileKind, CompileMode, RustcTargetData, Unit};
-use crate::core::dependency::DepKind;
 use crate::core::profiles::UnitFor;
 use crate::core::resolver::features::ResolvedFeatures;
 use crate::core::resolver::ResolveOpts;
@@ -149,11 +148,7 @@ pub fn generate_std_roots<'a>(
                 unit_for,
                 mode,
             );
-            let features = std_features.activated_features(
-                pkg.package_id(),
-                DepKind::Normal,
-                bcx.build_config.requested_kind,
-            );
+            let features = std_features.activated_features(pkg.package_id(), false);
             Ok(bcx.units.intern(
                 pkg, lib, profile, kind, mode, features, /*is_std*/ true,
             ))

src/cargo/core/compiler/unit_dependencies.rs

@@ -178,7 +178,7 @@ fn deps_of_roots<'a, 'cfg>(roots: &[Unit<'a>], mut state: &mut State<'a, 'cfg>)
             } else if unit.target.is_custom_build() {
                 // This normally doesn't happen, except `clean` aggressively
                 // generates all units.
-                UnitFor::new_build()
+                UnitFor::new_build(false)
             } else if unit.target.for_host() {
                 // Proc macro / plugin should never have panic set.
                 UnitFor::new_compiler()
@@ -230,53 +230,48 @@ fn compute_deps<'a, 'cfg>(
     unit_for: UnitFor,
 ) -> CargoResult<Vec<UnitDep<'a>>> {
     if unit.mode.is_run_custom_build() {
-        return compute_deps_custom_build(unit, unit_for.dep_kind(), state);
+        return compute_deps_custom_build(unit, unit_for, state);
     } else if unit.mode.is_doc() {
         // Note: this does not include doc test.
         return compute_deps_doc(unit, state);
     }
 
     let bcx = state.bcx;
     let id = unit.pkg.package_id();
-    let filtered_deps = state
-        .resolve()
-        .deps(id)
-        .map(|(id, deps)| {
-            assert!(!deps.is_empty());
-            let filtered_deps = deps.iter().filter(|dep| {
-                // If this target is a build command, then we only want build
-                // dependencies, otherwise we want everything *other than* build
-                // dependencies.
-                if unit.target.is_custom_build() != dep.is_build() {
-                    return false;
-                }
+    let filtered_deps = state.resolve().deps(id).filter(|&(_id, deps)| {
+        assert!(!deps.is_empty());
+        deps.iter().any(|dep| {
+            // If this target is a build command, then we only want build
+            // dependencies, otherwise we want everything *other than* build
+            // dependencies.
+            if unit.target.is_custom_build() != dep.is_build() {
+                return false;
+            }
 
-                // If this dependency is **not** a transitive dependency, then it
-                // only applies to test/example targets.
-                if !dep.is_transitive()
-                    && !unit.target.is_test()
-                    && !unit.target.is_example()
-                    && !unit.mode.is_any_test()
-                {
-                    return false;
-                }
+            // If this dependency is **not** a transitive dependency, then it
+            // only applies to test/example targets.
+            if !dep.is_transitive()
+                && !unit.target.is_test()
+                && !unit.target.is_example()
+                && !unit.mode.is_any_test()
+            {
+                return false;
+            }
 
-                // If this dependency is only available for certain platforms,
-                // make sure we're only enabling it for that platform.
-                if !bcx.target_data.dep_platform_activated(dep, unit.kind) {
-                    return false;
-                }
+            // If this dependency is only available for certain platforms,
+            // make sure we're only enabling it for that platform.
+            if !bcx.target_data.dep_platform_activated(dep, unit.kind) {
+                return false;
+            }
 
-                // If we've gotten past all that, then this dependency is
-                // actually used!
-                true
-            });
-            (id, filtered_deps.collect::<Vec<_>>())
+            // If we've gotten past all that, then this dependency is
+            // actually used!
+            true
         })
-        .filter(|(_id, deps)| !deps.is_empty());
+    });
 
     let mut ret = Vec::new();
-    for (id, deps) in filtered_deps {
+    for (id, _) in filtered_deps {
         let pkg = match state.get(id)? {
             Some(pkg) => pkg,
             None => continue,
@@ -286,23 +281,10 @@ fn compute_deps<'a, 'cfg>(
             None => continue,
         };
         let mode = check_or_build_mode(unit.mode, lib);
-        let dep_kind = if unit.target.is_custom_build() {
-            // Custom build scripts can *only* have build-dependencies.
-            DepKind::Build
-        } else if deps.iter().any(|dep| !dep.is_transitive()) {
-            // A dependency can be listed in both [dependencies] and
-            // [dev-dependencies], so this checks if any of the deps are
-            // listed in dev-dependencies. Note that `filtered_deps` has
-            // already removed dev-dependencies if it is not a
-            // test/bench/example, so it is not necessary to check `unit`
-            // here.
-            DepKind::Development
-        } else {
-            DepKind::Normal
-        };
         let dep_unit_for = unit_for
             .with_for_host(lib.for_host())
-            .with_dep_kind(dep_kind);
+            // If it is a custom build script, then it *only* has build dependencies.
+            .with_build_dep(unit.target.is_custom_build());
 
         if bcx.config.cli_unstable().dual_proc_macros && lib.proc_macro() && !unit.kind.is_host() {
             let unit_dep = new_unit_dep(state, unit, pkg, lib, dep_unit_for, unit.kind, mode)?;
@@ -330,7 +312,7 @@ fn compute_deps<'a, 'cfg>(
     if unit.target.is_custom_build() {
         return Ok(ret);
     }
-    ret.extend(dep_build_script(unit, unit_for.dep_kind(), state)?);
+    ret.extend(dep_build_script(unit, unit_for, state)?);
 
     // If this target is a binary, test, example, etc, then it depends on
     // the library of the same package. The call to `resolve.deps` above
@@ -382,14 +364,9 @@ fn compute_deps<'a, 'cfg>(
 ///
 /// The `unit` provided must represent an execution of a build script, and
 /// the returned set of units must all be run before `unit` is run.
-///
-/// `dep_kind` is the dependency kind of the package this build script is
-/// being built for. This ensures that the build script is built with the same
-/// features the package is built with (if the build script has cfg!()
-/// checks).
 fn compute_deps_custom_build<'a, 'cfg>(
     unit: &Unit<'a>,
-    dep_kind: DepKind,
+    unit_for: UnitFor,
     state: &mut State<'a, 'cfg>,
 ) -> CargoResult<Vec<UnitDep<'a>>> {
     if let Some(links) = unit.pkg.manifest().links() {
@@ -400,7 +377,7 @@ fn compute_deps_custom_build<'a, 'cfg>(
     }
     // All dependencies of this unit should use profiles for custom
     // builds.
-    let unit_for = UnitFor::new_build().with_dep_kind(dep_kind);
+    let script_unit_for = UnitFor::new_build(unit_for.is_for_build_dep());
     // When not overridden, then the dependencies to run a build script are:
     //
     // 1. Compiling the build script itself.
@@ -415,7 +392,7 @@ fn compute_deps_custom_build<'a, 'cfg>(
         unit,
         unit.pkg,
         unit.target,
-        unit_for,
+        script_unit_for,
         // Build scripts always compiled for the host.
         CompileKind::Host,
         CompileMode::Build,
@@ -482,7 +459,7 @@ fn compute_deps_doc<'a, 'cfg>(
     }
 
     // Be sure to build/run the build script for documented libraries.
-    ret.extend(dep_build_script(unit, DepKind::Normal, state)?);
+    ret.extend(dep_build_script(unit, UnitFor::new_normal(), state)?);
 
     // If we document a binary/example, we need the library available.
     if unit.target.is_bin() || unit.target.is_example() {
@@ -524,7 +501,7 @@ fn maybe_lib<'a>(
 /// build script.
 fn dep_build_script<'a>(
     unit: &Unit<'a>,
-    dep_kind: DepKind,
+    unit_for: UnitFor,
     state: &State<'a, '_>,
 ) -> CargoResult<Option<UnitDep<'a>>> {
     unit.pkg
@@ -538,7 +515,7 @@ fn dep_build_script<'a>(
                 .bcx
                 .profiles
                 .get_profile_run_custom_build(&unit.profile);
-            let script_unit_for = UnitFor::new_build().with_dep_kind(dep_kind);
+            let script_unit_for = UnitFor::new_build(unit_for.is_for_build_dep());
             new_unit_dep_with_profile(
                 state,
                 unit,
@@ -609,7 +586,7 @@ fn new_unit_dep_with_profile<'a>(
     let public = state
         .resolve()
         .is_public_dep(parent.pkg.package_id(), pkg.package_id());
-    let features = state.activated_features(pkg.package_id(), unit_for.dep_kind(), kind);
+    let features = state.activated_features(pkg.package_id(), unit_for.is_for_build_dep());
     let unit = state
         .bcx
         .units
@@ -714,18 +691,13 @@ impl<'a, 'cfg> State<'a, 'cfg> {
         }
     }
 
-    fn activated_features(
-        &self,
-        pkg_id: PackageId,
-        dep_kind: DepKind,
-        compile_kind: CompileKind,
-    ) -> Vec<InternedString> {
+    fn activated_features(&self, pkg_id: PackageId, for_build_dep: bool) -> Vec<InternedString> {
         let features = if self.is_std {
             self.std_features.unwrap()
         } else {
             self.usr_features
         };
-        features.activated_features(pkg_id, dep_kind, compile_kind)
+        features.activated_features(pkg_id, for_build_dep)
     }
 
     fn get(&mut self, id: PackageId) -> CargoResult<Option<&'a Package>> {

src/cargo/core/dependency.rs

@@ -92,23 +92,6 @@ pub enum DepKind {
     Build,
 }
 
-impl DepKind {
-    /// Convert a DepKind from a package to one of its dependencies.
-    ///
-    /// The rules here determine how feature decoupling works. This works in
-    /// conjunction with the new feature resolver.
-    pub fn sticky_kind(&self, to: DepKind) -> DepKind {
-        use DepKind::*;
-        match (self, to) {
-            (Normal, _) => to,
-            (Build, _) => Build,
-            (Development, Normal) => Development,
-            (Development, Build) => Build,
-            (Development, Development) => Development,
-        }
-    }
-}
-
 fn parse_req_with_deprecated(
     name: InternedString,
     req: &str,