Implement basic abstract structural implementation #55
Conversation
johanpel
added
enhancement
Codecov Report
@@ Coverage Diff @@
## master #55 +/- ##
==========================================
+ Coverage 78.41% 80.95% +2.54%
==========================================
Files 15 24 +9
Lines 2001 2626 +625
==========================================
+ Hits 1569 2126 +557
- Misses 432 500 +68
Continue to review full report at Codecov.
|
Implement a simple dot back-end to visualize structural implementations
|
|
||
| /// Trait to construct node interface references from various node types. | ||
| pub trait Interfaces { | ||
| fn io<K>(&self, key: K) -> Result<NodeIORef> |
There was a problem hiding this comment.
I'm not a fan of the pattern where connect takes these Results. I understand from the example that this results in a friendly API. However, I prefer to keep handling of Resulttypes up to users i.e. modify the signature of connect to take NodeIORef instead of Result<NodeIORef>. Or allow users to pass impl TryInto<InterfaceKey> into the connect function instead.
|
|
||
| /// The StructuralImplBuilder struct allows users to implement streamlets by structurally | ||
| /// combining streamlets into larger designs. | ||
| #[derive(Clone, PartialEq)] |
There was a problem hiding this comment.
It's good practice to have all pub items implement Debug.
| /// This function returns an Error if the streamlet reference is invalid w.r.t. the project. | ||
| pub fn try_new(project: &'prj Project, streamlet: StreamletRef) -> Result<Self> { | ||
| // Check if the reference is OK. | ||
| project.get_streamlet(streamlet.clone())?; |
There was a problem hiding this comment.
It's more ergonomic when the get_streamlet function clones when it needs to. However the current implementation takes a reference of that key to get the value so the argument of get_streamlet can be &StreamletRef.
| /// let mut builder = StructuralImplBuilder::try_new(&prj, foo)?; | ||
| /// | ||
| /// let beans = builder.this().io("beans"); // Obtain a (valid) reference to the beans interface. | ||
| /// let coffee = builder.this().io("covfefe"); // Oops, made a typo! The reference is invalid. |
There was a problem hiding this comment.
In addition to the comment above, here I would expect users to check the result.
let coffee = builder.this().io("covfefe")?;| } | ||
|
|
||
| /// Return the node representing the external interfaces of the streamlet itself. | ||
| pub fn this(&self) -> Node { |
There was a problem hiding this comment.
What is the reason to return an owned instance of the node instead of a reference here?
|
|
||
| /// Return an iterator over all nodes in the structural graph. | ||
| pub fn nodes(&self) -> impl Iterator<Item = &Node> { | ||
| self.nodes.iter().map(|(_, v)| v) |
There was a problem hiding this comment.
There is a values method on IndexMap.
| pub fn libraries(&self) -> impl Iterator<Item = &Library> { | ||
| self.libraries.iter() | ||
| self.libraries.iter().map(|(_, l)| l) |
There was a problem hiding this comment.
You can use the IndexMap Values iterator.
| pub fn set_implementation(&self, implementation: Implementation) -> Result<()> { | ||
| if let Some(r) = implementation.streamlet() { | ||
| if r.streamlet == self.key { | ||
| *self.implementation.borrow_mut().deref_mut() = implementation; |
There was a problem hiding this comment.
You may want to use try_borrow_mut here and return an error instead of a panic.
| @@ -1,5 +1,6 @@ | |||
| //! Error variants. | |||
| use log::SetLoggerError; | |||
| use nom::lib::std::convert::Infallible; | |||
There was a problem hiding this comment.
This looks like your editor is trying to be smart.
src/parser/nom.rs
Outdated
| // UniqueKeyBuilder::new() | ||
| // .with_item( | ||
| // Interface::try_new( | ||
| // "a", | ||
| // Mode::In, | ||
| // Rc::new( | ||
| // TydiType::try_new( | ||
| // "TODO", | ||
| // LogicalType::try_new_group(vec![("a", 1), ("b", 2)]) | ||
| // .unwrap() | ||
| // ) | ||
| // .unwrap() | ||
| // ), | ||
| // None | ||
| // ) | ||
| // .unwrap() | ||
| // ) | ||
| // .with_item( | ||
| // Interface::try_new( | ||
| // "c", | ||
| // Mode::Out, | ||
| // Rc::new(TydiType::try_new("TODO", LogicalType::Null).unwrap()), | ||
| // None | ||
| // ) | ||
| // .unwrap() | ||
| // ), | ||
| // None | ||
| // ) | ||
| // .unwrap() | ||
| // )) | ||
| // ); | ||
| // } | ||
|
|
||
| // #[test] | ||
| // fn parse_streamlet_docstring() { | ||
| // assert_eq!( | ||
| // streamlet( | ||
| // "/// Test | ||
| // // some other stuff | ||
| // /* that people could put here */ | ||
| // /* even though */ // it's not pretty | ||
| // /// unaligned doc string | ||
| // | ||
| // Streamlet x ( | ||
| // /// Such a weird interface | ||
| // a : in Null, | ||
| // /// And another one | ||
| // b : out Null )" | ||
| // ), | ||
| // Ok(( | ||
| // "", | ||
| // Streamlet::from_builder( | ||
| // Name::try_new("x").unwrap(), | ||
| // UniqueKeyBuilder::new().with_items(vec![ | ||
| // Interface::try_new( | ||
| // "a", | ||
| // Mode::In, | ||
| // Rc::new(TydiType::try_new("TODO", LogicalType::Null).unwrap()), | ||
| // Some(" Such a weird interface") | ||
| // ) | ||
| // .unwrap(), | ||
| // Interface::try_new( | ||
| // "b", | ||
| // Mode::Out, | ||
| // Rc::new(TydiType::try_new("TODO", LogicalType::Null).unwrap()), | ||
| // Some(" And another one") | ||
| // ) | ||
| // .unwrap(), | ||
| // ]), | ||
| // Some(" Test\n unaligned doc string"), | ||
| // ) | ||
| // .unwrap() | ||
| // )) | ||
| // ); | ||
| // } | ||
|
|
||
| // #[test] | ||
| // fn parse_list_of_streamlets() { | ||
| // assert_eq!( | ||
| // list_of_streamlets(concat!( | ||
| // "Streamlet a ( a: in Null, b: out Null)\n", | ||
| // "/* A comment */\n", | ||
| // "Streamlet b ( a: in Null, b: out Null)\n", | ||
| // "/// Multi-line...\n", | ||
| // "/// doc string...\n", | ||
| // "Streamlet c ( a: in Null, b: out Null)", | ||
| // )), | ||
| // Ok(( | ||
| // "", | ||
| // UniqueKeyBuilder::new() | ||
| // .with_items(vec![ | ||
| // streamlets::nulls_streamlet("a"), | ||
| // streamlets::nulls_streamlet("b"), | ||
| // streamlets::nulls_streamlet("c").with_doc(" Multi-line...\n doc string..."), | ||
| // ]) | ||
| // .finish() | ||
| // .unwrap() | ||
| // )) | ||
| // ); | ||
| // } |
There was a problem hiding this comment.
Why are these tests disabled?
|
This is stale. |
This PR will allow users to create structural implementations of streamlets, by combining primitive streamlets into larger designs, referencing them from various libraries within a project.
This should be geared towards two use cases:
To-do:
Non-goals for this PR: