From aec721f4fa9086dacd7a8f0603bf4d907ea56880 Mon Sep 17 00:00:00 2001 From: Mike Gerwitz Date: Tue, 25 Jul 2023 15:35:57 -0400 Subject: [PATCH] tamer: asg::graph: Work AsgRelMut specialization into `object_rel!` macro This formalizes the previous commit a bit more and adds documentation explaining why it exists and how it works. Look there for more information. This has been a lot of setup work. Hopefully things are now easier in the future. And now we have nice declarative type-level hooks into the graph! DEV-13163 --- tamer/src/asg/graph.rs | 98 ++++++++++++++++++++++++++++- tamer/src/asg/graph/object/doc.rs | 2 - tamer/src/asg/graph/object/expr.rs | 2 - tamer/src/asg/graph/object/ident.rs | 2 - tamer/src/asg/graph/object/meta.rs | 2 - tamer/src/asg/graph/object/pkg.rs | 2 - tamer/src/asg/graph/object/rel.rs | 13 +++- tamer/src/asg/graph/object/root.rs | 2 - tamer/src/asg/graph/object/tpl.rs | 52 +++++---------- 9 files changed, 123 insertions(+), 52 deletions(-) diff --git a/tamer/src/asg/graph.rs b/tamer/src/asg/graph.rs index 4daa1732..2dc7e9d6 100644 --- a/tamer/src/asg/graph.rs +++ b/tamer/src/asg/graph.rs @@ -40,6 +40,9 @@ use petgraph::{ }; use std::{fmt::Debug, result::Result}; +#[cfg(doc)] +use object::{ObjectIndexTo, Tpl}; + pub mod object; pub mod visit; pub mod xmli; @@ -403,7 +406,79 @@ fn diagnostic_node_missing_desc( /// trait, /// but the current module structure together with Rust's visibility /// with sibling modules doesn't seem to make that possible. -pub trait AsgRelMut: ObjectKind { +/// +/// How Does This Work With Trait Specialization? +/// ============================================= +/// [`Asg::add_edge`] is provided a [`ObjectIndexRelTo`], +/// which needs narrowing to an appropriate source [`ObjectKind`] so that +/// we can invoke [`::pre_add_edge`](AsgRelMut::pre_add_edge). +/// +/// At the time of writing, +/// there are two implementors of [`ObjectIndexRelTo`]: +/// +/// - [`ObjectIndex`], +/// for which we will know `O: ObjectKind`. +/// - [`ObjectIndexTo`], +/// for which we only know the _target_ `OB: ObjectKind`. +/// +/// The entire purpose of [`ObjectIndexTo`] is to allow for a dynamic +/// source [`ObjectKind`]; +/// we do not know what it is statically. +/// So [`ObjectIndexTo::pre_add_edge`] is a method that will dynamically +/// branch to an appropriate static path to invoke the correct +/// [`AsgRelMut::pre_add_edge`]. +/// +/// And there's the problem. +/// +/// We match on each [`ObjectRelTy`] based on +/// [`ObjectIndexTo::src_rel_ty`], +/// and invoke the appropriate [`AsgRelMut::pre_add_edge`]. +/// But the trait bound on `OB` for the `ObjectIndexRelTo` `impl` is +/// [`ObjectRelatable`]. +/// So it resolves as `AsgRelMut`. +/// +/// But we don't have that implementation. +/// We have implementations for _individual target [`ObjectRelatable`]s, +/// e.g. `impl AsgRelMut for Tpl`. +/// So Rust rightfully complains that `AsgRelMut` +/// is not implemented for [`Tpl`]. +/// (Go ahead and remove the generic `impl` block containing `default fn` +/// and see what happens.) +/// +/// Of course, +/// _we_ know that there's a trait implemented for every possible +/// [`ObjectRelFrom`], +/// because `object_rel!` does that for us based on the same +/// definition that generates those other types. +/// But Rust does not perform that type of analysis---​ +/// it does not know that we've accounted for every type. +/// So the `default fn`` uses the unstable `min_specialization` feature to +/// satisfy those more generic trait bounds, +/// making the compiler happy. +/// +/// But if Rust is seeing `OB: ObjectRelatable`, +/// why is it not monomorphizing to _this_ one rather than the more +/// specialized implementation? +/// +/// That type error described above is contemplating bounds for _any +/// potential caller_. +/// But when we're about to add an edge, +/// we're invoking with a specific type of `OB`. +/// Monomorphization takes place at that point, +/// with the expected type, +/// and uses the appropriate specialization. +/// +/// Because of other trait bounds leading up to this point, +/// including those on [`Asg::add_edge`] and [`ObjectIndexRelTo`], +/// this cannot be invoked for any `to_oi` that is not a valid target +/// for `Self`. +/// But we cannot be too strict on that bound _here_, +/// because otherwise it's not general enough for +/// [`ObjectIndexTo::pre_add_edge`]. +/// We could do more runtime verification and further refine types, +/// but that is a lot of work for no additional practical benefit, +/// at least at this time. +pub trait AsgRelMut: ObjectRelatable { /// Allow an object to handle or reject the creation of an edge from it /// to another object. /// @@ -440,6 +515,27 @@ pub trait AsgRelMut: ObjectKind { _to_oi: ObjectIndex, _ctx_span: Option, commit: impl FnOnce(&mut Asg), + ) -> Result<(), AsgError>; +} + +impl AsgRelMut for O { + /// Default edge creation method for all [`ObjectKind`]s. + /// + /// This takes the place of a default implementation on the trait itself + /// above. + /// It will be invoked any time there is not a more specialized + /// implementation. + /// Note that `object_rel!` doesn't provide method + /// definitions unless explicitly specified by the user, + /// so this is effective the method called for all edges _unless_ + /// overridden for a particular edge for a particular object + /// (see [`object::tpl`] as an example). + default fn pre_add_edge( + asg: &mut Asg, + _from_oi: ObjectIndex, + _to_oi: ObjectIndex, + _ctx_span: Option, + commit: impl FnOnce(&mut Asg), ) -> Result<(), AsgError> { Ok(commit(asg)) } diff --git a/tamer/src/asg/graph/object/doc.rs b/tamer/src/asg/graph/object/doc.rs index d125009a..29fb903a 100644 --- a/tamer/src/asg/graph/object/doc.rs +++ b/tamer/src/asg/graph/object/doc.rs @@ -91,5 +91,3 @@ object_rel! { // empty } } - -impl AsgRelMut for Doc {} diff --git a/tamer/src/asg/graph/object/expr.rs b/tamer/src/asg/graph/object/expr.rs index d131f7cb..b2d1e86b 100644 --- a/tamer/src/asg/graph/object/expr.rs +++ b/tamer/src/asg/graph/object/expr.rs @@ -268,5 +268,3 @@ impl ObjectIndex { self.add_edge_from(asg, oi_container, None) } } - -impl AsgRelMut for Expr {} diff --git a/tamer/src/asg/graph/object/ident.rs b/tamer/src/asg/graph/object/ident.rs index 3e9a3193..6ec4a6c8 100644 --- a/tamer/src/asg/graph/object/ident.rs +++ b/tamer/src/asg/graph/object/ident.rs @@ -1406,7 +1406,5 @@ impl ObjectIndex { } } -impl AsgRelMut for Ident {} - #[cfg(test)] mod test; diff --git a/tamer/src/asg/graph/object/meta.rs b/tamer/src/asg/graph/object/meta.rs index 6a6a9d1b..15db6c60 100644 --- a/tamer/src/asg/graph/object/meta.rs +++ b/tamer/src/asg/graph/object/meta.rs @@ -306,5 +306,3 @@ impl ObjectIndex { self.add_edge_to(asg, oi_ref, Some(oi_ref.span())) } } - -impl AsgRelMut for Meta {} diff --git a/tamer/src/asg/graph/object/pkg.rs b/tamer/src/asg/graph/object/pkg.rs index 1773c995..c07f5242 100644 --- a/tamer/src/asg/graph/object/pkg.rs +++ b/tamer/src/asg/graph/object/pkg.rs @@ -147,5 +147,3 @@ impl ObjectIndex { self.add_edge_to(asg, oi_doc, None) } } - -impl AsgRelMut for Pkg {} diff --git a/tamer/src/asg/graph/object/rel.rs b/tamer/src/asg/graph/object/rel.rs index d2419824..6a789422 100644 --- a/tamer/src/asg/graph/object/rel.rs +++ b/tamer/src/asg/graph/object/rel.rs @@ -58,7 +58,7 @@ macro_rules! object_rel { ( $(#[$attr:meta])+ $from:ident -> { - $($ety:ident $kind:ident,)* + $($ety:ident $kind:ident $({$($impl:tt)*})?,)* } $(can_recurse($rec_obj:ident) if $rec_expr:expr)? ) => {paste::paste! { @@ -171,6 +171,17 @@ macro_rules! object_rel { } } } + + // This generates a specialized implementation _per target `$kind`_ + // and allows for the caller to override methods on the trait. + // This takes advantage of trait specialization via + // `min_specialization`; + // see `AsgRelMut` for more information. + $( + impl AsgRelMut<$kind> for $from { + $( $($impl)* )? + } + )* }}; // Static edge types. diff --git a/tamer/src/asg/graph/object/root.rs b/tamer/src/asg/graph/object/root.rs index 6cfd536c..32c90db6 100644 --- a/tamer/src/asg/graph/object/root.rs +++ b/tamer/src/asg/graph/object/root.rs @@ -70,5 +70,3 @@ impl ObjectIndex { oi.add_edge_from(asg, *self, None) } } - -impl AsgRelMut for Root {} diff --git a/tamer/src/asg/graph/object/tpl.rs b/tamer/src/asg/graph/object/tpl.rs index 47199ea0..e81b97ff 100644 --- a/tamer/src/asg/graph/object/tpl.rs +++ b/tamer/src/asg/graph/object/tpl.rs @@ -167,7 +167,20 @@ object_rel! { Tpl -> { // Expressions must be able to be anonymous to allow templates in // any `Expr` context. - tree Expr, + tree Expr { + fn pre_add_edge( + asg: &mut Asg, + from_oi: ObjectIndex, + to_oi: ObjectIndex, + _ctx_span: Option, + commit: impl FnOnce(&mut Asg), + ) -> Result<(), AsgError> { + let span = to_oi.resolve(asg).span(); + from_oi.map_obj(asg, |tpl| tpl.overwrite(TplShape::Expr(span))); + + Ok(commit(asg)) + } + }, // Identifiers are used for both references and identifiers that // will expand into an application site. @@ -238,40 +251,3 @@ impl ObjectIndex { self.add_edge_to(asg, oi_doc, None) } } - -impl AsgRelMut for Tpl { - fn pre_add_edge( - asg: &mut Asg, - from_oi: ObjectIndex, - to_oi: ObjectIndex, - _ctx_span: Option, - commit: impl FnOnce(&mut Asg), - ) -> Result<(), AsgError> { - let span = to_oi.resolve(asg).span(); - from_oi.map_obj(asg, |tpl| tpl.overwrite(TplShape::Expr(span))); - - Ok(commit(asg)) - } -} - -// TODO: Merge this into the macro above -impl AsgRelMut for Tpl {} -impl AsgRelMut for Tpl {} -impl AsgRelMut for Tpl {} - -// This uses `min_specialization` to satisfy trait bounds for -// `::add_edge`. -// This will be better integrated in future commits. -// See message of the commit that introduced this comment for more -// information. -impl AsgRelMut for Tpl { - default fn pre_add_edge( - asg: &mut Asg, - _from_oi: ObjectIndex, - _to_oi: ObjectIndex, - _ctx_span: Option, - commit: impl FnOnce(&mut Asg), - ) -> Result<(), AsgError> { - Ok(commit(asg)) - } -}