r309750 - [clang] Change the condition of unnecessary packed warning
Yan Wang via cfe-commits
cfe-commits at lists.llvm.org
Tue Aug 1 14:41:39 PDT 2017
Author: yawanng
Date: Tue Aug 1 14:41:39 2017
New Revision: 309750
URL: http://llvm.org/viewvc/llvm-project?rev=309750&view=rev
Log:
[clang] Change the condition of unnecessary packed warning
Summary:
Change the condition of this unnecessary packed warning. The packed is unnecessary when
1. the alignment of the struct/class won't alter.
2. the size is unchanged.
3. the offset of each field is the same.
Remove all field-level warning.
Reviewers: chh, akyrtzi, rtrieu
Reviewed By: chh
Subscribers: rsmith, srhines, cfe-commits, xazax.hun
Differential Revision: https://reviews.llvm.org/D34114
Modified:
cfe/trunk/lib/AST/RecordLayoutBuilder.cpp
cfe/trunk/test/CodeGenCXX/warn-padded-packed.cpp
Modified: cfe/trunk/lib/AST/RecordLayoutBuilder.cpp
URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/AST/RecordLayoutBuilder.cpp?rev=309750&r1=309749&r2=309750&view=diff
==============================================================================
--- cfe/trunk/lib/AST/RecordLayoutBuilder.cpp (original)
+++ cfe/trunk/lib/AST/RecordLayoutBuilder.cpp Tue Aug 1 14:41:39 2017
@@ -632,6 +632,9 @@ protected:
/// pointer, as opposed to inheriting one from a primary base class.
bool HasOwnVFPtr;
+ /// \brief the flag of field offset changing due to packed attribute.
+ bool HasPackedField;
+
typedef llvm::DenseMap<const CXXRecordDecl *, CharUnits> BaseOffsetsMapTy;
/// Bases - base classes and their offsets in the record.
@@ -666,7 +669,7 @@ protected:
NonVirtualSize(CharUnits::Zero()),
NonVirtualAlignment(CharUnits::One()), PrimaryBase(nullptr),
PrimaryBaseIsVirtual(false), HasOwnVFPtr(false),
- FirstNearlyEmptyVBase(nullptr) {}
+ HasPackedField(false), FirstNearlyEmptyVBase(nullptr) {}
void Layout(const RecordDecl *D);
void Layout(const CXXRecordDecl *D);
@@ -1847,7 +1850,6 @@ void ItaniumRecordLayoutBuilder::FinishL
uint64_t UnpaddedSize = getSizeInBits() - UnfilledBitsInLastUnit;
uint64_t UnpackedSizeInBits =
llvm::alignTo(getSizeInBits(), Context.toBits(UnpackedAlignment));
- CharUnits UnpackedSize = Context.toCharUnitsFromBits(UnpackedSizeInBits);
uint64_t RoundedSize =
llvm::alignTo(getSizeInBits(), Context.toBits(Alignment));
@@ -1882,10 +1884,11 @@ void ItaniumRecordLayoutBuilder::FinishL
<< (InBits ? 1 : 0); // (byte|bit)
}
- // Warn if we packed it unnecessarily. If the alignment is 1 byte don't
- // bother since there won't be alignment issues.
- if (Packed && UnpackedAlignment > CharUnits::One() &&
- getSize() == UnpackedSize)
+ // Warn if we packed it unnecessarily, when the unpacked alignment is not
+ // greater than the one after packing, the size in bits doesn't change and
+ // the offset of each field is identical.
+ if (Packed && UnpackedAlignment <= Alignment &&
+ UnpackedSizeInBits == getSizeInBits() && !HasPackedField)
Diag(D->getLocation(), diag::warn_unnecessary_packed)
<< Context.getTypeDeclType(RD);
}
@@ -1977,13 +1980,10 @@ void ItaniumRecordLayoutBuilder::CheckFi
<< Context.getTypeDeclType(D->getParent())
<< PadSize
<< (InBits ? 1 : 0); // (byte|bit)
- }
-
- // Warn if we packed it unnecessarily. If the alignment is 1 byte don't
- // bother since there won't be alignment issues.
- if (isPacked && UnpackedAlign > CharBitNum && Offset == UnpackedOffset)
- Diag(D->getLocation(), diag::warn_unnecessary_packed)
- << D->getIdentifier();
+ }
+ if (isPacked && Offset != UnpackedOffset) {
+ HasPackedField = true;
+ }
}
static const CXXMethodDecl *computeKeyFunction(ASTContext &Context,
Modified: cfe/trunk/test/CodeGenCXX/warn-padded-packed.cpp
URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/test/CodeGenCXX/warn-padded-packed.cpp?rev=309750&r1=309749&r2=309750&view=diff
==============================================================================
--- cfe/trunk/test/CodeGenCXX/warn-padded-packed.cpp (original)
+++ cfe/trunk/test/CodeGenCXX/warn-padded-packed.cpp Tue Aug 1 14:41:39 2017
@@ -17,7 +17,7 @@ struct S3 {
} __attribute__((packed));
struct S4 {
- int i; // expected-warning {{packed attribute is unnecessary for 'i'}}
+ int i;
char c;
} __attribute__((packed));
@@ -46,18 +46,18 @@ struct S8 : B {
int i; // expected-warning {{padding struct 'S8' with 3 bytes to align 'i'}}
};
-struct S9 { // expected-warning {{packed attribute is unnecessary for 'S9'}}
- int x; // expected-warning {{packed attribute is unnecessary for 'x'}}
- int y; // expected-warning {{packed attribute is unnecessary for 'y'}}
+struct S9 {
+ int x;
+ int y;
} __attribute__((packed));
-struct S10 { // expected-warning {{packed attribute is unnecessary for 'S10'}}
- int x; // expected-warning {{packed attribute is unnecessary for 'x'}}
+struct S10 {
+ int x;
char a,b,c,d;
} __attribute__((packed));
-struct S11 {
+struct S11 { // expected-warning {{packed attribute is unnecessary for 'S11'}}
bool x;
char a,b,c,d;
} __attribute__((packed));
@@ -72,5 +72,82 @@ struct S13 { // expected-warning {{paddi
bool b : 10;
};
+struct S14 { // expected-warning {{packed attribute is unnecessary for 'S14'}}
+ char a,b,c,d;
+} __attribute__((packed));
+
+struct S15 { // expected-warning {{packed attribute is unnecessary for 'S15'}}
+ struct S14 s;
+ char a;
+} __attribute__((packed));
+
+struct S16 { // expected-warning {{padding size of 'S16' with 2 bytes to alignment boundary}} expected-warning {{packed attribute is unnecessary for 'S16'}}
+ char a,b;
+} __attribute__((packed, aligned(4)));
+
+struct S17 {
+ struct S16 s;
+ char a,b;
+} __attribute__((packed, aligned(2)));
+
+struct S18 { // expected-warning {{padding size of 'S18' with 2 bytes to alignment boundary}} expected-warning {{packed attribute is unnecessary for 'S18'}}
+ struct S16 s;
+ char a,b;
+} __attribute__((packed, aligned(4)));
+
+struct S19 { // expected-warning {{packed attribute is unnecessary for 'S19'}}
+ bool b;
+ char a;
+} __attribute__((packed, aligned(1)));
+
+struct S20 {
+ int i;
+ char a;
+} __attribute__((packed, aligned(1)));
+
+struct S21 { // expected-warning {{padding size of 'S21' with 4 bits to alignment boundary}}
+ unsigned char a : 6;
+ unsigned char b : 6;
+} __attribute__((packed, aligned(1)));
+
+struct S22 { // expected-warning {{packed attribute is unnecessary for 'S22'}}
+ unsigned char a : 4;
+ unsigned char b : 4;
+} __attribute__((packed));
+
+struct S23 { // expected-warning {{padding size of 'S23' with 4 bits to alignment boundary}} expected-warning {{packed attribute is unnecessary for 'S23'}}
+ unsigned char a : 2;
+ unsigned char b : 2;
+} __attribute__((packed));
+
+struct S24 {
+ unsigned char a : 6;
+ unsigned char b : 6;
+ unsigned char c : 6;
+ unsigned char d : 6;
+ unsigned char e : 6;
+ unsigned char f : 6;
+ unsigned char g : 6;
+ unsigned char h : 6;
+} __attribute__((packed));
+
+struct S25 { // expected-warning {{padding size of 'S25' with 7 bits to alignment boundary}} expected-warning {{packed attribute is unnecessary for 'S25'}}
+ unsigned char a;
+ unsigned char b : 1;
+} __attribute__((packed));
+
+struct S26 { // expected-warning {{packed attribute is unnecessary for 'S26'}}
+ unsigned char a : 1;
+ unsigned char b; //expected-warning {{padding struct 'S26' with 7 bits to align 'b'}}
+} __attribute__((packed));
+
+struct S27 { // expected-warning {{padding size of 'S27' with 7 bits to alignment boundary}}
+ unsigned char a : 1;
+ unsigned char b : 8;
+} __attribute__((packed));
+
+
// The warnings are emitted when the layout of the structs is computed, so we have to use them.
-void f(S1*, S2*, S3*, S4*, S5*, S6*, S7*, S8*, S9*, S10*, S11*, S12*, S13*) { }
+void f(S1*, S2*, S3*, S4*, S5*, S6*, S7*, S8*, S9*, S10*, S11*, S12*, S13*,
+ S14*, S15*, S16*, S17*, S18*, S19*, S20*, S21*, S22*, S23*, S24*, S25*,
+ S26*, S27*){}
More information about the cfe-commits
mailing list