Skip to content

Conversation

@hughbe
Copy link
Contributor

@hughbe hughbe commented Feb 19, 2017

The following struct has a different size with MSVC and Clang/GCC.

enum class CodeCompletionDeclKind {};

struct SwiftSemanticToken {
  unsigned ByteOffset;
  unsigned Length : 24;
  CodeCompletionDeclKind Kind : 6;
  bool IsRef : 1;
  bool IsSystem : 1;
};
static_assert(sizeof(SwiftSemanticToken) == 8, "Too big");

int main()
{
    std::cout << "Hello, world!\n";
}

Clang/GCC report 8
MSVC reports 12

So, I'm not sure if this is really the right fix! I'd like to be able to make this struct size 8, as I don't like nerfing behaviour based on the compiler.

Please suggest something if anyone has ideas.

Using #pragma pack(1) with MSVC makes the struct size 9. Still not good enough!

@hughbe
Copy link
Contributor Author

hughbe commented Feb 20, 2017

@compnerd if you have any ideas as our resident Windows compiler expert

@hughbe hughbe requested a review from nkcsgexi February 20, 2017 03:26
@jrose-apple
Copy link
Contributor

IIRC, MSVC won't combine signed and unsigned fields into the same unit, and bool is considered signed. (Not sure what CodeCompletionDeclKind counts as.) We usually make all bitfields unsigned int for this reason, and use accessors (with asserts) to convert back to the real type.

@hughbe
Copy link
Contributor Author

hughbe commented Feb 21, 2017

You're certainly correct! This works

enum class CodeCompletionDeclKind {};

struct SwiftSemanticToken {
  unsigned ByteOffset;
  unsigned Length : 24;
  CodeCompletionDeclKind Kind : 6;
  unsigned IsRef : 1;
  unsigned IsSystem : 1;
};
static_assert(sizeof(SwiftSemanticToken) == 8, "Too big");

@hughbe
Copy link
Contributor Author

hughbe commented Feb 21, 2017

@swift-ci please smoke test

@hughbe
Copy link
Contributor Author

hughbe commented Feb 21, 2017

That's working - let me know if you have any other comments @jrose-apple

@jrose-apple
Copy link
Contributor

Deferring back to @nkcsgexi.

@nkcsgexi
Copy link
Contributor

looks good to me.

@hughbe hughbe merged commit f66bbfb into swiftlang:master Feb 22, 2017
@hughbe hughbe deleted the msvc-static-assertion-failure branch February 22, 2017 02:35
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