Auto merge of #3742 - matklad:sets-are-monoid, r=alexcrichton

Simplify feature-handling code

A neat (imo :) ) hack to use an empty `&HashSet` instead of `Option<&HashSet>`.

src/cargo/core/resolver/encode.rs

@@ -154,6 +154,7 @@ impl EncodableResolve {
 
         Ok(Resolve {
             graph: g,
+            empty_features: HashSet::new(),
             features: HashMap::new(),
             replacements: replacements,
             checksums: checksums,

src/cargo/core/resolver/mod.rs

@@ -47,6 +47,7 @@
 
 use std::cmp::Ordering;
 use std::collections::{HashSet, HashMap, BinaryHeap, BTreeMap};
+use std::iter::FromIterator;
 use std::fmt;
 use std::ops::Range;
 use std::rc::Rc;
@@ -74,6 +75,7 @@ mod encode;
 pub struct Resolve {
     graph: Graph<PackageId>,
     replacements: HashMap<PackageId, PackageId>,
+    empty_features: HashSet<String>,
     features: HashMap<PackageId, HashSet<String>>,
     checksums: HashMap<PackageId, Option<String>>,
     metadata: Metadata,
@@ -210,8 +212,14 @@ unable to verify that `{0}` is the same as when the lockfile was generated
         &self.replacements
     }
 
-    pub fn features(&self, pkg: &PackageId) -> Option<&HashSet<String>> {
-        self.features.get(pkg)
+    pub fn features(&self, pkg: &PackageId) -> &HashSet<String> {
+        self.features.get(pkg).unwrap_or(&self.empty_features)
+    }
+
+    pub fn features_sorted(&self, pkg: &PackageId) -> Vec<&str> {
+        let mut v = Vec::from_iter(self.features(pkg).iter().map(|s| s.as_ref()));
+        v.sort();
+        v
     }
 
     pub fn query(&self, spec: &str) -> CargoResult<&PackageId> {
@@ -273,6 +281,7 @@ pub fn resolve(summaries: &[(Summary, Method)],
 
     let mut resolve = Resolve {
         graph: cx.resolve_graph,
+        empty_features: HashSet::new(),
         features: cx.resolve_features,
         checksums: HashMap::new(),
         metadata: BTreeMap::new(),

src/cargo/ops/cargo_compile.rs

@@ -283,19 +283,14 @@ pub fn compile_ws<'a>(ws: &Workspace<'a>,
     fn resolve_all_features(resolve_with_overrides: &Resolve,
                             package_id: &PackageId)
                             -> HashSet<String> {
-        let mut features = match resolve_with_overrides.features(package_id) {
-            Some(all_features) => all_features.clone(),
-            None => HashSet::new(),
-        };
+        let mut features = resolve_with_overrides.features(package_id).clone();
 
         // Include features enabled for use by dependencies so targets can also use them with the
         // required-features field when deciding whether to be built or skipped.
         let deps = resolve_with_overrides.deps(package_id);
         for dep in deps {
-            if let Some(dep_features) = resolve_with_overrides.features(dep) {
-                for feature in dep_features {
-                    features.insert(dep.name().to_string() + "/" + feature);
-                }
+            for feature in resolve_with_overrides.features(dep) {
+                features.insert(dep.name().to_string() + "/" + feature);
             }
         }
 

src/cargo/ops/cargo_rustc/context.rs

@@ -401,14 +401,7 @@ impl<'a, 'cfg> Context<'a, 'cfg> {
 
         // Also mix in enabled features to our metadata. This'll ensure that
         // when changing feature sets each lib is separately cached.
-        match self.resolve.features(unit.pkg.package_id()) {
-            Some(features) => {
-                let mut feat_vec: Vec<&String> = features.iter().collect();
-                feat_vec.sort();
-                feat_vec.hash(&mut hasher);
-            }
-            None => Vec::<&String>::new().hash(&mut hasher),
-        }
+        self.resolve.features_sorted(unit.pkg.package_id()).hash(&mut hasher);
 
         // Throw in the profile we're compiling with. This helps caching
         // panic=abort and panic=unwind artifacts, additionally with various
@@ -601,11 +594,8 @@ impl<'a, 'cfg> Context<'a, 'cfg> {
 
                 // If the dependency is optional, then we're only activating it
                 // if the corresponding feature was activated
-                if d.is_optional() {
-                    match self.resolve.features(id) {
-                        Some(f) if f.contains(d.name()) => {}
-                        _ => return false,
-                    }
+                if d.is_optional() && !self.resolve.features(id).contains(d.name()) {
+                    return false;
                 }
 
                 // If we've gotten past all that, then this dependency is

src/cargo/ops/cargo_rustc/custom_build.rs

@@ -120,10 +120,8 @@ fn build_work<'a, 'cfg>(cx: &mut Context<'a, 'cfg>, unit: &Unit<'a>)
 
     // Be sure to pass along all enabled features for this package, this is the
     // last piece of statically known information that we have.
-    if let Some(features) = cx.resolve.features(unit.pkg.package_id()) {
-        for feat in features.iter() {
-            cmd.env(&format!("CARGO_FEATURE_{}", super::envify(feat)), "1");
-        }
+    for feat in cx.resolve.features(unit.pkg.package_id()).iter() {
+        cmd.env(&format!("CARGO_FEATURE_{}", super::envify(feat)), "1");
     }
 
     let mut cfg_map = HashMap::new();

src/cargo/ops/cargo_rustc/fingerprint.rs

@@ -319,16 +319,6 @@ fn calculate<'a, 'cfg>(cx: &mut Context<'a, 'cfg>, unit: &Unit<'a>)
         return Ok(s.clone())
     }
 
-    // First, calculate all statically known "salt data" such as the profile
-    // information (compiler flags), the compiler version, activated features,
-    // and target configuration.
-    let features = cx.resolve.features(unit.pkg.package_id());
-    let features = features.map(|s| {
-        let mut v = s.iter().collect::<Vec<_>>();
-        v.sort();
-        v
-    });
-
     // Next, recursively calculate the fingerprint for all of our dependencies.
     //
     // Skip the fingerprints of build scripts as they may not always be
@@ -365,7 +355,7 @@ fn calculate<'a, 'cfg>(cx: &mut Context<'a, 'cfg>, unit: &Unit<'a>)
         rustc: util::hash_u64(&cx.config.rustc()?.verbose_version),
         target: util::hash_u64(&unit.target),
         profile: util::hash_u64(&unit.profile),
-        features: format!("{:?}", features),
+        features: format!("{:?}", cx.resolve.features_sorted(unit.pkg.package_id())),
         deps: deps,
         local: local,
         memoized_hash: Mutex::new(None),

src/cargo/ops/cargo_rustc/mod.rs

@@ -182,11 +182,10 @@ pub fn compile_targets<'a, 'cfg: 'a>(ws: &Workspace<'cfg>,
             }
         }
 
-        if let Some(feats) = cx.resolve.features(unit.pkg.package_id()) {
-            cx.compilation.cfgs.entry(unit.pkg.package_id().clone())
-                .or_insert_with(HashSet::new)
-                .extend(feats.iter().map(|feat| format!("feature=\"{}\"", feat)));
-        }
+        let feats = cx.resolve.features(&unit.pkg.package_id());
+        cx.compilation.cfgs.entry(unit.pkg.package_id().clone())
+            .or_insert_with(HashSet::new)
+            .extend(feats.iter().map(|feat| format!("feature=\"{}\"", feat)));
 
         output_depinfo(&mut cx, unit)?;
     }
@@ -293,11 +292,9 @@ fn rustc(cx: &mut Context, unit: &Unit, exec: Arc<Executor>) -> CargoResult<Work
     let package_id = unit.pkg.package_id().clone();
     let target = unit.target.clone();
     let profile = unit.profile.clone();
-    let features = cx.resolve.features(unit.pkg.package_id())
-                     .into_iter()
-                     .flat_map(|i| i)
-                     .map(|s| s.to_string())
-                     .collect::<Vec<_>>();
+    let features = cx.resolve.features(unit.pkg.package_id()).iter()
+                     .map(|s| s.to_owned())
+                     .collect();
 
     exec.init(cx);
     let exec = exec.clone();
@@ -533,10 +530,8 @@ fn rustdoc(cx: &mut Context, unit: &Unit) -> CargoResult<Work> {
 
     rustdoc.arg("-o").arg(doc_dir);
 
-    if let Some(features) = cx.resolve.features(unit.pkg.package_id()) {
-        for feat in features {
-            rustdoc.arg("--cfg").arg(&format!("feature=\"{}\"", feat));
-        }
+    for feat in cx.resolve.features(unit.pkg.package_id()) {
+        rustdoc.arg("--cfg").arg(&format!("feature=\"{}\"", feat));
     }
 
     if let Some(ref args) = unit.profile.rustdoc_args {
@@ -684,10 +679,8 @@ fn build_base_args(cx: &mut Context,
         cmd.arg("--cfg").arg("test");
     }
 
-    if let Some(features) = cx.resolve.features(unit.pkg.package_id()) {
-        for feat in features.iter() {
-            cmd.arg("--cfg").arg(&format!("feature=\"{}\"", feat));
-        }
+    for feat in cx.resolve.features(unit.pkg.package_id()).iter() {
+        cmd.arg("--cfg").arg(&format!("feature=\"{}\"", feat));
     }
 
     match cx.target_metadata(unit) {