Simplify lifetimes #19
Open
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Hi,
looking through this crate, I noticed that there were lots of lifetime parameters, some of which aren't useful IMO. In this MR, I'm proposing a refactoring to simplify the lifetimes while also making the crate more flexible:
There are several types in this crate that contain a
DevTree
, which is a wrapper around&[u8]
. These types currently contain a reference to the devtree, for examplefdt: &'a DevTree<'dt>
. This makes it a double reference (&'a &'dt [u8]
basically) and the outer reference doesn't provide significant value. I've therefore replaced occurrences of&'a DevTree<'dt>
in several types withDevTree<'dt>
(changing the attribute from being a reference to being a value). Here's a diff of one of the types I've changed:DevTree
has the memory layout of a slice, which is a fat pointer (pointer + length). Storing it by value instead of by reference makes the type containing it slightly larger, but also removes one level of indirection (which is good for cache locality).The other (more important) effect of this change is that it removes a lifetime parameter from lots of definitions. It also makes the APIs more flexible:
Currently, one needs to create an instance of
DevTree
which must outlive all iterators created from it. Something like this currently doesn't compile:With the proposed refactoring, this works fine:
DevTree
isn't required to outliveDevTreeIter
anymore, which is more flexible. The remaining'dt
lifetime makes sure that the data outlivesDevTreeIter
. Putting the data into the inner scope in the exmple above like this won't compile:Storing
DevTree
by value meant that I had to clone it in a couple of places. That's not an issue though IMO as it simply copies the fat pointer (16 bytes on a 64 bit system).Let me know if you have questions regarding this MR.
Looking forward to hearing from you!