Skip to content

Commit

Permalink
add comments
Browse files Browse the repository at this point in the history
  • Loading branch information
@Eh2406
Eh2406 committed May 15, 2019
1 parent 8080c98 commit 49eaa13
Show file tree
Hide file tree
Showing 2 changed files with 58 additions and 41 deletions.
19 changes: 19 additions & 0 deletions src/cargo/core/resolver/context.rs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -161,6 +161,25 @@ impl Context {
.and_then(|(s, l)| if s.package_id() == id { Some(*l) } else { None })
}

/// This calculates `is_active` after backtracking to `age` and activating `alternative`.
/// If the package is active returns the `ContextAge` when it was added
/// but only if it is older then `age`.
/// If it is not active but it is the `alternative` then it returns `age`.
pub fn is_older_active_or(
&self,
id: PackageId,
age: ContextAge,
alternative: PackageId,
) -> Option<ContextAge> {
if id == alternative {
// we are imagining that we used other instead
Some(age)
} else {
// we only care about things that are older then critical_age
self.is_active(id).filter(|&other_age| other_age < age)
}
}

/// Checks whether all of `parent` and the keys of `conflicting activations`
/// are still active.
/// If so returns the `ContextAge` when the newest one was added.
Expand Down
80 changes: 39 additions & 41 deletions src/cargo/core/resolver/mod.rs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -890,31 +890,38 @@ fn generalize_conflicting(
return None;
}

let our_activation_keys: HashSet<_> = our_candidates
.iter()
.map(|c| c.package_id().as_activations_key())
.collect();
let our_activation_key = {
// TODO: no reason to allocate a `HashSet` we are just going to throw it out if len > 1
let our_activation_keys: HashSet<_> = our_candidates
.iter()
.map(|c| c.package_id().as_activations_key())
.collect();

// if our dep only matches one semver range then we can fast path any other dep
// that also targets that semver range and has no overlap
let our_activation_key = if our_activation_keys.len() == 1 {
our_activation_keys.iter().next().cloned()
} else {
None
// If our dep only matches one semver range then we can fast path any other dep
// that also targets that semver range and has no overlap.
if our_activation_keys.len() == 1 {
our_activation_keys.iter().next().cloned()
} else {
None
}
};

let our_links: Option<HashSet<_>> = our_candidates.iter().map(|c| c.links()).collect();
let our_link = {
// TODO: no reason to allocate a `HashSet` we are just going to throw it out if len > 1
let our_links: HashSet<Option<_>> = our_candidates.iter().map(|c| c.links()).collect();

// if our dep only matches things that have a links set then we can fast path any other dep
// that also all use that links and has no overlap
let our_link = if let Some(our_links) = our_links {
// If our dep only matches things that have a links set then we can fast path any other dep
// that also all use that links and has no overlap.
if our_links.len() == 1 {
our_links.iter().next().cloned()
// All of `our_candidates` use the same `links`.
// If they all use Some(value), then we can use the fast path.
// If they all use None, then we cant.
our_links.into_iter().next().unwrap()
} else {
// Some of `our_candidates` use a different `links` so whatever `links` get used by
// the conflicting dep we can select the other. We cant use the fast path.
None
}
} else {
None
};

let our_candidates: HashSet<PackageId> =
Expand Down Expand Up @@ -942,11 +949,17 @@ fn generalize_conflicting(
.rev()
// the last one to be tried is the least likely to be in the cache, so start with that.
{
if (our_activation_key
.map_or(false, |our| other.package_id().as_activations_key() == our)
|| our_link.map_or(false, |_| other.links() == our_link))
&& !our_candidates.contains(&other.package_id())
if (
our_activation_key.map_or(false, |our| {
other.package_id().as_activations_key() == our
})
// `other` is semver compatible with all of `our_candidates`
|| our_link.map_or(false, |_| other.links() == our_link)
// or `other` have the same `links` as all of `our_candidates`
) && !our_candidates.contains(&other.package_id())
// and is not one of `our_candidates`.
{
// Thus `dep` can not be resolved when `critical_parents_dep` has bean resolved.
continue;
}

Expand All @@ -967,16 +980,7 @@ fn generalize_conflicting(

if let Some(conflicting) = past_conflicting_activations.find(
dep,
&|id| {
if id == other.package_id() {
// we are imagining that we used other instead
Some(backtrack_critical_age)
} else {
cx.is_active(id).filter(|&age|
// we only care about things that are older then critical_age
age < backtrack_critical_age)
}
},
&|id| cx.is_older_active_or(id, backtrack_critical_age, other.package_id()),
Some(other.package_id()),
) {
con.extend(
Expand All @@ -988,6 +992,9 @@ fn generalize_conflicting(
continue;
}

// We don't have a reason why `other` cant work.
// There for `critical_parents_dep` could have used it, and we can still be activated.
// We can not generalize over `critical_parents_dep`, but maybe the next `critical_parents_dep`
continue 'dep;
}

Expand Down Expand Up @@ -1043,16 +1050,7 @@ fn generalize_children_conflicting(
.filter_map(|(ref new_dep, _, _)| {
past_conflicting_activations.find(
new_dep,
&|id| {
if id == candidate.package_id() {
// we are imagining that we used other instead
Some(critical_age)
} else {
cx.is_active(id).filter(|&age|
// we only care about things that are older then critical_age
age < critical_age)
}
},
&|id| cx.is_older_active_or(id, critical_age, candidate.package_id()),
None,
)
})
Expand Down

0 comments on commit 49eaa13

Please sign in to comment.