[PATCH] D34114: [clang] Change the condition of unnecessary packed warning

2017-07-28 Thread Chih-Hung Hsieh via Phabricator via cfe-commits
chh added a comment. LGTM. rsmith, srhines, akyrtzi, rtrieu, do you have any comment? https://reviews.llvm.org/D34114 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D34114: [clang] Change the condition of unnecessary packed warning

2017-07-28 Thread Yan Wang via Phabricator via cfe-commits
yawanng updated this revision to Diff 108665. yawanng marked 3 inline comments as done. https://reviews.llvm.org/D34114 Files: lib/AST/RecordLayoutBuilder.cpp test/CodeGenCXX/warn-padded-packed.cpp Index: test/CodeGenCXX/warn-padded-packed.cpp

[PATCH] D34114: [clang] Change the condition of unnecessary packed warning

2017-07-28 Thread Chih-Hung Hsieh via Phabricator via cfe-commits
chh added inline comments. Comment at: lib/AST/RecordLayoutBuilder.cpp:1887 +// greater than the one after packing. +if (Packed && UnpackedAlignment <= Alignment) Diag(D->getLocation(), diag::warn_unnecessary_packed) chh wrote: > With this change,

[PATCH] D34114: [clang] Change the condition of unnecessary packed warning

2017-07-27 Thread Yan Wang via Phabricator via cfe-commits
yawanng updated this revision to Diff 108555. yawanng marked an inline comment as done. yawanng edited the summary of this revision. yawanng added a comment. Add more tests and restrict the conditions. https://reviews.llvm.org/D34114 Files: lib/AST/RecordLayoutBuilder.cpp test/CodeGenCXX/wa

[PATCH] D34114: [clang] Change the condition of unnecessary packed warning

2017-07-27 Thread Chih-Hung Hsieh via Phabricator via cfe-commits
chh added inline comments. Comment at: lib/AST/RecordLayoutBuilder.cpp:1887 +// greater than the one after packing. +if (Packed && UnpackedAlignment <= Alignment) Diag(D->getLocation(), diag::warn_unnecessary_packed) With this change, UnpackedSize

[PATCH] D34114: [clang] Change the condition of unnecessary packed warning

2017-07-27 Thread Yan Wang via Phabricator via cfe-commits
yawanng updated this revision to Diff 108513. yawanng marked 2 inline comments as done. yawanng edited the summary of this revision. https://reviews.llvm.org/D34114 Files: lib/AST/RecordLayoutBuilder.cpp test/CodeGenCXX/warn-padded-packed.cpp Index: test/CodeGenCXX/warn-padded-packed.cpp ===

[PATCH] D34114: [clang] Change the condition of unnecessary packed warning

2017-07-27 Thread Stephen Hines via Phabricator via cfe-commits
srhines added a comment. This looks better. It did lose context however, so some of the review is harder to read. Comment at: test/CodeGenCXX/warn-padded-packed.cpp:84 + +struct S16 { // expected-warning {{padding size of 'S16' with 2 bytes to alignment boundar}} expected-war

[PATCH] D34114: [clang] Change the condition of unnecessary packed warning

2017-07-26 Thread Yan Wang via Phabricator via cfe-commits
yawanng updated this revision to Diff 108401. yawanng added a comment. Add more tests. https://reviews.llvm.org/D34114 Files: lib/AST/RecordLayoutBuilder.cpp test/CodeGenCXX/warn-padded-packed.cpp Index: test/CodeGenCXX/warn-padded-packed.cpp ===

[PATCH] D34114: [clang] Change the condition of unnecessary packed warning

2017-07-26 Thread Yan Wang via Phabricator via cfe-commits
yawanng updated this revision to Diff 108397. https://reviews.llvm.org/D34114 Files: lib/AST/RecordLayoutBuilder.cpp test/CodeGenCXX/warn-padded-packed.cpp Index: test/CodeGenCXX/warn-padded-packed.cpp === --- test/CodeGenCXX/w

[PATCH] D34114: [clang] Change the condition of unnecessary packed warning

2017-07-26 Thread Yan Wang via Phabricator via cfe-commits
yawanng updated this revision to Diff 108395. yawanng edited the summary of this revision. yawanng added a comment. Including cases '__attribute__((packed, aligned(X)));'. When the alignment before packing is less or equal to the packed one, he 'packed' attribute not including 'aligned(X)' seems

[PATCH] D34114: [clang] Change the condition of unnecessary packed warning

2017-07-26 Thread Yan Wang via Phabricator via cfe-commits
yawanng added inline comments. Comment at: lib/AST/RecordLayoutBuilder.cpp:1888 -if (Packed && UnpackedAlignment > CharUnits::One() && -getSize() == UnpackedSize) Diag(D->getLocation(), diag::warn_unnecessary_packed) chh wrote: > Why not keepi

[PATCH] D34114: [clang] Change the condition of unnecessary packed warning

2017-07-26 Thread Chih-Hung Hsieh via Phabricator via cfe-commits
chh added inline comments. Comment at: lib/AST/RecordLayoutBuilder.cpp:1888 -if (Packed && UnpackedAlignment > CharUnits::One() && -getSize() == UnpackedSize) Diag(D->getLocation(), diag::warn_unnecessary_packed) Why not keeping the (getSize()