Skip to content
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

Struct tuple representation codegen #63

Merged
merged 4 commits into from
Jul 30, 2020
Merged

Conversation

warpfork
Copy link
Collaborator

@warpfork warpfork commented Jul 30, 2020

Codegen now supports structs with a tuple representation strategy -- that is, a representation that's a list, where the position alone corresponds to each field in the struct, and thus to the expected type and the meaning of the value.

This is a pretty exciting feature to have, because it's often used in protocols where it's desirable to minimize the encoded size (trading down on legibility (and information that could be used for version/feature-detection) to do so -- but that's often a valid trade to make!). (In particular, it's quite heavily used in the Filecoin project; support for this suddenly gets us the ability to model the vast majority of their protocol with our codegen.)

@warpfork warpfork merged commit d70a19d into master Jul 30, 2020
@warpfork warpfork deleted the struct-tuple-repr-codegen branch July 30, 2020 15:15
}

func (g structReprTupleReprGenerator) EmitNodeMethodLength(w io.Writer) {
// This is fun: it has to count down for any unset optional fields.
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

don't we have special rules for optional fields in tuple representation? like, only the last one can be optional for it to make much sense?

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yep. There's some comments about that at the top of the file. (There can be several optionals... as long as they're all consecutive, and the matching is greedy.)

I think I was lazy in this snippet: some of the code is very similar to code used in optional-value-presense-counting in structs representations as maps, and I just didn't bother to rewrite it to use the assumptions we're allowed here.

(Optimistically, I was thinking we might be able to make them share code entirely, but I experienced "mental OOM" trying to get there, so this is the non-DRY'd shrapnel. There will be a lot of stuff like this that deserves revisit in the near future: things will either turn out to be very "obvious" candidates for DRYing, or, very much deserving of further specialization -- but I'm frequently giving up trying to tell the difference as I chase feature-completeness right now. Mostly, I'm trying to at least mark them in comments, but I'm sure I often fail.)

@aschmahmann aschmahmann mentioned this pull request Feb 18, 2021
73 tasks
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants