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

Jordan Rose jordan_rose at apple.com
Wed Apr 10 10:30:42 PDT 2013


  - Remove now-unnecessary hack in `LValueExprEvaluator::VisitMaterializeTemporaryExpr`.
  - Add original reduced test case to the analyzer tests.

Hi doug.gregor, rsmith,

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

CHANGE SINCE LAST DIFF
  http://llvm-reviews.chandlerc.com/D651?vs=1578&id=1592#toc

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

Index: lib/AST/ExprConstant.cpp
===================================================================
--- lib/AST/ExprConstant.cpp
+++ lib/AST/ExprConstant.cpp
@@ -2865,25 +2865,12 @@
 
 bool LValueExprEvaluator::VisitMaterializeTemporaryExpr(
     const MaterializeTemporaryExpr *E) {
-  if (E->GetTemporaryExpr()->isRValue()) {
-    if (E->getType()->isRecordType())
-      return EvaluateTemporary(E->GetTemporaryExpr(), Result, Info);
+  if (E->getType()->isRecordType())
+    return EvaluateTemporary(E->GetTemporaryExpr(), Result, Info);
 
-    Result.set(E, Info.CurrentCall->Index);
-    return EvaluateInPlace(Info.CurrentCall->Temporaries[E], Info,
-                           Result, E->GetTemporaryExpr());
-  }
-
-  // Materialization of an lvalue temporary occurs when we need to force a copy
-  // (for instance, if it's a bitfield).
-  // FIXME: The AST should contain an lvalue-to-rvalue node for such cases.
-  if (!Visit(E->GetTemporaryExpr()))
-    return false;
-  if (!HandleLValueToRValueConversion(Info, E, E->getType(), Result,
-                                      Info.CurrentCall->Temporaries[E]))
-    return false;
   Result.set(E, Info.CurrentCall->Index);
-  return true;
+  return EvaluateInPlace(Info.CurrentCall->Temporaries[E], Info,
+                         Result, E->GetTemporaryExpr());
 }
 
 bool
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/Analysis/reference.cpp
===================================================================
--- test/Analysis/reference.cpp
+++ test/Analysis/reference.cpp
@@ -224,3 +224,13 @@
     return *x; // no-warning
   }
 }
+
+namespace PR15694 {
+  class C {
+    bool bit : 1;
+    template <class T> void bar(const T &obj) {}
+    void foo() {
+      bar(bit); // don't crash
+    }
+  };
+}
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.2.patch
Type: text/x-patch
Size: 8247 bytes
Desc: not available
URL: <http://lists.llvm.org/pipermail/cfe-commits/attachments/20130410/c419863d/attachment.bin>


More information about the cfe-commits mailing list