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

Fix struct field offsets #275

Merged
merged 3 commits into from
Nov 13, 2024
Merged

Fix struct field offsets #275

merged 3 commits into from
Nov 13, 2024

Conversation

TravisCardwell
Copy link
Collaborator

libclang reports offsets in bits, while Storable methods use offsets in bytes. We do not support bit fields yet, so this PR adds an assertion that all offsets are byte-aligned, converts from bits to bytes where necessary, and documents the unit.

In addition, documentation is added to clang_Cursor_getOffsetOfField, functions clang_Cursor_isBitField and clang_getFieldDeclBitWidth are added, and clang-ast-dump is updated to dump more information.

* Add `enum` integer type trace
* Add field offset trace
* Add field bit width trace
* Add `enum` constant integer value trace
* Add `typedef` name trace
* Reorganize `foldDecls` so that information based on the cursor kind
  can be output before comments
`libclang` reports offsets in *bits*, while `Storable` methods use
offsets in *bytes*.  We do not support bit fields yet, so this commit
adds an assertion that all offsets are byte-aligned, converts from bits
to bytes where necessary, and documents the unit.
@@ -124,6 +127,8 @@ mkStructField unit current = do
fieldOffset <- fromIntegral <$> clang_Cursor_getOffsetOfField current
fieldName <- CName <$> clang_getCursorDisplayName current

unless (fieldOffset `mod` 8 == 0) $ error "bit-fields not supported yet"
Copy link
Collaborator

Choose a reason for hiding this comment

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

fail. Don't use error in IO.

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

@edsko requested that I use error for such assertions, only using fail for things that may be handled. (I put similar assertions in the documentation reification, also running in IO.)

@edsko
Copy link
Collaborator

edsko commented Nov 13, 2024

Thanks for doing this. I'll merge this as-is, but I still think #22 (comment) is worth considering, a check such as "is `n divisible by 8" is too easy to forget.

@edsko edsko merged commit fa2b3ae into main Nov 13, 2024
8 checks passed
@edsko edsko deleted the struct-field-offsets branch November 13, 2024 07:18
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.

3 participants