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
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
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
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
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
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
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