[PATCH] D46218: PR37275 packed attribute should not apply to base classes

2018-04-30 Thread Richard Smith - zygoloid via Phabricator via cfe-commits
rsmith added a comment. In https://reviews.llvm.org/D46218#1082834, @probinson wrote: > In https://reviews.llvm.org/D46218#1081933, @rjmccall wrote: > > > I wonder if that was originally just an oversight that got turned into > > official policy. Anyway, if it's the policy, it's what we have to

[PATCH] D46218: PR37275 packed attribute should not apply to base classes

2018-04-30 Thread Paul Robinson via Phabricator via cfe-commits
probinson added a comment. In https://reviews.llvm.org/D46218#1081933, @rjmccall wrote: > I wonder if that was originally just an oversight that got turned into > official policy. Anyway, if it's the policy, it's what we have to do; LGTM. I think it's actually correct behavior. Why would an

[PATCH] D46218: PR37275 packed attribute should not apply to base classes

2018-04-28 Thread Richard Smith - zygoloid via Phabricator via cfe-commits
rsmith added a comment. In https://reviews.llvm.org/D46218#1081933, @rjmccall wrote: > Have you checked whether we do the right thing with `#pragma pack` on MSVC? Great question, I hadn't. Both MSVC and GCC treat `#pragma pack` as applying to base classes, which is what we do with/without this

[PATCH] D46218: PR37275 packed attribute should not apply to base classes

2018-04-28 Thread Richard Smith - zygoloid via Phabricator via cfe-commits
This revision was automatically updated to reflect the committed changes. Closed by commit rL331136: PR37275 packed attribute should not apply to base classes (authored by rsmith, committed by ). Herald added a subscriber: llvm-commits. Changed prior to commit: https://reviews.llvm.org/D46218?v

[PATCH] D46218: PR37275 packed attribute should not apply to base classes

2018-04-27 Thread John McCall via Phabricator via cfe-commits
rjmccall accepted this revision. rjmccall added a comment. This revision is now accepted and ready to land. I wonder if that was originally just an oversight that got turned into official policy. Anyway, if it's the policy, it's what we have to do; LGTM. Have you checked whether we do the right

[PATCH] D46218: PR37275 packed attribute should not apply to base classes

2018-04-27 Thread Richard Smith - zygoloid via Phabricator via cfe-commits
rsmith updated this revision to Diff 144415. rsmith added a comment. Added release notes. Repository: rC Clang https://reviews.llvm.org/D46218 Files: docs/ReleaseNotes.rst lib/AST/RecordLayoutBuilder.cpp test/CodeGenCXX/alignment.cpp test/SemaCXX/class-layout.cpp Index: test/SemaCXX

[PATCH] D46218: PR37275 packed attribute should not apply to base classes

2018-04-27 Thread Richard Smith - zygoloid via Phabricator via cfe-commits
rsmith created this revision. rsmith added a reviewer: rjmccall. Clang incorrectly applies the packed attribute to base classes. Per GCC's documentation and as can be observed from its behavior, packed only applies to members, not base classes. This change is conditioned behind `-fclang-abi-com