r218428 - Fix handling of preincrement on bit-fields. This gives a bit-field in C++, but

[Richard Smith]

Author: rsmith
Date: Wed Sep 24 18:55:00 2014
New Revision: 218428

URL: http://llvm.org/viewvc/llvm-project?rev=218428&view=rev
Log:
Fix handling of preincrement on bit-fields. This gives a bit-field in C++, but
we were failing to find that bit-field when performing integer promotions. This
brings us closer to following the standard, and closer to GCC.

In C, this change is technically a regression: we get bit-field promotions
completely wrong in C, promoting cases that are categorically not bit-field
designators. This change makes us do so slightly more consistently, though.

Added:
    cfe/trunk/test/SemaCXX/bitfield.cpp
Modified:
    cfe/trunk/lib/AST/ASTContext.cpp
    cfe/trunk/lib/AST/Expr.cpp
    cfe/trunk/test/Sema/bitfield.c

Modified: cfe/trunk/lib/AST/ASTContext.cpp
URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/AST/ASTContext.cpp?rev=218428&r1=218427&r2=218428&view=diff
==============================================================================
--- cfe/trunk/lib/AST/ASTContext.cpp (original)
+++ cfe/trunk/lib/AST/ASTContext.cpp Wed Sep 24 18:55:00 2014
@@ -4421,7 +4421,11 @@ unsigned ASTContext::getIntegerRank(cons
 QualType ASTContext::isPromotableBitField(Expr *E) const {
   if (E->isTypeDependent() || E->isValueDependent())
     return QualType();
-  
+
+  // FIXME: We should not do this unless E->refersToBitField() is true. This
+  // matters in C where getSourceBitField() will find bit-fields for various
+  // cases where the source expression is not a bit-field designator.
+
   FieldDecl *Field = E->getSourceBitField(); // FIXME: conditional bit-fields?
   if (!Field)
     return QualType();
@@ -4430,9 +4434,20 @@ QualType ASTContext::isPromotableBitFiel
 
   uint64_t BitWidth = Field->getBitWidthValue(*this);
   uint64_t IntSize = getTypeSize(IntTy);
-  // GCC extension compatibility: if the bit-field size is less than or equal
-  // to the size of int, it gets promoted no matter what its type is.
-  // For instance, unsigned long bf : 4 gets promoted to signed int.
+  // C++ [conv.prom]p5:
+  //   A prvalue for an integral bit-field can be converted to a prvalue of type
+  //   int if int can represent all the values of the bit-field; otherwise, it
+  //   can be converted to unsigned int if unsigned int can represent all the
+  //   values of the bit-field. If the bit-field is larger yet, no integral
+  //   promotion applies to it.
+  // C11 6.3.1.1/2:
+  //   [For a bit-field of type _Bool, int, signed int, or unsigned int:]
+  //   If an int can represent all values of the original type (as restricted by
+  //   the width, for a bit-field), the value is converted to an int; otherwise,
+  //   it is converted to an unsigned int.
+  //
+  // FIXME: C does not permit promotion of a 'long : 3' bitfield to int.
+  //        We perform that promotion here to match GCC and C++.
   if (BitWidth < IntSize)
     return IntTy;
 
@@ -4440,9 +4455,10 @@ QualType ASTContext::isPromotableBitFiel
     return FT->isSignedIntegerType() ? IntTy : UnsignedIntTy;
 
   // Types bigger than int are not subject to promotions, and therefore act
-  // like the base type.
-  // FIXME: This doesn't quite match what gcc does, but what gcc does here
-  // is ridiculous.
+  // like the base type. GCC has some weird bugs in this area that we
+  // deliberately do not follow (GCC follows a pre-standard resolution to
+  // C's DR315 which treats bit-width as being part of the type, and this leaks
+  // into their semantics in some cases).
   return QualType();
 }
 

Modified: cfe/trunk/lib/AST/Expr.cpp
URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/AST/Expr.cpp?rev=218428&r1=218427&r2=218428&view=diff
==============================================================================
--- cfe/trunk/lib/AST/Expr.cpp (original)
+++ cfe/trunk/lib/AST/Expr.cpp Wed Sep 24 18:55:00 2014
@@ -3278,6 +3278,10 @@ FieldDecl *Expr::getSourceBitField() {
       return BinOp->getRHS()->getSourceBitField();
   }
 
+  if (UnaryOperator *UnOp = dyn_cast<UnaryOperator>(E))
+    if (UnOp->isPrefix() && UnOp->isIncrementDecrementOp())
+      return UnOp->getSubExpr()->getSourceBitField();
+
   return nullptr;
 }
 

Modified: cfe/trunk/test/Sema/bitfield.c
URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/test/Sema/bitfield.c?rev=218428&r1=218427&r2=218428&view=diff
==============================================================================
--- cfe/trunk/test/Sema/bitfield.c (original)
+++ cfe/trunk/test/Sema/bitfield.c Wed Sep 24 18:55:00 2014
@@ -1,4 +1,4 @@
-// RUN: %clang_cc1 %s -fsyntax-only -verify 
+// RUN: %clang_cc1 %s -fsyntax-only -verify -std=c11
 enum e0; // expected-note{{forward declaration of 'enum e0'}}
 
 struct a {
@@ -54,3 +54,22 @@ void test4(struct Test4 *t) {
   (void) sizeof(t->var ? t->bitX : t->bitY); // not a bitfield designator in C
   (void) sizeof(t->var ? t->bitX : t->bitX); // not a bitfield designator in C
 }
+
+typedef unsigned Unsigned;
+typedef signed Signed;
+
+struct Test5 { unsigned n : 2; } t5;
+typedef __typeof__(t5.n) Unsigned; // Bitfield is unsigned
+typedef __typeof__(+t5.n) Signed;  // ... but promotes to signed.
+
+typedef __typeof__(t5.n + 0) Signed; // Arithmetic promotes.
+
+typedef __typeof__(+(t5.n = 0)) Signed;  // FIXME: Assignment should not; the result
+typedef __typeof__(+(t5.n += 0)) Signed; // is a non-bit-field lvalue of type unsigned.
+typedef __typeof__(+(t5.n *= 0)) Signed;
+
+typedef __typeof__(+(++t5.n)) Signed; // FIXME: Increment is equivalent to compound-assignment.
+typedef __typeof__(+(--t5.n)) Signed; // This should not promote to signed.
+
+typedef __typeof__(+(t5.n++)) Unsigned; // Post-increment is underspecified, but seems to
+typedef __typeof__(+(t5.n--)) Unsigned; // also act like compound-assignment.

Added: cfe/trunk/test/SemaCXX/bitfield.cpp
URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/test/SemaCXX/bitfield.cpp?rev=218428&view=auto
==============================================================================
--- cfe/trunk/test/SemaCXX/bitfield.cpp (added)
+++ cfe/trunk/test/SemaCXX/bitfield.cpp Wed Sep 24 18:55:00 2014
@@ -0,0 +1,32 @@
+// RUN: %clang_cc1 %s -verify
+
+// expected-no-diagnostics
+
+namespace PromotionVersusMutation {
+  typedef unsigned Unsigned;
+  typedef signed Signed;
+
+  struct T { unsigned n : 2; } t;
+
+  typedef __typeof__(t.n) Unsigned; // Bitfield is unsigned
+  typedef __typeof__(+t.n) Signed;  // ... but promotes to signed.
+
+  typedef __typeof__(t.n + 0) Signed; // Arithmetic promotes.
+
+  typedef __typeof__(t.n = 0) Unsigned;  // Assignment produces an lvalue...
+  typedef __typeof__(t.n += 0) Unsigned;
+  typedef __typeof__(t.n *= 0) Unsigned;
+  typedef __typeof__(+(t.n = 0)) Signed;  // ... which is a bit-field.
+  typedef __typeof__(+(t.n += 0)) Signed;
+  typedef __typeof__(+(t.n *= 0)) Signed;
+
+  typedef __typeof__(++t.n) Unsigned; // Increment is equivalent to compound-assignment.
+  typedef __typeof__(--t.n) Unsigned;
+  typedef __typeof__(+(++t.n)) Signed;
+  typedef __typeof__(+(--t.n)) Signed;
+
+  typedef __typeof__(t.n++) Unsigned; // Post-increment's result has the type
+  typedef __typeof__(t.n--) Unsigned; // of the operand...
+  typedef __typeof__(+(t.n++)) Unsigned; // ... and is not a bit-field (because
+  typedef __typeof__(+(t.n--)) Unsigned; // it's not a glvalue).
+}