From b414ec01cc03d6e9aaad911f7a582ce03bc5bd86 Mon Sep 17 00:00:00 2001 From: Andrew Gallant Date: Fri, 6 Sep 2024 13:13:54 -0400 Subject: [PATCH 1/3] pep508: remove hidden state from `MarkerTree` I split this change into its own commit because I'm hoping it crystalizes what it means when we say "a `MarkerTree` has hidden state." That is, it isn't so much that there is some explicit member of a `MarkerTree` that is omitted, but rather, the lower and upper version bounds on `python_full_version` are are rewritten as "unbounded" when traversing the ADD for display. We will actually retain this functionality, but rejigger it so that it's explicit when we do this. In particular, this simplification has been problematic for us because it fundamentally changes the truth tables of a marker expression *unless* you are extremely careful to interpret it only under the original context in which it was simplified. This is quite difficult to do generally, and in prior work in #6268, we completed a refactor where we worked around this type of simplification and moved it to the edges of uv. In subsequent commits, we'll re-implement this form of simplification as a more explicit step. --- crates/pep508-rs/src/marker/simplify.rs | 22 +++------------------- 1 file changed, 3 insertions(+), 19 deletions(-) diff --git a/crates/pep508-rs/src/marker/simplify.rs b/crates/pep508-rs/src/marker/simplify.rs index 998f71257a91..f270eb9c2295 100644 --- a/crates/pep508-rs/src/marker/simplify.rs +++ b/crates/pep508-rs/src/marker/simplify.rs @@ -285,26 +285,10 @@ fn collect_edges<'a, T>( where T: Ord + Clone + 'a, { - let len = map.len(); - let mut paths: IndexMap<_, Range<_>, FxBuildHasher> = IndexMap::default(); - for (i, (range, tree)) in map.enumerate() { - let (mut start, mut end) = range.bounding_range().unwrap(); - match (start, end) { - (Bound::Included(v1), Bound::Included(v2)) if v1 == v2 => {} - _ => { - // Expand the range of this variable to be unbounded. - // This helps remove redundant expressions such as `python_version >= '3.7'` - // when the range has already been restricted to `['3.7', inf)` - if i == 0 { - start = Bound::Unbounded; - } - if i == len - 1 { - end = Bound::Unbounded; - } - } - } - + for (range, tree) in map { + // OK because all ranges are guaranteed to be non-empty. + let (start, end) = range.bounding_range().unwrap(); // Combine the ranges. let range = Range::from_range_bounds((start.cloned(), end.cloned())); paths From 2e729fcf5de3029460fb349cb218d1ade064f7ef Mon Sep 17 00:00:00 2001 From: Andrew Gallant Date: Fri, 6 Sep 2024 14:07:57 -0400 Subject: [PATCH 2/3] pep508: implement marker simplification/complexification This adds new routines to `MarkerTree` for "simplifying" and "complexifying" a tree with respect to lower and upper Python version bounds. In effect, "simplifying" a marker is what you do when you write it to a lock file. Namely, since `uv.lock` includes a `requires-python` bound at the top, one can say that it acts as a bound on the supported Python versions. That is, it establishes a context in which one can assume that bound is true. Therefore, the markers we write can be simplified using this assumption. The reverse is "complexifying" a marker, and it's what you do when you read a marker from the lock file. Namely, once a marker is read, it can be very difficult in code to keep the corresponding requires-python context from the lock file. If you lose track of it and decide to operate on the "simplified" marker, then it's trivial for that to produce an incorrect result. --- crates/pep508-rs/src/marker/algebra.rs | 196 ++++++++++++++--- crates/pep508-rs/src/marker/tree.rs | 282 +++++++++++++++++++++---- 2 files changed, 413 insertions(+), 65 deletions(-) diff --git a/crates/pep508-rs/src/marker/algebra.rs b/crates/pep508-rs/src/marker/algebra.rs index 57b707334789..96e705ca86b6 100644 --- a/crates/pep508-rs/src/marker/algebra.rs +++ b/crates/pep508-rs/src/marker/algebra.rs @@ -371,43 +371,187 @@ impl InternerGuard<'_> { self.create_node(node.var.clone(), children) } - // Restrict the output of a given version variable in the tree. - // - // If the provided function `f` returns a `Some` range, the tree will be simplified with - // the assumption that the given variable is restricted to values in that range. If the function - // returns `None`, the variable will not be affected. - pub(crate) fn restrict_versions( + /// Simplify this tree by *assuming* that the Python version range provided + /// is true and that the complement of it is false. + /// + /// For example, with `requires-python = '>=3.8'` and a marker tree of + /// `python_full_version >= '3.8' and python_full_version <= '3.10'`, this + /// would result in a marker of `python_full_version <= '3.10'`. + pub(crate) fn simplify_python_versions( &mut self, i: NodeId, - f: &impl Fn(&Variable) -> Option>, + py_lower: Bound<&Version>, + py_upper: Bound<&Version>, ) -> NodeId { - if matches!(i, NodeId::TRUE | NodeId::FALSE) { + if matches!(i, NodeId::TRUE | NodeId::FALSE) + || matches!((py_lower, py_upper), (Bound::Unbounded, Bound::Unbounded)) + { return i; } let node = self.shared.node(i); - if let Edges::Version { edges: ref map } = node.children { - if let Some(allowed) = f(&node.var) { - // Restrict the output of this variable to the given range. - let mut simplified = SmallVec::new(); - for (range, node) in map { - let restricted = range.intersection(&allowed); - if restricted.is_empty() { - continue; - } + // Look for a `python_full_version` expression, otherwise + // we recursively simplify. + let Node { + var: Variable::Version(MarkerValueVersion::PythonFullVersion), + children: Edges::Version { ref edges }, + } = node + else { + // Simplify all nodes recursively. + let children = node.children.map(i, |node_id| { + self.simplify_python_versions(node_id, py_lower, py_upper) + }); + return self.create_node(node.var.clone(), children); + }; + let py_range = Range::from_range_bounds((py_lower.cloned(), py_upper.cloned())); + if py_range.is_empty() { + // Oops, the bounds imply there is nothing that can match, + // so we always evaluate to false. + return NodeId::FALSE; + } + let mut new = SmallVec::new(); + for &(ref range, node) in edges { + let overlap = range.intersection(&py_range); + if overlap.is_empty() { + continue; + } + new.push((overlap.clone(), node)); + } - simplified.push((restricted.clone(), *node)); - } + // Now that we know the only ranges left are those that + // intersect with our lower/upper Python version bounds, we + // can "extend" out the lower/upper bounds here all the way to + // negative and positive infinity, respectively. + // + // This has the effect of producing a marker that is only + // applicable in a context where the Python lower/upper bounds + // are known to be satisfied. + let &(ref first_range, first_node_id) = new.first().unwrap(); + let first_upper = first_range.bounding_range().unwrap().1; + let clipped = Range::from_range_bounds((Bound::Unbounded, first_upper.cloned())); + *new.first_mut().unwrap() = (clipped, first_node_id); + + let &(ref last_range, last_node_id) = new.last().unwrap(); + let last_lower = last_range.bounding_range().unwrap().0; + let clipped = Range::from_range_bounds((last_lower.cloned(), Bound::Unbounded)); + *new.last_mut().unwrap() = (clipped, last_node_id); + + self.create_node(node.var.clone(), Edges::Version { edges: new }) + .negate(i) + } - return self - .create_node(node.var.clone(), Edges::Version { edges: simplified }) - .negate(i); - } + /// Complexify this tree by requiring the given Python version + /// range to be true in order for this marker tree to evaluate to + /// true in all circumstances. + /// + /// For example, with `requires-python = '>=3.8'` and a marker tree of + /// `python_full_version <= '3.10'`, this would result in a marker of + /// `python_full_version >= '3.8' and python_full_version <= '3.10'`. + pub(crate) fn complexify_python_versions( + &mut self, + i: NodeId, + py_lower: Bound<&Version>, + py_upper: Bound<&Version>, + ) -> NodeId { + if matches!(i, NodeId::FALSE) + || matches!((py_lower, py_upper), (Bound::Unbounded, Bound::Unbounded)) + { + return i; } - // Restrict all nodes recursively. - let children = node.children.map(i, |node| self.restrict_versions(node, f)); - self.create_node(node.var.clone(), children) + let py_range = Range::from_range_bounds((py_lower.cloned(), py_upper.cloned())); + if py_range.is_empty() { + // Oops, the bounds imply there is nothing that can match, + // so we always evaluate to false. + return NodeId::FALSE; + } + if matches!(i, NodeId::TRUE) { + let var = Variable::Version(MarkerValueVersion::PythonFullVersion); + let edges = Edges::Version { + edges: Edges::from_range(&py_range), + }; + return self.create_node(var, edges).negate(i); + } + + let node = self.shared.node(i); + let Node { + var: Variable::Version(MarkerValueVersion::PythonFullVersion), + children: Edges::Version { ref edges }, + } = node + else { + // Complexify all nodes recursively. + let children = node.children.map(i, |node_id| { + self.complexify_python_versions(node_id, py_lower, py_upper) + }); + return self.create_node(node.var.clone(), children); + }; + // The approach we take here is to filter out any range that + // has no intersection with our Python lower/upper bounds. + // These ranges will now always be false, so we can dismiss + // them entirely. + // + // Then, depending on whether we have finite lower/upper bound, + // we "fix up" the edges by clipping the existing ranges and + // adding an additional range that covers the Python versions + // outside of our bounds by always mapping them to false. + let mut new: SmallVec<_> = edges + .iter() + .filter(|(range, _)| !py_range.intersection(range).is_empty()) + .cloned() + .collect(); + // Below, we assume `new` has at least one element. It's + // subtle, but since 1) edges is a disjoint covering of the + // universe and 2) our `py_range` is non-empty at this point, + // it must intersect with at least one range. + assert!( + !new.is_empty(), + "expected at least one non-empty intersection" + ); + // This is the NodeId we map to anything that should + // always be false. We have to "negate" it in case the + // parent is negated. + let exclude_node_id = NodeId::FALSE.negate(i); + if !matches!(py_lower, Bound::Unbounded) { + let &(ref first_range, first_node_id) = new.first().unwrap(); + let first_upper = first_range.bounding_range().unwrap().1; + // When the first range is always false, then we can just + // "expand" it out to negative infinity to reflect that + // anything less than our lower bound should evaluate to + // false. If we don't do this, then we could have two + // adjacent ranges map to the same node, which would not be + // a canonical representation. + if exclude_node_id == first_node_id { + let clipped = Range::from_range_bounds((Bound::Unbounded, first_upper.cloned())); + *new.first_mut().unwrap() = (clipped, first_node_id); + } else { + let clipped = Range::from_range_bounds((py_lower.cloned(), first_upper.cloned())); + *new.first_mut().unwrap() = (clipped, first_node_id); + + let py_range_lower = + Range::from_range_bounds((py_lower.cloned(), Bound::Unbounded)); + new.insert(0, (py_range_lower.complement(), NodeId::FALSE.negate(i))); + } + } + if !matches!(py_upper, Bound::Unbounded) { + let &(ref last_range, last_node_id) = new.last().unwrap(); + let last_lower = last_range.bounding_range().unwrap().0; + // See lower bound case above for why we do this. The + // same reasoning applies here: to maintain a canonical + // representation. + if exclude_node_id == last_node_id { + let clipped = Range::from_range_bounds((last_lower.cloned(), Bound::Unbounded)); + *new.last_mut().unwrap() = (clipped, last_node_id); + } else { + let clipped = Range::from_range_bounds((last_lower.cloned(), py_upper.cloned())); + *new.last_mut().unwrap() = (clipped, last_node_id); + + let py_range_upper = + Range::from_range_bounds((Bound::Unbounded, py_upper.cloned())); + new.push((py_range_upper.complement(), exclude_node_id)); + } + } + self.create_node(node.var.clone(), Edges::Version { edges: new }) + .negate(i) } } diff --git a/crates/pep508-rs/src/marker/tree.rs b/crates/pep508-rs/src/marker/tree.rs index b036bc4634a6..4751bfb91089 100644 --- a/crates/pep508-rs/src/marker/tree.rs +++ b/crates/pep508-rs/src/marker/tree.rs @@ -1132,25 +1132,63 @@ impl MarkerTree { extra_expression } - /// Remove the extras from a marker, returning `None` if the marker tree evaluates to `true`. + /// Simplify this marker by *assuming* that the Python version range + /// provided is true and that the complement of it is false. /// - /// Any `extra` markers that are always `true` given the provided extras will be removed. - /// Any `extra` markers that are always `false` given the provided extras will be left - /// unchanged. + /// For example, with `requires-python = '>=3.8'` and a marker tree of + /// `python_full_version >= '3.8' and python_full_version <= '3.10'`, this + /// would result in a marker of `python_full_version <= '3.10'`. /// - /// For example, if `dev` is a provided extra, given `sys_platform == 'linux' and extra == 'dev'`, - /// the marker will be simplified to `sys_platform == 'linux'`. + /// This is useful when one wants to write "simpler" markers in a + /// particular context with a bound on the supported Python versions. + /// In general, the simplified markers returned shouldn't be used for + /// evaluation. Instead, they should be turned back into their more + /// "complex" form first. + /// + /// Note that simplifying a marker and then complexifying it, even + /// with the same Python version bounds, is a lossy operation. For + /// example, simplifying `python_version < '3.7'` with `requires-python + /// = ">=3.8"` will result in a marker that always returns false (e.g., + /// `python_version < '0'`). Therefore, complexifying an always-false + /// marker will result in a marker that is still always false, despite + /// the fact that the original marker was true for `<3.7`. Therefore, + /// simplifying should only be done as a one-way transformation when it is + /// known that `requires-python` reflects an eternal lower bound on the + /// results of that simplification. (If `requires-python` changes, then one + /// should reconstitute all relevant markers from the source data.) #[must_use] #[allow(clippy::needless_pass_by_value)] - pub fn simplify_python_versions(self, python_version: Range) -> MarkerTree { - MarkerTree(INTERNER.lock().restrict_versions(self.0, &|var| match var { - // Note that `python_version` is normalized to `python_full_version` internally by the - // decision diagram. - Variable::Version(MarkerValueVersion::PythonFullVersion) => { - Some(python_version.clone()) - } - _ => None, - })) + pub fn simplify_python_versions( + self, + lower: Bound<&Version>, + upper: Bound<&Version>, + ) -> MarkerTree { + MarkerTree( + INTERNER + .lock() + .simplify_python_versions(self.0, lower, upper), + ) + } + + /// Complexify marker tree by requiring the given Python version range + /// to be true in order for this marker tree to evaluate to true in all + /// circumstances. + /// + /// For example, with `requires-python = '>=3.8'` and a marker tree of + /// `python_full_version <= '3.10'`, this would result in a marker of + /// `python_full_version >= '3.8' and python_full_version <= '3.10'`. + #[must_use] + #[allow(clippy::needless_pass_by_value)] + pub fn complexify_python_versions( + self, + lower: Bound<&Version>, + upper: Bound<&Version>, + ) -> MarkerTree { + MarkerTree( + INTERNER + .lock() + .complexify_python_versions(self.0, lower, upper), + ) } /// Remove the extras from a marker, returning `None` if the marker tree evaluates to `true`. @@ -1537,7 +1575,6 @@ mod test { use insta::assert_snapshot; use pep440_rs::Version; - use pubgrub::Range; use uv_normalize::ExtraName; use crate::marker::{MarkerEnvironment, MarkerEnvironmentBuilder}; @@ -1612,19 +1649,19 @@ mod test { assert_eq!( m("(python_version <= '3.11' and sys_platform == 'win32') or python_version > '3.11'") - .simplify_python_versions(Range::from_range_bounds(( - Bound::Excluded(Version::new([3, 12])), - Bound::Unbounded, - ))), + .simplify_python_versions( + Bound::Excluded(Version::new([3, 12])).as_ref(), + Bound::Unbounded.as_ref(), + ), MarkerTree::TRUE ); assert_eq!( m("python_version < '3.10'") - .simplify_python_versions(Range::from_range_bounds(( - Bound::Excluded(Version::new([3, 7])), - Bound::Unbounded, - ))) + .simplify_python_versions( + Bound::Excluded(Version::new([3, 7])).as_ref(), + Bound::Unbounded.as_ref(), + ) .try_to_string() .unwrap(), "python_full_version < '3.10'" @@ -1633,28 +1670,29 @@ mod test { // Note that `3.12.1` will still match. assert_eq!( m("python_version <= '3.12'") - .simplify_python_versions(Range::from_range_bounds(( - Bound::Excluded(Version::new([3, 12])), - Bound::Unbounded, - ))) + .simplify_python_versions( + Bound::Excluded(Version::new([3, 12])).as_ref(), + Bound::Unbounded.as_ref(), + ) .try_to_string() .unwrap(), "python_full_version < '3.13'" ); assert_eq!( - m("python_full_version <= '3.12'").simplify_python_versions(Range::from_range_bounds( - (Bound::Excluded(Version::new([3, 12])), Bound::Unbounded,) - )), + m("python_full_version <= '3.12'").simplify_python_versions( + Bound::Excluded(Version::new([3, 12])).as_ref(), + Bound::Unbounded.as_ref(), + ), MarkerTree::FALSE ); assert_eq!( m("python_full_version <= '3.12.1'") - .simplify_python_versions(Range::from_range_bounds(( - Bound::Excluded(Version::new([3, 12])), - Bound::Unbounded, - ))) + .simplify_python_versions( + Bound::Excluded(Version::new([3, 12])).as_ref(), + Bound::Unbounded.as_ref(), + ) .try_to_string() .unwrap(), "python_full_version <= '3.12.1'" @@ -2422,10 +2460,9 @@ mod test { #[test] fn test_requires_python() { fn simplified(marker: &str) -> MarkerTree { - m(marker).simplify_python_versions(Range::from_range_bounds(( - Bound::Included(Version::new([3, 8])), - Bound::Unbounded, - ))) + let lower = Bound::Included(Version::new([3, 8])); + let upper = Bound::Unbounded; + m(marker).simplify_python_versions(lower.as_ref(), upper.as_ref()) } assert_eq!(simplified("python_version >= '3.8'"), MarkerTree::TRUE); @@ -2687,4 +2724,171 @@ mod test { let (left, right) = (m(left.as_ref()), m(right.as_ref())); left.is_disjoint(&right) && right.is_disjoint(&left) } + + #[test] + fn complexified_markers() { + // Takes optional lower (inclusive) and upper (exclusive) + // bounds representing `requires-python` and a "simplified" + // marker, and returns the "complexified" marker. That is, a + // marker that embeds the `requires-python` constraint into it. + let complexify = + |lower: Option<[u64; 2]>, upper: Option<[u64; 2]>, marker: &str| -> MarkerTree { + let lower = lower + .map(|release| Bound::Included(Version::new(release))) + .unwrap_or(Bound::Unbounded); + let upper = upper + .map(|release| Bound::Excluded(Version::new(release))) + .unwrap_or(Bound::Unbounded); + m(marker).complexify_python_versions(lower.as_ref(), upper.as_ref()) + }; + + assert_eq!( + complexify(None, None, "python_full_version < '3.10'"), + m("python_full_version < '3.10'"), + ); + assert_eq!( + complexify(Some([3, 8]), None, "python_full_version < '3.10'"), + m("python_full_version >= '3.8' and python_full_version < '3.10'"), + ); + assert_eq!( + complexify(None, Some([3, 8]), "python_full_version < '3.10'"), + m("python_full_version < '3.8'"), + ); + assert_eq!( + complexify(Some([3, 8]), Some([3, 8]), "python_full_version < '3.10'"), + // Kinda weird, but this normalizes to `false`, just like the above. + m("python_full_version < '0' and python_full_version > '0'"), + ); + + assert_eq!( + complexify(Some([3, 11]), None, "python_full_version < '3.10'"), + // Kinda weird, but this normalizes to `false`, just like the above. + m("python_full_version < '0' and python_full_version > '0'"), + ); + assert_eq!( + complexify(Some([3, 11]), None, "python_full_version >= '3.10'"), + m("python_full_version >= '3.11'"), + ); + assert_eq!( + complexify(Some([3, 11]), None, "python_full_version >= '3.12'"), + m("python_full_version >= '3.12'"), + ); + + assert_eq!( + complexify(None, Some([3, 11]), "python_full_version > '3.12'"), + // Kinda weird, but this normalizes to `false`, just like the above. + m("python_full_version < '0' and python_full_version > '0'"), + ); + assert_eq!( + complexify(None, Some([3, 11]), "python_full_version <= '3.12'"), + m("python_full_version < '3.11'"), + ); + assert_eq!( + complexify(None, Some([3, 11]), "python_full_version <= '3.10'"), + m("python_full_version <= '3.10'"), + ); + + assert_eq!( + complexify(Some([3, 11]), None, "python_full_version == '3.8'"), + // Kinda weird, but this normalizes to `false`, just like the above. + m("python_full_version < '0' and python_full_version > '0'"), + ); + assert_eq!( + complexify( + Some([3, 11]), + None, + "python_full_version == '3.8' or python_full_version == '3.12'" + ), + m("python_full_version == '3.12'"), + ); + assert_eq!( + complexify( + Some([3, 11]), + None, + "python_full_version == '3.8' \ + or python_full_version == '3.11' \ + or python_full_version == '3.12'" + ), + m("python_full_version == '3.11' or python_full_version == '3.12'"), + ); + + // Tests a tricky case where if a marker is always true, then + // complexifying it will proceed correctly by adding the + // requires-python constraint. This is a regression test for + // an early implementation that special cased the "always + // true" case to return "always true" regardless of the + // requires-python bounds. + assert_eq!( + complexify( + Some([3, 12]), + None, + "python_full_version < '3.10' or python_full_version >= '3.10'" + ), + m("python_full_version >= '3.12'"), + ); + } + + #[test] + fn simplified_markers() { + // Takes optional lower (inclusive) and upper (exclusive) + // bounds representing `requires-python` and a "complexified" + // marker, and returns the "simplified" marker. That is, a + // marker that assumes `requires-python` is true. + let simplify = + |lower: Option<[u64; 2]>, upper: Option<[u64; 2]>, marker: &str| -> MarkerTree { + let lower = lower + .map(|release| Bound::Included(Version::new(release))) + .unwrap_or(Bound::Unbounded); + let upper = upper + .map(|release| Bound::Excluded(Version::new(release))) + .unwrap_or(Bound::Unbounded); + m(marker).simplify_python_versions(lower.as_ref(), upper.as_ref()) + }; + + assert_eq!( + simplify( + Some([3, 8]), + None, + "python_full_version >= '3.8' and python_full_version < '3.10'" + ), + m("python_full_version < '3.10'"), + ); + assert_eq!( + simplify(Some([3, 8]), None, "python_full_version < '3.7'"), + // Kinda weird, but this normalizes to `false`, just like the above. + m("python_full_version < '0' and python_full_version > '0'"), + ); + assert_eq!( + simplify( + Some([3, 8]), + Some([3, 11]), + "python_full_version == '3.7.*' \ + or python_full_version == '3.8.*' \ + or python_full_version == '3.10.*' \ + or python_full_version == '3.11.*' \ + " + ), + // Given `requires-python = '>=3.8,<3.11'`, only `3.8.*` + // and `3.10.*` can possibly be true. So this simplifies + // to `!= 3.9.*`. + m("python_full_version != '3.9.*'"), + ); + assert_eq!( + simplify( + Some([3, 8]), + None, + "python_full_version >= '3.8' and sys_platform == 'win32'" + ), + m("sys_platform == 'win32'"), + ); + assert_eq!( + simplify( + Some([3, 8]), + None, + "python_full_version >= '3.9' \ + and (sys_platform == 'win32' or python_full_version >= '3.8')", + ), + m("python_full_version >= '3.9'"), + ); + } } From f37c755d60abb741d7c1481f260b0ade25948c5a Mon Sep 17 00:00:00 2001 From: Andrew Gallant Date: Fri, 6 Sep 2024 14:09:01 -0400 Subject: [PATCH 3/3] uv-resolver: use new simplify/complexify marker routines This finally gets rid of our hack for working around "hidden" state. We no longer do a roundtrip marker serialization and deserialization just to avoid the hidden state. --- crates/uv-resolver/src/requires_python.rs | 20 +++++--------------- 1 file changed, 5 insertions(+), 15 deletions(-) diff --git a/crates/uv-resolver/src/requires_python.rs b/crates/uv-resolver/src/requires_python.rs index f433832b669b..31e79577971d 100644 --- a/crates/uv-resolver/src/requires_python.rs +++ b/crates/uv-resolver/src/requires_python.rs @@ -317,14 +317,8 @@ impl RequiresPython { /// markers are "complexified" to put the `requires-python` assumption back /// into the marker explicitly. pub(crate) fn simplify_markers(&self, marker: MarkerTree) -> MarkerTree { - let simplified = marker.simplify_python_versions(Range::from(self.range().clone())); - // FIXME: This is a hack to avoid the hidden state created by - // ADD's `restrict_versions`. I believe this is sound, but it's - // wasteful and silly. - simplified - .try_to_string() - .map(|s| s.parse().unwrap()) - .unwrap_or(MarkerTree::TRUE) + let (lower, upper) = (self.range().lower(), self.range().upper()); + marker.simplify_python_versions(lower.0.as_ref(), upper.0.as_ref()) } /// The inverse of `simplify_markers`. @@ -342,13 +336,9 @@ impl RequiresPython { /// ```text /// python_full_version >= '3.8' and python_full_version < '3.12' /// ``` - pub(crate) fn complexify_markers(&self, mut marker: MarkerTree) -> MarkerTree { - // PERF: There's likely a way to amortize this, particularly - // the construction of `to_marker_tree`. But at time of - // writing, it wasn't clear if this was an actual perf problem - // or not. If it is, try a `std::sync::OnceLock`. - marker.and(self.to_marker_tree()); - marker + pub(crate) fn complexify_markers(&self, marker: MarkerTree) -> MarkerTree { + let (lower, upper) = (self.range().lower(), self.range().upper()); + marker.complexify_python_versions(lower.0.as_ref(), upper.0.as_ref()) } /// Returns `false` if the wheel's tags state it can't be used in the given Python version