Conversation
| not_packed, | ||
| i8, | ||
| i16, | ||
| WaitQueue, |
There was a problem hiding this comment.
This looks inconsistent with the existing values, in particular not_packed vs WaitQueue?
Which proposal is this from btw? I'd like to read the background.
There was a problem hiding this comment.
The context is in the linked issue and in particular in Thomas's issue. I'm implementing the second paragraph in that issue which is WaitQueue as a packed type. Since I'm implementing it as a packed type, I think this is the right place for this but let me know if I got it wrong.
The problem that we're trying to solve is that WaitQueue needs to be partially opaque, but we'd also like existing struct accessors to be able to mutate its control word.
There was a problem hiding this comment.
Just realized that you meant the naming. I don't mind changing it, but it looks like most enums values in this repo use PascalCase, maybe we should change not_packed to NotPacked instead? Either way is ok to me.
There was a problem hiding this comment.
Yeah, you're right, maybe it should be NotPacked. I guess we can leave that as separate from this PR then.
Reading some of the discussion on the substance, this seems... complicated.
There was a problem hiding this comment.
Let's make a decision though on planning to rename to NotPacked, or something else. I think I'd be in favor of that.
There was a problem hiding this comment.
Sounds good to me! Will change it in the next PR.
There was a problem hiding this comment.
sg, though actually let's wait for @tlively to weigh in, as maybe there was a reason for not_packed that I don't recall...
kripken
left a comment
There was a problem hiding this comment.
Seems the spec discussion is ongoing but lgtm to land this if it helps investigation for the spec.
|
Oh, I would fuzz this before landing. I wouldn't be surprised if the fuzzer can do something surprising with the new testcase here - may need to disallow it. |
Part of #8315. Adds initial binary + text parsing and printing for the packed waitqueue type.