WIP give ComponentInterface members a pointer to their parent.
#1003
Conversation
| @Suppress("UNNECESSARY_SAFE_CALL") // codegen is much simpler if we unconditionally emit safe calls here | ||
| override fun destroy() { | ||
| when(this) { | ||
| {%- for variant in e.variants() %} | ||
| is {{ e.name()|class_name_kt }}.{{ variant.name()|class_name_kt }} -> { | ||
| {% for field in variant.fields() -%} | ||
| {%- if ci.type_contains_object_references(field.type_()) -%} | ||
| {%- if field.contains_object_references() -%} |
There was a problem hiding this comment.
This seems like a nice improvement in usability for the template logic.
| pub struct Enum<'a> { | ||
| pub(super) parent: &'a ComponentInterface, | ||
| pub(super) descr: &'a EnumDescr, | ||
| } |
There was a problem hiding this comment.
So the Enum struct is now a kind of "fat pointer" thing that contains both a pointer to the data and parent ref to the containing object.
| } | ||
|
|
||
| pub fn variants(&self) -> Vec<&Variant> { | ||
| self.variants.iter().collect() | ||
| pub fn variants(&self) -> Vec<Variant<'_, Self>> { |
There was a problem hiding this comment.
Here, when consuming code lists the variants of the enum, so see that we create a bunch of Variant instances on demand with the appropriate pointers.
| } | ||
|
|
||
| pub fn contains_object_references(&self, ci: &ComponentInterface) -> bool { | ||
| // *sigh* at the clone here, the relationship between a ComponentInterace | ||
| // and its contained types could use a bit of a cleanup. |
There was a problem hiding this comment.
Hey look, no more clone!
|
|
||
| #[derive(Debug)] | ||
| pub struct Variant<'a, Parent: CINode> { | ||
| pub(super) parent: &'a Parent, |
There was a problem hiding this comment.
So this starts to get a bit complicated...the Variant struct may be used as part of an Enum or as part of an Error, so its parent pointer type needs to be generic. The main property that we want from the parent pointer is the ability to get all the way up to the containing ComponentInterface, so I made a trait for that.
An alternative could be to have it point directly to the containing ComponentInterface rather than its containing object, which would make this more of an "owner" pointer than a "parent" pointer. But, I can imagine reasons why a variant might need to be able to inspect the properties of its containing Enum/Error.
| } | ||
| } | ||
|
|
||
| impl<'a, Parent: CINode + 'a> CINode for Variant<'a, Parent> { |
There was a problem hiding this comment.
I could imagine generating these impls automatically from some sort of macro.
| pub fn iter_enum_definitions(&self) -> Vec<Enum<'_>> { | ||
| self.enums | ||
| .iter() | ||
| .map(|e| Enum { |
There was a problem hiding this comment.
Here again we see that we're synthesizing the aggregate pointer thingy on demand.
| name: self.identifier.0.to_string(), | ||
| fields: self.members.body.convert(ci)?, | ||
| }) | ||
| } | ||
| } | ||
|
|
||
| // Represents an individual field on a Record. | ||
| #[derive(Debug)] | ||
| pub struct Field<'a, Parent: CINode> { |
There was a problem hiding this comment.
We re-use the Field struct in two places, for enum variants fields and record fields, hence the generic.
Which makes me wonder...I wonder if enum variants could actually contain a wrapped Record in the same way that errors contain a wrapped Enum...
| @@ -172,7 +200,7 @@ mod test { | |||
| assert_eq!(record.fields().len(), 1); | |||
| assert_eq!(record.fields()[0].name(), "field"); | |||
| assert_eq!(record.fields()[0].type_().canonical_name(), "u32"); | |||
| assert!(!record.fields()[0].required); | |||
| assert!(!record.fields()[0].required()); | |||
There was a problem hiding this comment.
We can no longer access the fields directly because they're hidden behind a pointer.
| pub fn contains_unsigned_types(&self, ci: &ComponentInterface) -> bool { | ||
| self.variants().iter().any(|v| { | ||
| v.fields() | ||
| pub fn contains_unsigned_types(&self, _ci: &ComponentInterface) -> bool { |
There was a problem hiding this comment.
What's the reason to keep _ci here? Is it necessary or just work-in-progress for now?
Here's something I wanted to toy with given recent work on refactoring our code-generation templates, and the desire for each interface member to be able to render itself. The issue is that sometimes we need to know information about the surrounding interface or its types in order to do the code generation, which shows up in awkward calls like:
What would it take to make this be just a plain method call without extra global context? Like so:
In a garbage-collected language, we might conveniently give the
Record,Enumetc objects a reference to their containingComponentInterfaceand be done with it. That's hard to do in the obvious way in Rust, and the usual workarounds like handing out anArc<RefCell<ComponentInterface>>seem extremely verbose/complex for our needs.Here's a sketch of something that might work, although I'm not convinced of the value/complexity tradeoff.
Taking the
Enumstruct as an example, I've split it up into two parts:EnumDescris the data that describes an enum. Instances of this are owned by aComponentInterfacein a normal tree structure without backwards references.Enumis a kind of aggregate transient pointer, combining a reference to anEnumDescrwith a reference to its containingComponentInterface. Instances of this are synthesized on demand when inspecting aComponentInterface.From the consumer's perspective, it calls
ci.get_enum_definition("MyEnum")and receives anEnumstruct, and it can call methods on theEnumstruct to inspect its definition. But these methods all delegate to the underlyingEnumDescrfor the actual data.It gets a little complicated in the details of nested strcuts like
VariantandField, and I'll leave some musings inline.Thoughts?