Add warning when assigning enums to bitfields without an explicit unsigned underlying type

Summary:
Add a warning when assigning enums to bitfields without an explicit
unsigned underlying type. This is to prevent problems with MSVC
compatibility, since the Microsoft ABI defaults to storing enums with a
signed type, causing inconsistencies with saving to/reading from
bitfields.

Also disabled the warning in the dr0xx.cpp test which throws the error,
and added a test for the warning.

The warning can be disabled with -Wno-signed-enum-bitfield.

Patch by Sasha Bermeister!

Reviewers: rnk, aaron.ballman

Subscribers: mehdi_amini, aaron.ballman, cfe-commits, thakis, dcheng

Differential Revision: https://reviews.llvm.org/D24289

llvm-svn: 287177

4 files changed, 65 insertions, 1 deletions

diff --git a/clang/include/clang/Basic/DiagnosticGroups.td b/clang/include/clang/Basic/DiagnosticGroups.td
index 35a3473f6a4..ad92b4da685 100644
--- a/clang/include/clang/Basic/DiagnosticGroups.td
+++ b/clang/include/clang/Basic/DiagnosticGroups.td
@@ -543,6 +543,7 @@ def GCCWriteStrings : DiagGroup<"write-strings" , [WritableStrings]>;
 def CharSubscript : DiagGroup<"char-subscripts">;
 def LargeByValueCopy : DiagGroup<"large-by-value-copy">;
 def DuplicateArgDecl : DiagGroup<"duplicate-method-arg">;
+def SignedEnumBitfield : DiagGroup<"signed-enum-bitfield">;
 
 // Unreachable code warning groups.
 //
@@ -661,6 +662,7 @@ def Most : DiagGroup<"most", [
     ReturnType,
     SelfAssignment,
     SelfMove,
+    SignedEnumBitfield,
     SizeofArrayArgument,
     SizeofArrayDecay,
     StringPlusInt,
diff --git a/clang/include/clang/Basic/DiagnosticSemaKinds.td b/clang/include/clang/Basic/DiagnosticSemaKinds.td
index 85aa1c5f6e6..32c9ca03ee5 100644
--- a/clang/include/clang/Basic/DiagnosticSemaKinds.td
+++ b/clang/include/clang/Basic/DiagnosticSemaKinds.td
@@ -3020,6 +3020,10 @@ def warn_int_to_void_pointer_cast : Warning<
   "cast to %1 from smaller integer type %0">,
   InGroup<IntToVoidPointerCast>;
 
+def warn_no_underlying_type_specified_for_enum_bitfield : Warning<
+  "enums in the Microsoft ABI are signed integers by default; consider giving "
+  "the enum %0 an unsigned underlying type to make this code portable">,
+  InGroup<DiagGroup<"signed-enum-bitfield">>, DefaultIgnore;
 def warn_attribute_packed_for_bitfield : Warning<
   "'packed' attribute was ignored on bit-fields with single-byte alignment "
   "in older versions of GCC and Clang">,
diff --git a/clang/lib/Sema/SemaChecking.cpp b/clang/lib/Sema/SemaChecking.cpp
index 56bc3ace139..c1db062f35d 100644
--- a/clang/lib/Sema/SemaChecking.cpp
+++ b/clang/lib/Sema/SemaChecking.cpp
@@ -8518,6 +8518,24 @@ bool AnalyzeBitFieldAssignment(Sema &S, FieldDecl *Bitfield, Expr *Init,
     return false;
 
   // White-list bool bitfields.
+  QualType BitfieldType = Bitfield->getType();
+  if (BitfieldType->isBooleanType())
+     return false;
+
+  if (BitfieldType->isEnumeralType()) {
+    EnumDecl *BitfieldEnumDecl = BitfieldType->getAs<EnumType>()->getDecl();
+    // If the underlying enum type was not explicitly specified as an unsigned
+    // type and the enum contain only positive values, MSVC++ will cause an
+    // inconsistency by storing this as a signed type.
+    if (S.getLangOpts().CPlusPlus11 &&
+        !BitfieldEnumDecl->getIntegerTypeSourceInfo() &&
+        BitfieldEnumDecl->getNumPositiveBits() > 0 &&
+        BitfieldEnumDecl->getNumNegativeBits() == 0) {
+      S.Diag(InitLoc, diag::warn_no_underlying_type_specified_for_enum_bitfield)
+        << BitfieldEnumDecl->getNameAsString();
+    }
+  }
+
   if (Bitfield->getType()->isBooleanType())
     return false;
 
@@ -8547,7 +8565,7 @@ bool AnalyzeBitFieldAssignment(Sema &S, FieldDecl *Bitfield, Expr *Init,
 
   // Compute the value which the bitfield will contain.
   llvm::APSInt TruncatedValue = Value.trunc(FieldWidth);
-  TruncatedValue.setIsSigned(Bitfield->getType()->isSignedIntegerType());
+  TruncatedValue.setIsSigned(BitfieldType->isSignedIntegerType());
 
   // Check whether the stored value is equal to the original value.
   TruncatedValue = TruncatedValue.extend(OriginalWidth);
diff --git a/clang/test/SemaCXX/warn-msvc-enum-bitfield.cpp b/clang/test/SemaCXX/warn-msvc-enum-bitfield.cpp
new file mode 100644
index 00000000000..99e1669018a
--- /dev/null
+++ b/clang/test/SemaCXX/warn-msvc-enum-bitfield.cpp
@@ -0,0 +1,40 @@
+// RUN: %clang_cc1 -fsyntax-only -Wsigned-enum-bitfield -verify %s --std=c++11
+
+// Enums used in bitfields with no explicitly specified underlying type.
+void test0() {
+  enum E { E1, E2 };
+  enum F { F1, F2 };
+  struct { E e1 : 1; E e2; F f1 : 1; F f2; } s;
+
+  s.e1 = E1; // expected-warning {{enums in the Microsoft ABI are signed integers by default; consider giving the enum E an unsigned underlying type to make this code portable}}
+  s.f1 = F1; // expected-warning {{enums in the Microsoft ABI are signed integers by default; consider giving the enum F an unsigned underlying type to make this code portable}}
+
+  s.e2 = E2;
+  s.f2 = F2;
+}
+
+// Enums used in bitfields with an explicit signed underlying type.
+void test1() {
+  enum E : signed { E1, E2 };
+  enum F : long { F1, F2 };
+  struct { E e1 : 1; E e2; F f1 : 1; F f2; } s;
+
+  s.e1 = E1;
+  s.f1 = F1;
+
+  s.e2 = E2;
+  s.f2 = F2;
+}
+
+// Enums used in bitfields with an explicitly unsigned underlying type.
+void test3() {
+  enum E : unsigned { E1, E2 };
+  enum F : unsigned long { F1, F2 };
+  struct { E e1 : 1; E e2; F f1 : 1; F f2; } s;
+
+  s.e1 = E1;
+  s.f1 = F1;
+
+  s.e2 = E2;
+  s.f2 = F2;
+}