Allow custom field offsets to be passed for define-cstruct

[BartAdv]

This should be useful when dealing with structures whose layout is not standardized across implementation. It simply allows to pass a list of integers which is then used as offsets instead of the ones computed from field types and alignment.

[mflatt]

Thanks for the patch.

Would it be more convenient or less convenient if offsets were attached to individual fields, instead of having a list for all fields?

[BartAdv]

Indeed it would be more readable. I'll look into it.

On 2/11/15, Matthew Flatt notifications@github.com wrote:

Thanks for the patch.

Would it be more convenient or less convenient if offsets were attached to
individual fields, instead of having a list for all fields?

---

Reply to this email directly or view it on GitHub:
#876 (comment)

[samth]

@BartAdv also, if you look at the status checks on GitHub, you'll see that there's a build error with this pull request.

[BartAdv]

Yes, but it doesn't matter, it's gonna be reworked.

On Wed, Feb 11, 2015 at 2:42 PM, Sam Tobin-Hochstadt <
notifications@github.com> wrote:

@BartAdv https://github.com/BartAdv also, if you look at the status
checks on GitHub, you'll see that there's a build error with this pull
request.

—
Reply to this email directly or view it on GitHub
#876 (comment).

[BartAdv]

I have modified it to include offsets along with field specification, but don't treat it like finished patch - I must say I struggled a bit with this (basically, I needed to learn macros basics), and I don't like the way I'm extracting optional values. I'd very much appreciate just some review.

Thanks

[samth]

This is still checking the same thing it was before.

[BartAdv]

OK, found out where's the error, I'm on it

[BartAdv]

OK, looks like the tests are passing now - I wonder, what are the edge cases to consider, the previous error was caused by not checking the case with super-id...

[BartAdv]

Added syntax test case and rebased + squashed the commits.

I'm unsure if I'm syntax-case'ing idiomatically, previously the [slot slot-type] was at the top level syntax case, I've merged those into single case and I'm pattern matching deeper, after parsing additional options. I've put some comments in the diff @6beb5df.

[mflatt]

I'd like to add an #:offset keyword before the offset, in anticipation of handling future options uniformly. I'll make that change, generally take a closer look at the implementation, update the docs, and then merge.

Thanks!

[BartAdv]

One more thing occured to me - if we allow custom offsets, shouldn't we allow specifying custom size?

The scenario for which I needed this change was just about shuffling the order of the fields, so the size stays the same. However, it's easy to imagine one could do more crazy things with it.

[mflatt]

I think that custom sizes are handled well enough through the field's type. It would also be possible to set offsets by playing games with field types, but it would be tricky, and that's not how ctypes are intended to work. Specifying a representation size, through, is properly a job of a ctype.

[mflatt]

Merged - thanks!