Implement skip handling for fields of fixed size
#1646
Merged
Conversation
This file contains 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
bbannier
commented
Jan 10, 2024
bbannier
force-pushed
the
topic/bbannier/issue-1640
branch
from
1feb1e1 to
8731132
Compare
rsmmr
reviewed
Jan 16, 2024
bbannier
commented
Jan 16, 2024
bbannier
force-pushed
the
topic/bbannier/issue-1640
branch
from
8731132 to
1b175d1
Compare
rsmmr
reviewed
Jan 17, 2024
| @@ -30,3 +33,30 @@ std::optional<std::pair<const Expression, std::optional<const Type>>> spicy::typ | |||
|
|
|||
| return {}; | |||
| } | |||
|
|
|||
| std::optional<const Expression> spicy::type::unit::item::Field::size() const { | |||
There was a problem hiding this comment.
Couple things:
Good point that the size can depend not just on types, but also on field attributes, I didn't think about that. Given that, I'm now wondering if the type-side size() method is really as helpful as I originally thought because we now have two levels of size(), each with just a couple of cases/implementations (Type::size() and (Field::size). So maybe putting it all here into Field::size() is the better approach.
And then I think I'd use a visitor instead of the if/else-chain, both for readability/consistency-in-style and (I believe ...) performance.
There was a problem hiding this comment.
I changed this as suggested. PTAL.
rsmmr
previously approved these changes
Jan 17, 2024
This patch adds `skip` support for fields with `&size` attribute or of builtin type with known size. If a unit has a known size and it is specified in a `&size` attribute this also allows to skip over unit fields. Closes #1640.
bbannier
force-pushed
the
topic/bbannier/issue-1640
branch
from
5c96c06 to
ed937e4
Compare
|
I backported 2559ed4) to |
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
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 #1640.