-
Notifications
You must be signed in to change notification settings - Fork 0
feat: array memory mapping #20
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
feat: array memory mapping #20
Conversation
…factor/memory-mapping
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This already looks so much cleaner than the previous implementation. Great job!
pub struct Foo { | ||
a: i32, | ||
b: [Bar], | ||
c: [Baz], |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Can this code be removed?
@@ -637,6 +637,1423 @@ fn map_struct_all() { | |||
); | |||
} | |||
|
|||
#[test] |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
All of these tests, test if the value of a certain instance has not changed. However, this is done through get
which also performs casting if possible. I think besides testing whether or not the content changes, it should also test whether or not the type actually changes.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The tests also feel very verbose to me. Although its a good thing because they thoroughly test the code it also makes them much harder to maintain if we change details down the line. Im wondering if its possible to reduce the number of lines required to setup a test like this... Its just a thought.
…-lang#447) Updates the requirements on ra_ap_text_edit to permit the latest version. --- updated-dependencies: - dependency-name: ra_ap_text_edit dependency-type: direct:production ... Signed-off-by: dependabot[bot] <support@github.com> Signed-off-by: dependabot[bot] <support@github.com> Co-authored-by: dependabot[bot] <49699333+dependabot[bot]@users.noreply.github.com>
…-lang#449) Updates the requirements on ra_ap_text_edit to permit the latest version. --- updated-dependencies: - dependency-name: ra_ap_text_edit dependency-type: direct:production ... Signed-off-by: dependabot[bot] <support@github.com> Signed-off-by: dependabot[bot] <support@github.com> Co-authored-by: dependabot[bot] <49699333+dependabot[bot]@users.noreply.github.com>
Updates the requirements on [criterion](https://github.com/bheisler/criterion.rs) to permit the latest version. - [Release notes](https://github.com/bheisler/criterion.rs/releases) - [Changelog](https://github.com/bheisler/criterion.rs/blob/master/CHANGELOG.md) - [Commits](bheisler/criterion.rs@0.3.0...0.4.0) --- updated-dependencies: - dependency-name: criterion dependency-type: direct:production ... Signed-off-by: dependabot[bot] <support@github.com> Signed-off-by: dependabot[bot] <support@github.com> Co-authored-by: dependabot[bot] <49699333+dependabot[bot]@users.noreply.github.com>
Updates the requirements on [lockfile](https://github.com/derekdreery/lockfile-rs) to permit the latest version. - [Release notes](https://github.com/derekdreery/lockfile-rs/releases) - [Changelog](https://github.com/derekdreery/lockfile-rs/blob/master/CHANGELOG.md) - [Commits](https://github.com/derekdreery/lockfile-rs/commits) --- updated-dependencies: - dependency-name: lockfile dependency-type: direct:production ... Signed-off-by: dependabot[bot] <support@github.com> Signed-off-by: dependabot[bot] <support@github.com> Co-authored-by: dependabot[bot] <49699333+dependabot[bot]@users.noreply.github.com>
…-lang#451) Updates the requirements on ra_ap_text_edit to permit the latest version. --- updated-dependencies: - dependency-name: ra_ap_text_edit dependency-type: direct:production ... Signed-off-by: dependabot[bot] <support@github.com> Signed-off-by: dependabot[bot] <support@github.com> Co-authored-by: dependabot[bot] <49699333+dependabot[bot]@users.noreply.github.com>
…-lang#460) Updates the requirements on ra_ap_text_edit to permit the latest version. --- updated-dependencies: - dependency-name: ra_ap_text_edit dependency-type: direct:production ... Signed-off-by: dependabot[bot] <support@github.com> Signed-off-by: dependabot[bot] <support@github.com> Co-authored-by: dependabot[bot] <49699333+dependabot[bot]@users.noreply.github.com>
…-lang#473) Updates the requirements on ra_ap_text_edit to permit the latest version. --- updated-dependencies: - dependency-name: ra_ap_text_edit dependency-type: direct:production ... Signed-off-by: dependabot[bot] <support@github.com> Signed-off-by: dependabot[bot] <support@github.com> Co-authored-by: dependabot[bot] <49699333+dependabot[bot]@users.noreply.github.com>
used_deletions | ||
.into_iter() | ||
.zip(deletions.into_iter()) | ||
.for_each(|(used, (_, old_index, ty, _))| { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Same here, I would write this as a regular for
statement.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I actually like these as you're already explicitly working with zipped iterators. I changed the other one
used_insertions | ||
.into_iter() | ||
.zip(insertions.into_iter()) | ||
.for_each(|(used, (_, new_index, ty, _))| { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Same
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Also didn't resolve for same reason as above comment
Creates a clearer separation between the calculation of a memory mapping and the execution thereof. Also, adds support for array memory mapping.
Supported array mappings (recursively) are:
[f64] <-> f64 (array from/to primitive + valid casts)
[f32] <-> [[f64]] (array from/to primitive + valid casts)
[Foo] <-> Foo (array from/to struct + struct mapping)