Skip to content

Commit

Permalink
Add a lowered representation for markers (#9341)
Browse files Browse the repository at this point in the history
## Summary

This PR introduces a set of parallel structs to `MarkerValueString`,
`MarkerValueExtra`, and `MarkerValueVersion` that remove various pieces
of information and representations that shouldn't be available in the
marker algebra. To start, I've _just_ removed the invalid extra
component from `MarkerValueExtra` -- there are no other changes to the
representation. So, throughout the marker algebra, we can't represent
and thus don't have to worry about clauses with invalid extras.

The subsequent changes I plan to make are:

1. Removing `python_version`, since we exclusively use
`python_version_full` in the algebra.
2. Removing the deprecated aliases, such that we re-map to the correct
marker value.
3. Consolidating `sys_platform` and `platform_release` (the original
motivation).
  • Loading branch information
charliermarsh committed Nov 25, 2024
1 parent 7016cd5 commit 47cb7a5
Show file tree
Hide file tree
Showing 9 changed files with 320 additions and 102 deletions.
3 changes: 2 additions & 1 deletion crates/uv-pep508/src/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -28,7 +28,8 @@ use url::Url;

use cursor::Cursor;
pub use marker::{
ContainsMarkerTree, ExtraMarkerTree, ExtraOperator, InMarkerTree, MarkerEnvironment,
ContainsMarkerTree, ExtraMarkerTree, ExtraOperator, InMarkerTree, LoweredMarkerValueExtra,
LoweredMarkerValueString, LoweredMarkerValueVersion, MarkerEnvironment,
MarkerEnvironmentBuilder, MarkerExpression, MarkerOperator, MarkerTree, MarkerTreeContents,
MarkerTreeKind, MarkerValue, MarkerValueExtra, MarkerValueString, MarkerValueVersion,
MarkerWarningKind, StringMarkerTree, StringVersion, VersionMarkerTree,
Expand Down
90 changes: 66 additions & 24 deletions crates/uv-pep508/src/marker/algebra.rs
Original file line number Diff line number Diff line change
Expand Up @@ -56,9 +56,12 @@ use std::sync::LazyLock;
use uv_pep440::{release_specifier_to_range, Operator, Version, VersionSpecifier};
use version_ranges::Ranges;

use crate::marker::lowering::{
LoweredMarkerValueExtra, LoweredMarkerValueString, LoweredMarkerValueVersion,
};
use crate::marker::MarkerValueExtra;
use crate::ExtraOperator;
use crate::{MarkerExpression, MarkerOperator, MarkerValueString, MarkerValueVersion};
use crate::{MarkerExpression, MarkerOperator, MarkerValueVersion};

/// The global node interner.
pub(crate) static INTERNER: LazyLock<Interner> = LazyLock::new(Interner::default);
Expand Down Expand Up @@ -161,7 +164,7 @@ impl InternerGuard<'_> {
specifier,
} => match python_version_to_full_version(normalize_specifier(specifier)) {
Ok(specifier) => (
Variable::Version(MarkerValueVersion::PythonFullVersion),
Variable::Version(LoweredMarkerValueVersion::PythonFullVersion),
Edges::from_specifier(specifier),
),
Err(node) => return node,
Expand All @@ -172,24 +175,25 @@ impl InternerGuard<'_> {
negated,
} => match Edges::from_python_versions(versions, negated) {
Ok(edges) => (
Variable::Version(MarkerValueVersion::PythonFullVersion),
Variable::Version(LoweredMarkerValueVersion::PythonFullVersion),
edges,
),
Err(node) => return node,
},
// A variable representing the output of a version key. Edges correspond
// to disjoint version ranges.
MarkerExpression::Version { key, specifier } => {
(Variable::Version(key), Edges::from_specifier(specifier))
}
MarkerExpression::Version { key, specifier } => (
Variable::Version(key.into()),
Edges::from_specifier(specifier),
),
// A variable representing the output of a version key. Edges correspond
// to disjoint version ranges.
MarkerExpression::VersionIn {
key,
versions,
negated,
} => (
Variable::Version(key),
Variable::Version(key.into()),
Edges::from_versions(&versions, negated),
),
// The `in` and `contains` operators are a bit different than other operators.
Expand All @@ -206,38 +210,76 @@ impl InternerGuard<'_> {
key,
operator: MarkerOperator::In,
value,
} => (Variable::In { key, value }, Edges::from_bool(true)),
} => (
Variable::In {
key: key.into(),
value,
},
Edges::from_bool(true),
),
MarkerExpression::String {
key,
operator: MarkerOperator::NotIn,
value,
} => (Variable::In { key, value }, Edges::from_bool(false)),
} => (
Variable::In {
key: key.into(),
value,
},
Edges::from_bool(false),
),
MarkerExpression::String {
key,
operator: MarkerOperator::Contains,
value,
} => (Variable::Contains { key, value }, Edges::from_bool(true)),
} => (
Variable::Contains {
key: key.into(),
value,
},
Edges::from_bool(true),
),
MarkerExpression::String {
key,
operator: MarkerOperator::NotContains,
value,
} => (Variable::Contains { key, value }, Edges::from_bool(false)),
} => (
Variable::Contains {
key: key.into(),
value,
},
Edges::from_bool(false),
),
// A variable representing the output of a string key. Edges correspond
// to disjoint string ranges.
MarkerExpression::String {
key,
operator,
value,
} => (Variable::String(key), Edges::from_string(operator, value)),
} => (
Variable::String(key.into()),
Edges::from_string(operator, value),
),
// A variable representing the existence or absence of a particular extra.
MarkerExpression::Extra {
name,
name: MarkerValueExtra::Extra(extra),
operator: ExtraOperator::Equal,
} => (Variable::Extra(name), Edges::from_bool(true)),
} => (
Variable::Extra(LoweredMarkerValueExtra::Extra(extra)),
Edges::from_bool(true),
),
MarkerExpression::Extra {
name,
name: MarkerValueExtra::Extra(extra),
operator: ExtraOperator::NotEqual,
} => (Variable::Extra(name), Edges::from_bool(false)),
} => (
Variable::Extra(LoweredMarkerValueExtra::Extra(extra)),
Edges::from_bool(false),
),
// Invalid extras are always `false`.
MarkerExpression::Extra {
name: MarkerValueExtra::Arbitrary(_),
..
} => return NodeId::FALSE,
};

self.create_node(var, children)
Expand Down Expand Up @@ -391,7 +433,7 @@ impl InternerGuard<'_> {
// Look for a `python_full_version` expression, otherwise
// we recursively simplify.
let Node {
var: Variable::Version(MarkerValueVersion::PythonFullVersion),
var: Variable::Version(LoweredMarkerValueVersion::PythonFullVersion),
children: Edges::Version { ref edges },
} = node
else {
Expand Down Expand Up @@ -464,7 +506,7 @@ impl InternerGuard<'_> {
return NodeId::FALSE;
}
if matches!(i, NodeId::TRUE) {
let var = Variable::Version(MarkerValueVersion::PythonFullVersion);
let var = Variable::Version(LoweredMarkerValueVersion::PythonFullVersion);
let edges = Edges::Version {
edges: Edges::from_range(&py_range),
};
Expand All @@ -473,7 +515,7 @@ impl InternerGuard<'_> {

let node = self.shared.node(i);
let Node {
var: Variable::Version(MarkerValueVersion::PythonFullVersion),
var: Variable::Version(LoweredMarkerValueVersion::PythonFullVersion),
children: Edges::Version { ref edges },
} = node
else {
Expand Down Expand Up @@ -569,26 +611,26 @@ pub(crate) enum Variable {
///
/// This is the highest order variable as it typically contains the most complex
/// ranges, allowing us to merge ranges at the top-level.
Version(MarkerValueVersion),
Version(LoweredMarkerValueVersion),
/// A string marker, such as `os_name`.
String(MarkerValueString),
String(LoweredMarkerValueString),
/// A variable representing a `<key> in <value>` expression for a particular
/// string marker and value.
In {
key: MarkerValueString,
key: LoweredMarkerValueString,
value: String,
},
/// A variable representing a `<value> in <key>` expression for a particular
/// string marker and value.
Contains {
key: MarkerValueString,
key: LoweredMarkerValueString,
value: String,
},
/// A variable representing the existence or absence of a given extra.
///
/// We keep extras at the leaves of the tree, so when simplifying extras we can
/// trivially remove the leaves without having to reconstruct the entire tree.
Extra(MarkerValueExtra),
Extra(LoweredMarkerValueExtra),
}

/// A decision node in an Algebraic Decision Diagram.
Expand Down
43 changes: 22 additions & 21 deletions crates/uv-pep508/src/marker/environment.rs
Original file line number Diff line number Diff line change
Expand Up @@ -2,7 +2,7 @@ use std::sync::Arc;

use uv_pep440::{Version, VersionParseError};

use crate::{MarkerValueString, MarkerValueVersion, StringVersion};
use crate::{LoweredMarkerValueString, LoweredMarkerValueVersion, StringVersion};

/// The marker values for a python interpreter, normally the current one
///
Expand Down Expand Up @@ -33,35 +33,36 @@ struct MarkerEnvironmentInner {

impl MarkerEnvironment {
/// Returns of the PEP 440 version typed value of the key in the current environment
pub fn get_version(&self, key: MarkerValueVersion) -> &Version {
pub fn get_version(&self, key: LoweredMarkerValueVersion) -> &Version {
match key {
MarkerValueVersion::ImplementationVersion => &self.implementation_version().version,
MarkerValueVersion::PythonFullVersion => &self.python_full_version().version,
MarkerValueVersion::PythonVersion => &self.python_version().version,
LoweredMarkerValueVersion::ImplementationVersion => {
&self.implementation_version().version
}
LoweredMarkerValueVersion::PythonFullVersion => &self.python_full_version().version,
LoweredMarkerValueVersion::PythonVersion => &self.python_version().version,
}
}

/// Returns of the stringly typed value of the key in the current environment
pub fn get_string(&self, key: MarkerValueString) -> &str {
pub fn get_string(&self, key: LoweredMarkerValueString) -> &str {
match key {
MarkerValueString::ImplementationName => self.implementation_name(),
MarkerValueString::OsName | MarkerValueString::OsNameDeprecated => self.os_name(),
MarkerValueString::PlatformMachine | MarkerValueString::PlatformMachineDeprecated => {
self.platform_machine()
LoweredMarkerValueString::ImplementationName => self.implementation_name(),
LoweredMarkerValueString::OsName | LoweredMarkerValueString::OsNameDeprecated => {
self.os_name()
}
MarkerValueString::PlatformPythonImplementation
| MarkerValueString::PlatformPythonImplementationDeprecated
| MarkerValueString::PythonImplementationDeprecated => {
LoweredMarkerValueString::PlatformMachine
| LoweredMarkerValueString::PlatformMachineDeprecated => self.platform_machine(),
LoweredMarkerValueString::PlatformPythonImplementation
| LoweredMarkerValueString::PlatformPythonImplementationDeprecated
| LoweredMarkerValueString::PythonImplementationDeprecated => {
self.platform_python_implementation()
}
MarkerValueString::PlatformRelease => self.platform_release(),
MarkerValueString::PlatformSystem => self.platform_system(),
MarkerValueString::PlatformVersion | MarkerValueString::PlatformVersionDeprecated => {
self.platform_version()
}
MarkerValueString::SysPlatform | MarkerValueString::SysPlatformDeprecated => {
self.sys_platform()
}
LoweredMarkerValueString::PlatformRelease => self.platform_release(),
LoweredMarkerValueString::PlatformSystem => self.platform_system(),
LoweredMarkerValueString::PlatformVersion
| LoweredMarkerValueString::PlatformVersionDeprecated => self.platform_version(),
LoweredMarkerValueString::SysPlatform
| LoweredMarkerValueString::SysPlatformDeprecated => self.sys_platform(),
}
}
}
Expand Down
Loading

0 comments on commit 47cb7a5

Please sign in to comment.