Skip to content
This repository has been archived by the owner on Jan 28, 2024. It is now read-only.

Support for Packed struct #205

Merged
merged 9 commits into from
Apr 16, 2021
Merged

Conversation

mannprerak2
Copy link
Contributor

@mannprerak2 mannprerak2 commented Apr 16, 2021

Closes dart-lang/native#352

  • Added support for @Packed(X) Struct annotation.
  • Added config key structs -> pack to override the pack values.
  • Updated SDK constraints to >=2.13.0-211.6.beta.
  • Updated version to 3.0.0-beta.1
  • Added tests, Updated changelog.

@mannprerak2 mannprerak2 marked this pull request as ready for review April 16, 2021 11:51
@mannprerak2 mannprerak2 requested a review from dcharkes April 16, 2021 12:27
@mannprerak2
Copy link
Contributor Author

mannprerak2 commented Apr 16, 2021

@dcharkes
From our plan dart-lang/native#352, It seems like we cannot reliably get a align value if only __attribute__((aligned(X)) is used. Since it could be inside a #pragma pack(...) which changes its behavior to be the exact opposite.

I have added some tests and it all seems to be working. I'll try to do some manual testing on Win32 as you suggested.

There's a config key struct -> pack that can override the generated value. It targets the generated name instead of the original name (in the C header). I think this makes more sense since we are overriding the value which would be easier done by the user by directly matching with the generated name, this has also been documented in the readme.

CHANGELOG.md Outdated Show resolved Hide resolved
lib/src/config_provider/config_types.dart Outdated Show resolved Hide resolved
@@ -33,6 +34,39 @@ class _ParsedStruc {
(dartHandleMember && config.useDartHandle) ||
incompleteStructMember;

// A struct without any attribute is definitely not packed. #pragma pack(...)
// also adds an attribute, but it's unexposed and cannot be travesed.
Copy link
Contributor

Choose a reason for hiding this comment

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

Please document in the readme which of the two ways clang allows to detect in which ones we don't. Then users have a starting point for manually verifying whether it did the right thing for them.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

lib/src/header_parser/sub_parsers/structdecl_parser.dart Outdated Show resolved Hide resolved
lib/src/header_parser/sub_parsers/structdecl_parser.dart Outdated Show resolved Hide resolved
lib/src/header_parser/utils.dart Outdated Show resolved Hide resolved
external int a;
}

/// Should be packed with 8.
Copy link
Contributor

Choose a reason for hiding this comment

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

?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Turns out that #pragma pack(X) sets the Upper bound value of the alignment. (While the alignment(X) attribute sets the lower bound.) I have updated the test accordingly.

tool/libclang_config.yaml Show resolved Hide resolved
@mannprerak2 mannprerak2 requested a review from dcharkes April 16, 2021 13:58
@mannprerak2
Copy link
Contributor Author

mannprerak2 commented Apr 16, 2021

I ran this on windows.h (Win32) and the Packed values seem to match those in https://github.com/timsneath/win32 🎉

@dcharkes dcharkes merged commit 12e9146 into dart-archive:master Apr 16, 2021
@dcharkes
Copy link
Contributor

Awesome! Thanks @mannprerak2 !

@mannprerak2 mannprerak2 deleted the packed_struct branch April 16, 2021 16:10
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Development

Successfully merging this pull request may close these issues.

packed structs
2 participants