[PATCH] Force a load when creating a reference to a temporary copied from a bitfield.

[Jordan Rose]

Hi doug.gregor, rsmith,

For this source:
  const int &ref = someStruct.bitfield;

We used to generate this AST:

  DeclStmt [...]
  `-VarDecl [...] ref 'const int &'
    `-MaterializeTemporaryExpr [...] 'const int' lvalue
      `-ImplicitCastExpr [...] 'const int' lvalue <NoOp>
        `-MemberExpr [...] 'int' lvalue bitfield .bitfield [...]
          `-DeclRefExpr [...] 'struct X' lvalue ParmVar [...] 'someStruct' 'struct X'

Notice the lvalue inside the MaterializeTemporaryExpr, which is very confusing (and caused an assertion to fire in the analyzer -PR15694).

We now generate this:

  DeclStmt [...]
  `-VarDecl [...] ref 'const int &'
    `-MaterializeTemporaryExpr [...] 'const int' lvalue
      `-ImplicitCastExpr [...] 'int' <LValueToRValue>
        `-MemberExpr [...] 'int' lvalue bitfield .bitfield [...]
          `-DeclRefExpr [...] 'struct X' lvalue ParmVar [...] 'someStruct' 'struct X'

Which makes a lot more sense, and matches what CodeGen would do anyway. (CGExpr.cpp:232).

The commit also makes Clang accept this (legal) C++11 code:

  int &&ref = std::move(someStruct).bitfield

[[http://llvm.org/bugs/show_bug.cgi?id=15694 | PR15694]] / <rdar://problem/13600396>

http://llvm-reviews.chandlerc.com/D651

Files:
  lib/Sema/SemaInit.cpp
  test/CXX/dcl.decl/dcl.init/dcl.init.ref/p5-0x.cpp

Index: lib/Sema/SemaInit.cpp
===================================================================
--- lib/Sema/SemaInit.cpp
+++ lib/Sema/SemaInit.cpp
@@ -3351,6 +3351,48 @@
                                  T1Quals, cv2T2, T2, T2Quals, Sequence);
 }
 
+static ExprValueKind
+convertQualifiersAndValueKindIfNecessary(Sema &S,
+                                         InitializationSequence &Sequence,
+                                         Expr *Initializer,
+                                         QualType cv1T1,
+                                         Qualifiers T1Quals,
+                                         Qualifiers T2Quals,
+                                         bool IsLValueRef) {
+  bool IsNonAddressableType = Initializer->getBitField() ||
+                              Initializer->refersToVectorElement();
+
+  if (IsNonAddressableType) {
+    // If this is a non-const or volatile lvalue reference, we can't bind to
+    // this value. Give up.
+    if (IsLValueRef && (!T1Quals.hasConst() || T1Quals.hasVolatile())) {
+      assert(Initializer->isGLValue());
+      return Initializer->getValueKind();
+    }
+
+    // Force an lvalue-to-rvalue conversion from the non-addressable lvalue
+    // types (bitfields and vector elements).
+    ImplicitConversionSequence ICS
+      = S.TryImplicitConversion(Initializer, cv1T1,
+                                /*SuppressUserConversions=*/true,
+                                /*AllowExplicit=*/false,
+                                /*FIXME:InOverloadResolution=*/false,
+                                /*CStyle=*/false,
+                                /*AllowObjCWritebackConversion=*/false);
+    assert(!ICS.isBad());
+    Sequence.AddConversionSequenceStep(ICS, cv1T1);
+    return VK_RValue;
+  }
+
+  if (T1Quals != T2Quals) {
+    Sequence.AddQualificationConversionStep(cv1T1,
+                                            Initializer->getValueKind());
+  }
+
+  return Initializer->getValueKind();
+}
+
+
 /// \brief Reference initialization without resolving overloaded functions.
 static void TryReferenceInitializationCore(Sema &S,
                                            const InitializedEntity &Entity,
@@ -3406,11 +3448,11 @@
         Sequence.AddObjCObjectConversionStep(
                                      S.Context.getQualifiedType(T1, T2Quals));
 
-      if (T1Quals != T2Quals)
-        Sequence.AddQualificationConversionStep(cv1T1, VK_LValue);
-      bool BindingTemporary = T1Quals.hasConst() && !T1Quals.hasVolatile() &&
-        (Initializer->getBitField() || Initializer->refersToVectorElement());
-      Sequence.AddReferenceBindingStep(cv1T1, BindingTemporary);
+      ExprValueKind ValueKind =
+        convertQualifiersAndValueKindIfNecessary(S, Sequence, Initializer,
+                                                 cv1T1, T1Quals, T2Quals,
+                                                 isLValueRef);
+      Sequence.AddReferenceBindingStep(cv1T1, ValueKind == VK_RValue);
       return;
     }
 
@@ -3493,10 +3535,12 @@
       Sequence.AddObjCObjectConversionStep(
                                        S.Context.getQualifiedType(T1, T2Quals));
 
-    if (T1Quals != T2Quals)
-      Sequence.AddQualificationConversionStep(cv1T1, ValueKind);
-    Sequence.AddReferenceBindingStep(cv1T1,
-                                 /*bindingTemporary=*/InitCategory.isPRValue());
+    ValueKind = convertQualifiersAndValueKindIfNecessary(S, Sequence,
+                                                         Initializer, cv1T1,
+                                                         T1Quals, T2Quals,
+                                                         isLValueRef);
+
+    Sequence.AddReferenceBindingStep(cv1T1, ValueKind == VK_RValue);
     return;
   }
 
@@ -5104,6 +5148,9 @@
       break;
 
     case SK_BindReferenceToTemporary:
+      // Make sure the "temporary" is actually an rvalue.
+      assert(CurInit.get()->isRValue() && "not a temporary");
+
       // Check exception specifications
       if (S.CheckExceptionSpecCompatibility(CurInit.get(), DestType))
         return ExprError();
Index: test/CXX/dcl.decl/dcl.init/dcl.init.ref/p5-0x.cpp
===================================================================
--- test/CXX/dcl.decl/dcl.init/dcl.init.ref/p5-0x.cpp
+++ test/CXX/dcl.decl/dcl.init/dcl.init.ref/p5-0x.cpp
@@ -200,3 +200,37 @@
   X &&f1(Y &y) { return y; } // expected-error{{rvalue reference to type 'rdar13278115::X' cannot bind to lvalue of type 'rdar13278115::Y'}}
   const X &&f2(Y &y) { return y; } // expected-error{{rvalue reference to type 'const rdar13278115::X' cannot bind to lvalue of type 'rdar13278115::Y'}}
 }
+
+namespace bitfields {
+  struct IntBitfield {
+    int i : 17; // expected-note 3 {{bit-field is declared here}}
+  };
+
+  // A simplified version of std::move.
+  template <typename T>
+  T &&move(T &obj) {
+    return static_cast<T &&>(obj);
+  }
+
+  void test() {
+    int & ir1 = (lvalue<IntBitfield>().i); // expected-error{{non-const reference cannot bind to bit-field 'i'}}
+    int & ir2 = (xvalue<IntBitfield>().i); // expected-error{{non-const lvalue reference to type 'int' cannot bind to a temporary of type 'int'}}
+    int && ir3 = (xvalue<IntBitfield>().i); // no-warning
+    int && ir4 = move(lvalue<IntBitfield>()).i; // no-warning
+
+    volatile int & vir1 = (lvalue<IntBitfield>().i); // expected-error{{non-const reference cannot bind to bit-field 'i'}}
+    volatile int & vir2 = (xvalue<IntBitfield>().i); // expected-error{{volatile lvalue reference to type 'volatile int' cannot bind to a temporary of type 'int'}}
+    volatile int && vir3 = (xvalue<IntBitfield>().i); // no-warning
+    volatile int && vir4 = move(lvalue<IntBitfield>()).i; // no-warning
+
+    const int & cir1 = (lvalue<IntBitfield>().i); // no-warning
+    const int & cir2 = (xvalue<IntBitfield>().i); // no-warning
+    const int && cir3 = (xvalue<IntBitfield>().i); // no-warning
+    const int && cir4 = move(lvalue<IntBitfield>()).i; // no-warning
+
+    const volatile int & cvir1 = (lvalue<IntBitfield>().i); // expected-error{{non-const reference cannot bind to bit-field 'i'}}
+    const volatile int & cvir2 = (xvalue<IntBitfield>().i); // expected-error{{volatile lvalue reference to type 'const volatile int' cannot bind to a temporary of type 'int'}}
+    const volatile int && cvir3 = (xvalue<IntBitfield>().i); // no-warning
+    const volatile int && cvir4 = move(lvalue<IntBitfield>()).i; // no-warning
+  }
+}
-------------- next part --------------
A non-text attachment was scrubbed...
Name: D651.1.patch
Type: text/x-patch
Size: 6512 bytes
Desc: not available
URL: <http://lists.llvm.org/pipermail/cfe-commits/attachments/20130409/6fa13faa/attachment.bin>