From a468f5a3db616ad19046462414f8fdc4772f8d95 Mon Sep 17 00:00:00 2001 From: Andrew Gallant Date: Sat, 23 Nov 2024 10:51:42 -0500 Subject: [PATCH] uv-resolver: add new ConflictMarker type This is strictly a refactor. It doesn't change any behavior. The purpose of this refactor is to make room for supporting more complex conflict markers. Namely, we need to incorporate package names and then eventually group names. But we also need sophisticated boolean algebraic simplification, and the only (cheap, in terms of development cost) to do that at the moment is to encode our semantics into PEP 508 markers. So this type will own that encapsulation. (Right now, there's no real translation happening because we only support extra names, which PEP 508 already supports.) --- crates/uv-resolver/src/lib.rs | 2 +- crates/uv-resolver/src/lock/mod.rs | 23 +- .../uv-resolver/src/lock/requirements_txt.rs | 6 +- ...missing_dependency_source_unambiguous.snap | 2 +- ...dependency_source_version_unambiguous.snap | 2 +- ...issing_dependency_version_unambiguous.snap | 2 +- .../uv-resolver/src/resolver/environment.rs | 17 +- crates/uv-resolver/src/resolver/mod.rs | 4 +- crates/uv-resolver/src/universal_marker.rs | 248 +++++++++++++++--- 9 files changed, 239 insertions(+), 67 deletions(-) diff --git a/crates/uv-resolver/src/lib.rs b/crates/uv-resolver/src/lib.rs index 90cd4634ce795..bce29ec2eff34 100644 --- a/crates/uv-resolver/src/lib.rs +++ b/crates/uv-resolver/src/lib.rs @@ -22,7 +22,7 @@ pub use resolver::{ PackageVersionsResult, Reporter as ResolverReporter, Resolver, ResolverEnvironment, ResolverProvider, VersionsResponse, WheelMetadataResult, }; -pub use universal_marker::UniversalMarker; +pub use universal_marker::{ConflictMarker, UniversalMarker}; pub use version_map::VersionMap; pub use yanks::AllowedYanks; diff --git a/crates/uv-resolver/src/lock/mod.rs b/crates/uv-resolver/src/lock/mod.rs index cb8a192f9cb7f..2c43f99c6be24 100644 --- a/crates/uv-resolver/src/lock/mod.rs +++ b/crates/uv-resolver/src/lock/mod.rs @@ -19,7 +19,7 @@ pub use crate::lock::target::InstallTarget; pub use crate::lock::tree::TreeDisplay; use crate::requires_python::SimplifiedMarkerTree; use crate::resolution::{AnnotatedDist, ResolutionGraphNode}; -use crate::universal_marker::UniversalMarker; +use crate::universal_marker::{ConflictMarker, UniversalMarker}; use crate::{ ExcludeNewer, InMemoryIndex, MetadataResponse, PrereleaseMode, RequiresPython, ResolutionMode, ResolverOutput, @@ -61,21 +61,21 @@ static LINUX_MARKERS: LazyLock = LazyLock::new(|| { "platform_system == 'Linux' and os_name == 'posix' and sys_platform == 'linux'", ) .unwrap(); - UniversalMarker::new(pep508, MarkerTree::TRUE) + UniversalMarker::new(pep508, ConflictMarker::TRUE) }); static WINDOWS_MARKERS: LazyLock = LazyLock::new(|| { let pep508 = MarkerTree::from_str( "platform_system == 'Windows' and os_name == 'nt' and sys_platform == 'win32'", ) .unwrap(); - UniversalMarker::new(pep508, MarkerTree::TRUE) + UniversalMarker::new(pep508, ConflictMarker::TRUE) }); static MAC_MARKERS: LazyLock = LazyLock::new(|| { let pep508 = MarkerTree::from_str( "platform_system == 'Darwin' and os_name == 'posix' and sys_platform == 'darwin'", ) .unwrap(); - UniversalMarker::new(pep508, MarkerTree::TRUE) + UniversalMarker::new(pep508, ConflictMarker::TRUE) }); #[derive(Clone, Debug, serde::Deserialize)] @@ -1446,7 +1446,9 @@ impl TryFrom for Lock { .map(|simplified_marker| simplified_marker.into_marker(&wire.requires_python)) // TODO(ag): Consider whether this should also deserialize a conflict marker. // We currently aren't serializing. Dropping it completely is likely to be wrong. - .map(|complexified_marker| UniversalMarker::new(complexified_marker, MarkerTree::TRUE)) + .map(|complexified_marker| { + UniversalMarker::new(complexified_marker, ConflictMarker::TRUE) + }) .collect(); let lock = Lock::new( wire.version, @@ -2245,7 +2247,7 @@ impl PackageWire { // TODO(ag): Consider whether this should also deserialize a conflict marker. // We currently aren't serializing. Dropping it completely is likely to be wrong. .map(|complexified_marker| { - UniversalMarker::new(complexified_marker, MarkerTree::TRUE) + UniversalMarker::new(complexified_marker, ConflictMarker::TRUE) }) .collect(), dependencies: unwire_deps(self.dependencies)?, @@ -3580,8 +3582,11 @@ impl Dependency { if let Some(marker) = self.simplified_marker.try_to_string() { table.insert("marker", value(marker)); } - if let Some(conflict_marker) = self.complexified_marker.conflict().try_to_string() { - table.insert("conflict-marker", value(conflict_marker)); + if !self.complexified_marker.conflict().is_true() { + table.insert( + "conflict-marker", + value(self.complexified_marker.conflict().to_string()), + ); } table @@ -3619,7 +3624,7 @@ struct DependencyWire { #[serde(default)] marker: SimplifiedMarkerTree, #[serde(rename = "conflict-marker", default)] - conflict_marker: MarkerTree, + conflict_marker: ConflictMarker, } impl DependencyWire { diff --git a/crates/uv-resolver/src/lock/requirements_txt.rs b/crates/uv-resolver/src/lock/requirements_txt.rs index 21635809b5304..239794f045e0b 100644 --- a/crates/uv-resolver/src/lock/requirements_txt.rs +++ b/crates/uv-resolver/src/lock/requirements_txt.rs @@ -20,7 +20,7 @@ use uv_pypi_types::{ParsedArchiveUrl, ParsedGitUrl}; use crate::graph_ops::marker_reachability; use crate::lock::{LockErrorKind, Package, PackageId, Source}; -use crate::universal_marker::UniversalMarker; +use crate::universal_marker::{ConflictMarker, UniversalMarker}; use crate::{InstallTarget, LockError}; /// An export of a [`Lock`] that renders in `requirements.txt` format. @@ -123,7 +123,7 @@ impl<'lock> RequirementsTxtExport<'lock> { // `marker_reachability` wants and it (probably) isn't // worth inventing a new abstraction so that it can accept // graphs with either `MarkerTree` or `UniversalMarker`. - MarkerTree::TRUE, + ConflictMarker::TRUE, ), ); @@ -172,7 +172,7 @@ impl<'lock> RequirementsTxtExport<'lock> { dep.simplified_marker.as_simplified_marker_tree().clone(), // See note above for other `UniversalMarker::new` for // why this is OK. - MarkerTree::TRUE, + ConflictMarker::TRUE, ), ); diff --git a/crates/uv-resolver/src/lock/snapshots/uv_resolver__lock__tests__missing_dependency_source_unambiguous.snap b/crates/uv-resolver/src/lock/snapshots/uv_resolver__lock__tests__missing_dependency_source_unambiguous.snap index 0ee9dabab8330..52b180c7e3298 100644 --- a/crates/uv-resolver/src/lock/snapshots/uv_resolver__lock__tests__missing_dependency_source_unambiguous.snap +++ b/crates/uv-resolver/src/lock/snapshots/uv_resolver__lock__tests__missing_dependency_source_unambiguous.snap @@ -137,7 +137,7 @@ Ok( ), complexified_marker: UniversalMarker { pep508_marker: python_full_version >= '3.12', - conflict_marker: true, + conflict_marker: ConflictMarker(true), }, }, ], diff --git a/crates/uv-resolver/src/lock/snapshots/uv_resolver__lock__tests__missing_dependency_source_version_unambiguous.snap b/crates/uv-resolver/src/lock/snapshots/uv_resolver__lock__tests__missing_dependency_source_version_unambiguous.snap index 0ee9dabab8330..52b180c7e3298 100644 --- a/crates/uv-resolver/src/lock/snapshots/uv_resolver__lock__tests__missing_dependency_source_version_unambiguous.snap +++ b/crates/uv-resolver/src/lock/snapshots/uv_resolver__lock__tests__missing_dependency_source_version_unambiguous.snap @@ -137,7 +137,7 @@ Ok( ), complexified_marker: UniversalMarker { pep508_marker: python_full_version >= '3.12', - conflict_marker: true, + conflict_marker: ConflictMarker(true), }, }, ], diff --git a/crates/uv-resolver/src/lock/snapshots/uv_resolver__lock__tests__missing_dependency_version_unambiguous.snap b/crates/uv-resolver/src/lock/snapshots/uv_resolver__lock__tests__missing_dependency_version_unambiguous.snap index 0ee9dabab8330..52b180c7e3298 100644 --- a/crates/uv-resolver/src/lock/snapshots/uv_resolver__lock__tests__missing_dependency_version_unambiguous.snap +++ b/crates/uv-resolver/src/lock/snapshots/uv_resolver__lock__tests__missing_dependency_version_unambiguous.snap @@ -137,7 +137,7 @@ Ok( ), complexified_marker: UniversalMarker { pep508_marker: python_full_version >= '3.12', - conflict_marker: true, + conflict_marker: ConflictMarker(true), }, }, ], diff --git a/crates/uv-resolver/src/resolver/environment.rs b/crates/uv-resolver/src/resolver/environment.rs index 78ad18311ead9..2af4f11ce5d31 100644 --- a/crates/uv-resolver/src/resolver/environment.rs +++ b/crates/uv-resolver/src/resolver/environment.rs @@ -6,7 +6,7 @@ use uv_pypi_types::{ConflictItem, ConflictItemRef, ResolverMarkerEnvironment}; use crate::pubgrub::{PubGrubDependency, PubGrubPackage}; use crate::requires_python::RequiresPythonRange; use crate::resolver::ForkState; -use crate::universal_marker::UniversalMarker; +use crate::universal_marker::{ConflictMarker, UniversalMarker}; use crate::PythonRequirement; /// Represents one or more marker environments for a resolution. @@ -380,23 +380,16 @@ impl ResolverEnvironment { .. } => { // FIXME: Account for groups here in addition to extras. - let mut conflict_marker = MarkerTree::TRUE; + let mut conflict_marker = ConflictMarker::TRUE; for item in exclude.iter() { if let Some(extra) = item.extra() { - let operator = uv_pep508::ExtraOperator::NotEqual; - let name = uv_pep508::MarkerValueExtra::Extra(extra.clone()); - let expr = uv_pep508::MarkerExpression::Extra { operator, name }; - let exclude_extra_marker = MarkerTree::expression(expr); - conflict_marker.and(exclude_extra_marker); + conflict_marker = + conflict_marker.and(ConflictMarker::extra(extra.clone()).negate()); } } for item in include.iter() { if let Some(extra) = item.extra() { - let operator = uv_pep508::ExtraOperator::Equal; - let name = uv_pep508::MarkerValueExtra::Extra(extra.clone()); - let expr = uv_pep508::MarkerExpression::Extra { operator, name }; - let exclude_extra_marker = MarkerTree::expression(expr); - conflict_marker.and(exclude_extra_marker); + conflict_marker = conflict_marker.and(ConflictMarker::extra(extra.clone())); } } Some(UniversalMarker::new(markers.clone(), conflict_marker)) diff --git a/crates/uv-resolver/src/resolver/mod.rs b/crates/uv-resolver/src/resolver/mod.rs index 1edffb9656cd1..deab61c4d9901 100644 --- a/crates/uv-resolver/src/resolver/mod.rs +++ b/crates/uv-resolver/src/resolver/mod.rs @@ -63,7 +63,7 @@ pub(crate) use crate::resolver::availability::{ }; use crate::resolver::batch_prefetch::BatchPrefetcher; pub use crate::resolver::derivation::DerivationChainBuilder; -use crate::universal_marker::UniversalMarker; +use crate::universal_marker::{ConflictMarker, UniversalMarker}; use crate::resolver::groups::Groups; pub use crate::resolver::index::InMemoryIndex; @@ -2617,7 +2617,7 @@ impl ResolutionDependencyEdge { // We specifically do not account for conflict // markers here. Instead, those are computed via // a traversal on the resolution graph. - UniversalMarker::new(self.marker.clone(), MarkerTree::TRUE) + UniversalMarker::new(self.marker.clone(), ConflictMarker::TRUE) } } diff --git a/crates/uv-resolver/src/universal_marker.rs b/crates/uv-resolver/src/universal_marker.rs index 1d09ba21457b8..23f8daca3d36d 100644 --- a/crates/uv-resolver/src/universal_marker.rs +++ b/crates/uv-resolver/src/universal_marker.rs @@ -1,10 +1,18 @@ +#![allow(warnings)] + use itertools::Itertools; use uv_configuration::{DevGroupsManifest, ExtrasSpecification}; use uv_normalize::ExtraName; -use uv_pep508::{MarkerEnvironment, MarkerTree}; +use uv_pep508::{MarkerEnvironment, MarkerEnvironmentBuilder, MarkerTree}; use uv_pypi_types::Conflicts; +// BREADCRUMBS: Work on a new `ConflictMarker` type instead of using +// `MarkerTree` directly below. And instead of storing only extra/group +// names, we need to store package names too. Wire everything up. This +// will also take us toward making groups work. But keep going with just +// extras for now. + /// A representation of a marker for use in universal resolution. /// /// (This also degrades gracefully to a standard PEP 508 marker in the case of @@ -20,24 +28,27 @@ use uv_pypi_types::Conflicts; #[derive(Debug, Default, Clone, Eq, Hash, PartialEq, PartialOrd, Ord)] pub struct UniversalMarker { pep508_marker: MarkerTree, - conflict_marker: MarkerTree, + conflict_marker: ConflictMarker, } impl UniversalMarker { /// A constant universal marker that always evaluates to `true`. pub(crate) const TRUE: UniversalMarker = UniversalMarker { pep508_marker: MarkerTree::TRUE, - conflict_marker: MarkerTree::TRUE, + conflict_marker: ConflictMarker::TRUE, }; /// A constant universal marker that always evaluates to `false`. pub(crate) const FALSE: UniversalMarker = UniversalMarker { pep508_marker: MarkerTree::FALSE, - conflict_marker: MarkerTree::FALSE, + conflict_marker: ConflictMarker::FALSE, }; /// Creates a new universal marker from its constituent pieces. - pub(crate) fn new(pep508_marker: MarkerTree, conflict_marker: MarkerTree) -> UniversalMarker { + pub(crate) fn new( + pep508_marker: MarkerTree, + conflict_marker: ConflictMarker, + ) -> UniversalMarker { UniversalMarker { pep508_marker, conflict_marker, @@ -49,7 +60,7 @@ impl UniversalMarker { /// `other` evaluate to `true`. pub(crate) fn or(&mut self, other: UniversalMarker) { self.pep508_marker.or(other.pep508_marker); - self.conflict_marker.or(other.conflict_marker); + self.conflict_marker = self.conflict_marker.or(other.conflict_marker); } /// Combine this universal marker with the one given in a way that @@ -57,7 +68,7 @@ impl UniversalMarker { /// `self` and `other` evaluate to `true`. pub(crate) fn and(&mut self, other: UniversalMarker) { self.pep508_marker.and(other.pep508_marker); - self.conflict_marker.and(other.conflict_marker); + self.conflict_marker = self.conflict_marker.and(other.conflict_marker); } /// Imbibes the world knowledge expressed by `conflicts` into this marker. @@ -76,7 +87,7 @@ impl UniversalMarker { // program. But it's doing it every time this routine // is called. We should refactor the caller to build // a marker from the `conflicts` once. - let mut marker = MarkerTree::FALSE; + let mut marker = ConflictMarker::FALSE; for set in conflicts.iter() { for (item1, item2) in set.iter().tuple_combinations() { // FIXME: Account for groups here. And extra/group @@ -84,26 +95,14 @@ impl UniversalMarker { let (Some(extra1), Some(extra2)) = (item1.extra(), item2.extra()) else { continue; }; - - let operator = uv_pep508::ExtraOperator::Equal; - let name = uv_pep508::MarkerValueExtra::Extra(extra1.clone()); - let expr = uv_pep508::MarkerExpression::Extra { operator, name }; - let marker1 = MarkerTree::expression(expr); - - let operator = uv_pep508::ExtraOperator::Equal; - let name = uv_pep508::MarkerValueExtra::Extra(extra2.clone()); - let expr = uv_pep508::MarkerExpression::Extra { operator, name }; - let marker2 = MarkerTree::expression(expr); - - let mut pair = MarkerTree::TRUE; - pair.and(marker1); - pair.and(marker2); - marker.or(pair); + let pair = ConflictMarker::extra(extra1.clone()) + .and(ConflictMarker::extra(extra2.clone())); + marker = marker.or(pair); } } - let mut marker = marker.negate(); - marker.implies(std::mem::take(&mut self.conflict_marker)); - self.conflict_marker = marker; + self.conflict_marker = marker + .negate() + .implies(std::mem::take(&mut self.conflict_marker)); } /// Assumes that a given extra is activated. @@ -111,8 +110,7 @@ impl UniversalMarker { /// This may simplify the conflicting marker component of this universal /// marker. pub(crate) fn assume_extra(&mut self, extra: &ExtraName) { - self.conflict_marker = std::mem::take(&mut self.conflict_marker) - .simplify_extras_with(|candidate| candidate == extra); + self.conflict_marker = self.conflict_marker.assume_extra(extra); } /// Returns true if this universal marker will always evaluate to `true`. @@ -137,7 +135,7 @@ impl UniversalMarker { /// Returns true if this universal marker is satisfied by the given /// marker environment and list of activated extras. pub(crate) fn evaluate(&self, env: &MarkerEnvironment, extras: &[ExtraName]) -> bool { - self.pep508_marker.evaluate(env, extras) && self.conflict_marker.evaluate(env, extras) + self.pep508_marker.evaluate(env, extras) && self.conflict_marker.evaluate(extras) } /// Returns true if this universal marker is satisfied by the given @@ -159,7 +157,7 @@ impl UniversalMarker { ExtrasSpecification::None => &[][..], ExtrasSpecification::Some(ref list) => list, }; - self.conflict_marker.evaluate(env, extra_list) + self.conflict_marker.evaluate(extra_list) } /// Returns the PEP 508 marker for this universal marker. @@ -185,7 +183,7 @@ impl UniversalMarker { /// of non-trivial conflict markers and fails if any are found. (Because /// conflict markers cannot be represented in the `requirements.txt` /// format.) - pub fn conflict(&self) -> &MarkerTree { + pub fn conflict(&self) -> &ConflictMarker { &self.conflict_marker } } @@ -197,14 +195,190 @@ impl std::fmt::Display for UniversalMarker { } match ( self.pep508_marker.contents(), - self.conflict_marker.contents(), + self.conflict_marker.is_true(), ) { - (None, None) => write!(f, "`true`"), - (Some(pep508), None) => write!(f, "`{pep508}`"), - (None, Some(conflict)) => write!(f, "`true` (conflict marker: `{conflict}`)"), - (Some(pep508), Some(conflict)) => { - write!(f, "`{pep508}` (conflict marker: `{conflict}`)") + (None, true) => write!(f, "`true`"), + (Some(pep508), true) => write!(f, "`{pep508}`"), + (None, false) => write!(f, "`true` (conflict marker: `{}`)", self.conflict_marker), + (Some(pep508), false) => { + write!( + f, + "`{pep508}` (conflict marker: `{}`)", + self.conflict_marker + ) } } } } + +#[derive(Default, Clone, Eq, Hash, PartialEq, PartialOrd, Ord)] +pub struct ConflictMarker { + marker: MarkerTree, +} + +impl ConflictMarker { + /// A constant conflict marker that always evaluates to `true`. + pub const TRUE: ConflictMarker = ConflictMarker { + marker: MarkerTree::TRUE, + }; + + /// A constant conflict marker that always evaluates to `false`. + pub const FALSE: ConflictMarker = ConflictMarker { + marker: MarkerTree::FALSE, + }; + + /// Create a conflict marker that is true only when the given extra is + /// activated. + pub fn extra(name: ExtraName) -> ConflictMarker { + let operator = uv_pep508::ExtraOperator::Equal; + let name = uv_pep508::MarkerValueExtra::Extra(name); + let expr = uv_pep508::MarkerExpression::Extra { operator, name }; + let marker = MarkerTree::expression(expr); + ConflictMarker { marker } + } + + /// Returns a new conflict marker that is the negation of this one. + #[must_use] + pub fn negate(&self) -> ConflictMarker { + ConflictMarker { + marker: self.marker.negate(), + } + } + + /// Returns a new conflict marker corresponding to the union of `self` and + /// `other`. + #[must_use] + pub fn or(&self, other: ConflictMarker) -> ConflictMarker { + let mut marker = self.marker.clone(); + marker.or(other.marker); + ConflictMarker { marker } + } + + /// Returns a new conflict marker corresponding to the intersection of + /// `self` and `other`. + #[must_use] + pub fn and(&self, other: ConflictMarker) -> ConflictMarker { + let mut marker = self.marker.clone(); + marker.and(other.marker); + ConflictMarker { marker } + } + + /// Returns a new conflict marker corresponding to the logical implication + /// of `self` and the given consequent. + /// + /// If the conflict marker returned is always `true`, then it can be said + /// that `self` implies `consequent`. + #[must_use] + pub fn implies(&self, other: ConflictMarker) -> ConflictMarker { + let mut marker = self.marker.clone(); + marker.implies(other.marker); + ConflictMarker { marker } + } + + /// Returns a new conflict marker with the given extra assumed to be true. + /// + /// This may simplify the marker. + #[must_use] + pub(crate) fn assume_extra(&self, extra: &ExtraName) -> ConflictMarker { + let marker = self + .marker + .clone() + .simplify_extras_with(|candidate| candidate == extra); + ConflictMarker { marker } + } + + /// Returns true if this conflict marker will always evaluate to `true`. + pub fn is_true(&self) -> bool { + self.marker.is_true() + } + + /// Returns true if this conflict marker will always evaluate to `false`. + pub fn is_false(&self) -> bool { + self.marker.is_false() + } + + /// Returns true if this conflict marker is disjoint with the one given. + /// + /// Two conflict markers are disjoint when it is impossible for them both + /// to evaluate to `true` simultaneously. + pub(crate) fn is_disjoint(&self, other: &ConflictMarker) -> bool { + self.marker.is_disjoint(&other.marker) + } + + /// Returns true if this conflict marker is satisfied by the given + /// list of activated extras. + pub(crate) fn evaluate(&self, extras: &[ExtraName]) -> bool { + static DUMMY: std::sync::LazyLock = std::sync::LazyLock::new(|| { + MarkerEnvironment::try_from(MarkerEnvironmentBuilder { + implementation_name: "", + implementation_version: "3.7", + os_name: "linux", + platform_machine: "", + platform_python_implementation: "", + platform_release: "", + platform_system: "", + platform_version: "", + python_full_version: "3.7", + python_version: "3.7", + sys_platform: "linux", + }) + .unwrap() + }); + self.marker.evaluate(&DUMMY, extras) + } +} + +impl std::fmt::Display for ConflictMarker { + fn fmt(&self, f: &mut std::fmt::Formatter) -> std::fmt::Result { + // TODO: This shouldn't be exposing the internal marker, + // but instead transforming it. + if self.marker.is_false() { + return write!(f, "false"); + } + let Some(contents) = self.marker.contents() else { + return write!(f, "true"); + }; + write!(f, "{contents}") + } +} + +impl std::fmt::Debug for ConflictMarker { + fn fmt(&self, f: &mut std::fmt::Formatter) -> std::fmt::Result { + // This intentionally exposes the underlying PEP 508 marker + // representation so that programmers can debug it if there's + // something wrong with it. + write!(f, "ConflictMarker({:?})", self.marker) + } +} + +// TODO: This should parse a custom format. But we expose +// the raw PEP 508 marker for now for simplicity. We'll +// write the custom parser later. +impl std::str::FromStr for ConflictMarker { + type Err = uv_pep508::Pep508Error; + + fn from_str(markers: &str) -> Result { + Ok(ConflictMarker { + marker: markers.parse()?, + }) + } +} + +impl<'de> serde::Deserialize<'de> for ConflictMarker { + fn deserialize(deserializer: D) -> Result + where + D: serde::Deserializer<'de>, + { + let s = String::deserialize(deserializer)?; + std::str::FromStr::from_str(&s).map_err(serde::de::Error::custom) + } +} + +impl serde::Serialize for ConflictMarker { + fn serialize(&self, serializer: S) -> Result + where + S: serde::Serializer, + { + serializer.serialize_str(&self.to_string()) + } +}