Rewrite and Fix Remaining Padding Bugs#35
Merged
Conversation
LPGhatguy
changed the title
Owner
Author
|
Success! I found three bugs in Naga doing this:
|
Owner
Author
|
Looking good, time to merge and move onto the next round of work. |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
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.
Closes #29.
It's been awhile since I've dug into this problem. In this PR, I rewrote Crevice almost completely to fix all the remaining layout bugs and tighten up the test suite to be much more effective.
I think that some of our snapshot tests ended up incorrect for whatever reason, so I've replaced them with a macro that generates
offset_of!assertions. I think this will be much more clear and encourage test authors to read through the spec and reason about results instead of committing snapshots as-is.Generated structs now have padding after each field instead of before. This lets us pad at the end of structs consistently. As part of that refactor, I improved the clarity of the derive macro code dramatically. It should be a lot easier to follow.
Also done during this PR is tightening of names with the crate's traits. The associated type on
AsStd{140,430}is now calledOutput. I may rename these traits toIntoStd{140,430}to make them less semantically awkward, and then propagate that through all the method names.All in all, this implementation has little in common with the old one and hopefully has fewer bugs.