Skip to content

Commit c6a306f

Browse files
committed
uv-pep508: add more routines for manipulating extras
In the course of working on #9289, I've had to devise some additions to our markers. While we are still staying strictly compatible with the PEP 508 format, we will be abusing the `extra` expression to carry a lot more information. Specifically, we want the following additional operations: * Simplify `extra != 'foo'` * Remove all extra expressions * Remove everything except extra expressions My work on #9289 requires all of these (which will be in a future in PR).
1 parent 3a92c71 commit c6a306f

2 files changed

Lines changed: 238 additions & 1 deletion

File tree

crates/uv-pep508/src/marker/algebra.rs

Lines changed: 65 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -412,7 +412,7 @@ impl InternerGuard<'_> {
412412
// Restrict this variable to the given output by merging it
413413
// with the relevant child.
414414
let node = if value { high } else { low };
415-
return node.negate(i);
415+
return self.restrict(node.negate(i), f);
416416
}
417417
}
418418

@@ -421,6 +421,70 @@ impl InternerGuard<'_> {
421421
self.create_node(node.var.clone(), children)
422422
}
423423

424+
/// Returns a new tree where the only nodes remaining are non-`extra`
425+
/// nodes.
426+
///
427+
/// If there are only `extra` nodes, then this returns a tree that is
428+
/// always true.
429+
///
430+
/// This works by assuming all `extra` nodes are always true.
431+
///
432+
/// For example, given a marker like
433+
/// `((os_name == ... and extra == foo) or (sys_platform == ... and extra != foo))`,
434+
/// this would return a marker
435+
/// `os_name == ... or sys_platform == ...`.
436+
pub(crate) fn without_extras(&mut self, mut i: NodeId) -> NodeId {
437+
if matches!(i, NodeId::TRUE | NodeId::FALSE) {
438+
return i;
439+
}
440+
441+
let parent = i;
442+
let node = self.shared.node(i);
443+
if matches!(node.var, Variable::Extra(_)) {
444+
i = NodeId::FALSE;
445+
for child in node.children.nodes() {
446+
i = self.or(i, child.negate(parent));
447+
}
448+
if i.is_true() {
449+
return NodeId::TRUE;
450+
}
451+
self.without_extras(i)
452+
} else {
453+
// Restrict all nodes recursively.
454+
let children = node.children.map(i, |node| self.without_extras(node));
455+
self.create_node(node.var.clone(), children)
456+
}
457+
}
458+
459+
/// Returns a new tree where the only nodes remaining are `extra` nodes.
460+
///
461+
/// If there are no extra nodes, then this returns a tree that is always
462+
/// true.
463+
///
464+
/// This works by assuming all non-`extra` nodes are always true.
465+
pub(crate) fn only_extras(&mut self, mut i: NodeId) -> NodeId {
466+
if matches!(i, NodeId::TRUE | NodeId::FALSE) {
467+
return i;
468+
}
469+
470+
let parent = i;
471+
let node = self.shared.node(i);
472+
if !matches!(node.var, Variable::Extra(_)) {
473+
i = NodeId::FALSE;
474+
for child in node.children.nodes() {
475+
i = self.or(i, child.negate(parent));
476+
}
477+
if i.is_true() {
478+
return NodeId::TRUE;
479+
}
480+
self.only_extras(i)
481+
} else {
482+
// Restrict all nodes recursively.
483+
let children = node.children.map(i, |node| self.only_extras(node));
484+
self.create_node(node.var.clone(), children)
485+
}
486+
}
487+
424488
/// Simplify this tree by *assuming* that the Python version range provided
425489
/// is true and that the complement of it is false.
426490
///

crates/uv-pep508/src/marker/tree.rs

Lines changed: 173 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1063,6 +1063,21 @@ impl MarkerTree {
10631063
self.simplify_extras_with(|name| extras.contains(name))
10641064
}
10651065

1066+
/// Remove negated extras from a marker, returning `None` if the marker
1067+
/// tree evaluates to `true`.
1068+
///
1069+
/// Any negated `extra` markers that are always `true` given the provided
1070+
/// extras will be removed. Any `extra` markers that are always `false`
1071+
/// given the provided extras will be left unchanged.
1072+
///
1073+
/// For example, if `dev` is a provided extra, given `sys_platform
1074+
/// == 'linux' and extra != 'dev'`, the marker will be simplified to
1075+
/// `sys_platform == 'linux'`.
1076+
#[must_use]
1077+
pub fn simplify_not_extras(self, extras: &[ExtraName]) -> MarkerTree {
1078+
self.simplify_not_extras_with(|name| extras.contains(name))
1079+
}
1080+
10661081
/// Remove the extras from a marker, returning `None` if the marker tree evaluates to `true`.
10671082
///
10681083
/// Any `extra` markers that are always `true` given the provided predicate will be removed.
@@ -1082,12 +1097,57 @@ impl MarkerTree {
10821097
self.simplify_extras_with_impl(&is_extra)
10831098
}
10841099

1100+
/// Remove negated extras from a marker, returning `None` if the marker tree evaluates to
1101+
/// `true`.
1102+
///
1103+
/// Any negated `extra` markers that are always `true` given the provided
1104+
/// predicate will be removed. Any `extra` markers that are always `false`
1105+
/// given the provided predicate will be left unchanged.
1106+
///
1107+
/// For example, if `is_extra('dev')` is true, given
1108+
/// `sys_platform == 'linux' and extra != 'dev'`, the marker will be simplified to
1109+
/// `sys_platform == 'linux'`.
1110+
#[must_use]
1111+
pub fn simplify_not_extras_with(self, is_extra: impl Fn(&ExtraName) -> bool) -> MarkerTree {
1112+
// Because `simplify_extras_with_impl` is recursive, and we need to use
1113+
// our predicate in recursive calls, we need the predicate itself to
1114+
// have some indirection (or else we'd have to clone it). To avoid a
1115+
// recursive type at codegen time, we just introduce the indirection
1116+
// here, but keep the calling API ergonomic.
1117+
self.simplify_not_extras_with_impl(&is_extra)
1118+
}
1119+
1120+
/// Returns a new `MarkerTree` where all `extra` expressions are removed.
1121+
///
1122+
/// If the marker only consisted of `extra` expressions, then a marker that
1123+
/// is always true is returned.
1124+
#[must_use]
1125+
pub fn without_extras(self) -> MarkerTree {
1126+
MarkerTree(INTERNER.lock().without_extras(self.0))
1127+
}
1128+
1129+
/// Returns a new `MarkerTree` where only `extra` expressions are removed.
1130+
///
1131+
/// If the marker did not contain any `extra` expressions, then a marker
1132+
/// that is always true is returned.
1133+
#[must_use]
1134+
pub fn only_extras(self) -> MarkerTree {
1135+
MarkerTree(INTERNER.lock().only_extras(self.0))
1136+
}
1137+
10851138
fn simplify_extras_with_impl(self, is_extra: &impl Fn(&ExtraName) -> bool) -> MarkerTree {
10861139
MarkerTree(INTERNER.lock().restrict(self.0, &|var| match var {
10871140
Variable::Extra(name) => is_extra(name.extra()).then_some(true),
10881141
_ => None,
10891142
}))
10901143
}
1144+
1145+
fn simplify_not_extras_with_impl(self, is_extra: &impl Fn(&ExtraName) -> bool) -> MarkerTree {
1146+
MarkerTree(INTERNER.lock().restrict(self.0, &|var| match var {
1147+
Variable::Extra(name) => is_extra(name.extra()).then_some(false),
1148+
_ => None,
1149+
}))
1150+
}
10911151
}
10921152

10931153
impl fmt::Debug for MarkerTree {
@@ -2068,6 +2128,59 @@ mod test {
20682128
assert_eq!(simplified, expected);
20692129
}
20702130

2131+
#[test]
2132+
fn test_simplify_not_extras() {
2133+
// Given `os_name == "nt" and extra != "dev"`, simplify to `os_name == "nt"`.
2134+
let markers = MarkerTree::from_str(r#"os_name == "nt" and extra != "dev""#).unwrap();
2135+
let simplified = markers.simplify_not_extras(&[ExtraName::from_str("dev").unwrap()]);
2136+
let expected = MarkerTree::from_str(r#"os_name == "nt""#).unwrap();
2137+
assert_eq!(simplified, expected);
2138+
2139+
// Given `os_name == "nt" or extra != "dev"`, remove the marker entirely.
2140+
let markers = MarkerTree::from_str(r#"os_name == "nt" or extra != "dev""#).unwrap();
2141+
let simplified = markers.simplify_not_extras(&[ExtraName::from_str("dev").unwrap()]);
2142+
assert_eq!(simplified, MarkerTree::TRUE);
2143+
2144+
// Given `extra != "dev"`, remove the marker entirely.
2145+
let markers = MarkerTree::from_str(r#"extra != "dev""#).unwrap();
2146+
let simplified = markers.simplify_not_extras(&[ExtraName::from_str("dev").unwrap()]);
2147+
assert_eq!(simplified, MarkerTree::TRUE);
2148+
2149+
// Given `extra != "dev" and extra != "test"`, simplify to `extra != "test"`.
2150+
let markers = MarkerTree::from_str(r#"extra != "dev" and extra != "test""#).unwrap();
2151+
let simplified = markers.simplify_not_extras(&[ExtraName::from_str("dev").unwrap()]);
2152+
let expected = MarkerTree::from_str(r#"extra != "test""#).unwrap();
2153+
assert_eq!(simplified, expected);
2154+
2155+
// Given `os_name == "nt" and extra != "test"`, don't simplify.
2156+
let markers = MarkerTree::from_str(r#"os_name != "nt" and extra != "test""#).unwrap();
2157+
let simplified = markers
2158+
.clone()
2159+
.simplify_not_extras(&[ExtraName::from_str("dev").unwrap()]);
2160+
assert_eq!(simplified, markers);
2161+
2162+
// Given `os_name == "nt" and (python_version == "3.7" or extra != "dev")`, simplify to
2163+
// `os_name == "nt".
2164+
let markers = MarkerTree::from_str(
2165+
r#"os_name == "nt" and (python_version == "3.7" or extra != "dev")"#,
2166+
)
2167+
.unwrap();
2168+
let simplified = markers.simplify_not_extras(&[ExtraName::from_str("dev").unwrap()]);
2169+
let expected = MarkerTree::from_str(r#"os_name == "nt""#).unwrap();
2170+
assert_eq!(simplified, expected);
2171+
2172+
// Given `os_name == "nt" or (python_version == "3.7" and extra != "dev")`, simplify to
2173+
// `os_name == "nt" or python_version == "3.7"`.
2174+
let markers = MarkerTree::from_str(
2175+
r#"os_name == "nt" or (python_version == "3.7" and extra != "dev")"#,
2176+
)
2177+
.unwrap();
2178+
let simplified = markers.simplify_not_extras(&[ExtraName::from_str("dev").unwrap()]);
2179+
let expected =
2180+
MarkerTree::from_str(r#"os_name == "nt" or python_version == "3.7""#).unwrap();
2181+
assert_eq!(simplified, expected);
2182+
}
2183+
20712184
#[test]
20722185
fn test_marker_simplification() {
20732186
assert_false("python_version == '3.9.1'");
@@ -3008,4 +3121,64 @@ mod test {
30083121
m("python_full_version >= '3.9'"),
30093122
);
30103123
}
3124+
3125+
#[test]
3126+
fn without_extras() {
3127+
assert_eq!(
3128+
m("os_name == 'Linux'").without_extras(),
3129+
m("os_name == 'Linux'"),
3130+
);
3131+
assert!(m("extra == 'foo'").without_extras().is_true());
3132+
assert_eq!(
3133+
m("os_name == 'Linux' and extra == 'foo'").without_extras(),
3134+
m("os_name == 'Linux'"),
3135+
);
3136+
3137+
assert!(m("
3138+
(os_name == 'Linux' and extra == 'foo')
3139+
or (os_name != 'Linux' and extra == 'bar')",)
3140+
.without_extras()
3141+
.is_true());
3142+
3143+
assert_eq!(
3144+
m("os_name == 'Linux' and extra != 'foo'").without_extras(),
3145+
m("os_name == 'Linux'"),
3146+
);
3147+
3148+
assert!(
3149+
m("extra != 'extra-project-bar' and extra == 'extra-project-foo'")
3150+
.without_extras()
3151+
.is_true()
3152+
);
3153+
}
3154+
3155+
#[test]
3156+
fn only_extras() {
3157+
assert!(m("os_name == 'Linux'").only_extras().is_true());
3158+
assert_eq!(m("extra == 'foo'").only_extras(), m("extra == 'foo'"));
3159+
assert_eq!(
3160+
m("os_name == 'Linux' and extra == 'foo'").only_extras(),
3161+
m("extra == 'foo'"),
3162+
);
3163+
assert!(m("
3164+
(os_name == 'foo' and extra == 'foo')
3165+
or (os_name == 'bar' and extra != 'foo')",)
3166+
.only_extras()
3167+
.is_true());
3168+
assert_eq!(
3169+
m("
3170+
(os_name == 'Linux' and extra == 'foo')
3171+
or (os_name != 'Linux' and extra == 'bar')")
3172+
.only_extras(),
3173+
m("extra == 'foo' or extra == 'bar'"),
3174+
);
3175+
3176+
assert_eq!(
3177+
m("
3178+
(implementation_name != 'pypy' and os_name == 'nt' and sys_platform == 'darwin')
3179+
or (os_name == 'nt' and sys_platform == 'win32')")
3180+
.only_extras(),
3181+
m("os_name == 'Linux' or os_name != 'Linux'"),
3182+
);
3183+
}
30113184
}

0 commit comments

Comments
 (0)