use a BTreeSet to store features

Author: Eh2406

src/cargo/core/resolver/context.rs

@@ -1,4 +1,4 @@
-use std::collections::{HashMap, HashSet};
+use std::collections::HashMap;
 use std::num::NonZeroU64;
 use std::rc::Rc;
 
@@ -20,6 +20,7 @@ use super::types::{ConflictMap, DepInfo, Method};
 pub use super::encode::{EncodableDependency, EncodablePackageId, EncodableResolve};
 pub use super::encode::{Metadata, WorkspaceResolve};
 pub use super::resolve::Resolve;
+use std::collections::btree_set::BTreeSet;
 
 // A `Context` is basically a bunch of local resolution information which is
 // kept around for all `BacktrackFrame` instances. As a result, this runs the
@@ -29,7 +30,7 @@ pub use super::resolve::Resolve;
 pub struct Context {
     pub activations: Activations,
     /// list the features that are activated for each package
-    pub resolve_features: im_rc::HashMap<PackageId, Rc<HashSet<InternedString>>>,
+    pub resolve_features: im_rc::HashMap<PackageId, Rc<BTreeSet<InternedString>>>,
     /// get the package that will be linking to a native library by its links attribute
     pub links: im_rc::HashMap<InternedString, PackageId>,
     /// for each package the list of names it can see,
@@ -102,7 +103,7 @@ impl Context {
     /// Activate this summary by inserting it into our list of known activations.
     ///
     /// Returns `true` if this summary with the given method is already activated.
-    pub fn flag_activated(&mut self, summary: &Summary, method: &Method<'_>) -> CargoResult<bool> {
+    pub fn flag_activated(&mut self, summary: &Summary, method: &Method) -> CargoResult<bool> {
         let id = summary.package_id();
         let age: ContextAge = self.age();
         match self.activations.entry(id.as_activations_key()) {
@@ -127,7 +128,7 @@ impl Context {
             }
         }
         debug!("checking if {} is already activated", summary.package_id());
-        let (features, use_default) = match *method {
+        let (features, use_default) = match method {
             Method::Everything
             | Method::Required {
                 all_features: true, ..
@@ -142,7 +143,7 @@ impl Context {
         let has_default_feature = summary.features().contains_key("default");
         Ok(match self.resolve_features.get(&id) {
             Some(prev) => {
-                features.iter().all(|f| prev.contains(f))
+                features.is_subset(prev)
                     && (!use_default || prev.contains("default") || !has_default_feature)
             }
             None => features.is_empty() && (!use_default || !has_default_feature),
@@ -154,7 +155,7 @@ impl Context {
         registry: &mut dep_cache::RegistryQueryer<'_>,
         parent: Option<PackageId>,
         candidate: &Summary,
-        method: &Method<'_>,
+        method: &Method,
     ) -> ActivateResult<Vec<DepInfo>> {
         // First, figure out our set of dependencies based on the requested set
         // of features. This also calculates what features we're going to enable
@@ -166,7 +167,7 @@ impl Context {
             Rc::make_mut(
                 self.resolve_features
                     .entry(candidate.package_id())
-                    .or_insert_with(|| Rc::new(HashSet::with_capacity(used_features.len()))),
+                    .or_insert_with(Rc::default),
             )
             .extend(used_features);
         }

src/cargo/core/resolver/dep_cache.rs

@@ -1,5 +1,5 @@
 use std::cmp::Ordering;
-use std::collections::{HashMap, HashSet};
+use std::collections::{BTreeSet, HashMap, HashSet};
 use std::rc::Rc;
 
 use log::debug;
@@ -181,10 +181,10 @@ impl<'a> RegistryQueryer<'a> {
 pub fn resolve_features<'b>(
     parent: Option<PackageId>,
     s: &'b Summary,
-    method: &'b Method<'_>,
+    method: &'b Method,
 ) -> ActivateResult<(
     HashSet<InternedString>,
-    Vec<(Dependency, Vec<InternedString>)>,
+    Vec<(Dependency, BTreeSet<InternedString>)>,
 )> {
     let dev_deps = match *method {
         Method::Everything => true,
@@ -198,7 +198,7 @@ pub fn resolve_features<'b>(
     let reqs = build_requirements(s, method)?;
     let mut ret = Vec::new();
     let mut used_features = HashSet::new();
-    let default_dep = (false, Vec::new());
+    let default_dep = (false, BTreeSet::new());
 
     // Next, collect all actually enabled dependencies and their features.
     for dep in deps {
@@ -278,11 +278,11 @@ pub fn resolve_features<'b>(
 /// dependency features in a `Requirements` object, returning it to the resolver.
 fn build_requirements<'a, 'b: 'a>(
     s: &'a Summary,
-    method: &'b Method<'_>,
+    method: &'b Method,
 ) -> CargoResult<Requirements<'a>> {
     let mut reqs = Requirements::new(s);
 
-    match *method {
+    match method {
         Method::Everything
         | Method::Required {
             all_features: true, ..
@@ -329,7 +329,7 @@ struct Requirements<'a> {
     // specified set of features enabled. The boolean indicates whether this
     // package was specifically requested (rather than just requesting features
     // *within* this package).
-    deps: HashMap<InternedString, (bool, Vec<InternedString>)>,
+    deps: HashMap<InternedString, (bool, BTreeSet<InternedString>)>,
     // The used features set is the set of features which this local package had
     // enabled, which is later used when compiling to instruct the code what
     // features were enabled.
@@ -355,9 +355,9 @@ impl Requirements<'_> {
         self.used.insert(package);
         self.deps
             .entry(package)
-            .or_insert((false, Vec::new()))
+            .or_insert((false, BTreeSet::new()))
             .1
-            .push(feat);
+            .insert(feat);
     }
 
     fn seen(&mut self, feat: InternedString) -> bool {
@@ -373,7 +373,7 @@ impl Requirements<'_> {
         if self.seen(pkg) {
             return;
         }
-        self.deps.entry(pkg).or_insert((false, Vec::new())).0 = true;
+        self.deps.entry(pkg).or_insert((false, BTreeSet::new())).0 = true;
     }
 
     fn require_feature(&mut self, feat: InternedString) -> CargoResult<()> {

src/cargo/core/resolver/mod.rs

@@ -47,7 +47,7 @@
 //! that we're implementing something that probably shouldn't be allocating all
 //! over the place.
 
-use std::collections::{BTreeMap, HashMap, HashSet};
+use std::collections::{BTreeMap, BTreeSet, HashMap, HashSet};
 use std::mem;
 use std::rc::Rc;
 use std::time::{Duration, Instant};
@@ -121,7 +121,7 @@ mod types;
 ///     When we have a decision for how to implement is without breaking existing functionality
 ///     this flag can be removed.
 pub fn resolve(
-    summaries: &[(Summary, Method<'_>)],
+    summaries: &[(Summary, Method)],
     replacements: &[(PackageIdSpec, Dependency)],
     registry: &mut dyn Registry,
     try_to_use: &HashSet<PackageId>,
@@ -169,7 +169,7 @@ pub fn resolve(
 fn activate_deps_loop(
     mut cx: Context,
     registry: &mut RegistryQueryer<'_>,
-    summaries: &[(Summary, Method<'_>)],
+    summaries: &[(Summary, Method)],
     config: Option<&Config>,
 ) -> CargoResult<Context> {
     let mut backtrack_stack = Vec::new();
@@ -374,7 +374,7 @@ fn activate_deps_loop(
             let pid = candidate.summary.package_id();
             let method = Method::Required {
                 dev_deps: false,
-                features: &features,
+                features: Rc::clone(&features),
                 all_features: false,
                 uses_default_features: dep.uses_default_features(),
             };
@@ -597,7 +597,7 @@ fn activate(
     registry: &mut RegistryQueryer<'_>,
     parent: Option<(&Summary, &Dependency)>,
     candidate: Candidate,
-    method: &Method<'_>,
+    method: &Method,
 ) -> ActivateResult<Option<(DepsFrame, Duration)>> {
     let candidate_pid = candidate.summary.package_id();
     if let Some((parent, dep)) = parent {
@@ -704,7 +704,7 @@ struct BacktrackFrame {
     remaining_candidates: RemainingCandidates,
     parent: Summary,
     dep: Dependency,
-    features: Rc<Vec<InternedString>>,
+    features: Rc<BTreeSet<InternedString>>,
     conflicting_activations: ConflictMap,
 }
 

src/cargo/core/resolver/types.rs

@@ -1,5 +1,5 @@
 use std::cmp::Ordering;
-use std::collections::BTreeMap;
+use std::collections::{BTreeMap, BTreeSet};
 use std::ops::Range;
 use std::rc::Rc;
 use std::time::{Duration, Instant};
@@ -89,26 +89,26 @@ impl ResolverProgress {
     }
 }
 
-#[derive(Clone, Copy)]
-pub enum Method<'a> {
+#[derive(Clone)]
+pub enum Method {
     Everything, // equivalent to Required { dev_deps: true, all_features: true, .. }
     Required {
         dev_deps: bool,
-        features: &'a [InternedString],
+        features: Rc<BTreeSet<InternedString>>,
         all_features: bool,
         uses_default_features: bool,
     },
 }
 
-impl<'r> Method<'r> {
-    pub fn split_features(features: &[String]) -> Vec<InternedString> {
+impl Method {
+    pub fn split_features(features: &[String]) -> BTreeSet<InternedString> {
         features
             .iter()
             .flat_map(|s| s.split_whitespace())
             .flat_map(|s| s.split(','))
             .filter(|s| !s.is_empty())
-            .map(|s| InternedString::new(s))
-            .collect::<Vec<InternedString>>()
+            .map(InternedString::new)
+            .collect::<BTreeSet<InternedString>>()
     }
 }
 
@@ -221,7 +221,7 @@ impl RemainingDeps {
 // Information about the dependencies for a crate, a tuple of:
 //
 // (dependency info, candidates, features activated)
-pub type DepInfo = (Dependency, Rc<Vec<Candidate>>, Rc<Vec<InternedString>>);
+pub type DepInfo = (Dependency, Rc<Vec<Candidate>>, Rc<BTreeSet<InternedString>>);
 
 /// All possible reasons that a package might fail to activate.
 ///

src/cargo/ops/cargo_compile.rs

@@ -23,6 +23,7 @@
 use std::collections::{BTreeSet, HashMap, HashSet};
 use std::iter::FromIterator;
 use std::path::PathBuf;
+use std::rc::Rc;
 use std::sync::Arc;
 
 use crate::core::compiler::{BuildConfig, BuildContext, Compilation, Context};
@@ -299,7 +300,7 @@ pub fn compile_ws<'a>(
     let features = Method::split_features(features);
     let method = Method::Required {
         dev_deps: ws.require_optional_deps() || filter.need_dev_deps(build_config.mode),
-        features: &features,
+        features: Rc::new(features),
         all_features,
         uses_default_features: !no_default_features,
     };

src/cargo/ops/resolve.rs

@@ -1,4 +1,5 @@
 use std::collections::HashSet;
+use std::rc::Rc;
 
 use log::{debug, trace};
 
@@ -43,7 +44,7 @@ pub fn resolve_ws_precisely<'a>(
     } else {
         Method::Required {
             dev_deps: true,
-            features: &features,
+            features: Rc::new(features),
             all_features: false,
             uses_default_features: !no_default_features,
         }
@@ -53,7 +54,7 @@ pub fn resolve_ws_precisely<'a>(
 
 pub fn resolve_ws_with_method<'a>(
     ws: &Workspace<'a>,
-    method: Method<'_>,
+    method: Method,
     specs: &[PackageIdSpec],
 ) -> CargoResult<(PackageSet<'a>, Resolve)> {
     let mut registry = PackageRegistry::new(ws.config())?;
@@ -138,7 +139,7 @@ fn resolve_with_registry<'cfg>(
 pub fn resolve_with_previous<'cfg>(
     registry: &mut PackageRegistry<'cfg>,
     ws: &Workspace<'cfg>,
-    method: Method<'_>,
+    method: Method,
     previous: Option<&Resolve>,
     to_avoid: Option<&HashSet<PackageId>>,
     specs: &[PackageIdSpec],
@@ -222,7 +223,7 @@ pub fn resolve_with_previous<'cfg>(
     let mut summaries = Vec::new();
     if ws.config().cli_unstable().package_features {
         let mut members = Vec::new();
-        match method {
+        match &method {
             Method::Everything => members.extend(ws.members()),
             Method::Required {
                 features,
@@ -241,7 +242,7 @@ pub fn resolve_with_previous<'cfg>(
                 // of current workspace. Add all packages from workspace to get `foo`
                 // into the resolution graph.
                 if members.is_empty() {
-                    if !(features.is_empty() && !all_features && uses_default_features) {
+                    if !(features.is_empty() && !all_features && *uses_default_features) {
                         failure::bail!("cannot specify features for packages outside of workspace");
                     }
                     members.extend(ws.members());
@@ -250,7 +251,7 @@ pub fn resolve_with_previous<'cfg>(
         }
         for member in members {
             let summary = registry.lock(member.summary().clone());
-            summaries.push((summary, method))
+            summaries.push((summary, method.clone()))
         }
     } else {
         for member in ws.members() {
@@ -281,13 +282,13 @@ pub fn resolve_with_previous<'cfg>(
                 } => {
                     let base = Method::Required {
                         dev_deps,
-                        features: &[],
+                        features: Rc::default(),
                         all_features,
                         uses_default_features: true,
                     };
                     let member_id = member.package_id();
                     match ws.current_opt() {
-                        Some(current) if member_id == current.package_id() => method,
+                        Some(current) if member_id == current.package_id() => method.clone(),
                         _ => {
                             if specs.iter().any(|spec| spec.matches(member_id)) {
                                 base