[cfe-commits] [PATCH] CallExprs returning references can be lvalues

[Jordy Rose]

Fixes a crasher, by allowing CallExprs to be lvalues, and making sure
assignment LHSes are treated as lvalues.

char& ref();
void t5 () {
  ref() = 'c';
}

And an incorrect "undefined dereference" warning, by noting that assigning
to a reference variable does not kill its value.

char t1 () {
  char& r = ref();
  r = 'c'; // no-warning
  if (r) return r;
  return *(char*)0; // no-warning
}
-------------- next part --------------
Index: lib/Analysis/CFG.cpp
===================================================================
--- lib/Analysis/CFG.cpp	(revision 105255)
+++ lib/Analysis/CFG.cpp	(working copy)
@@ -538,6 +538,15 @@
     addStmt(B->getRHS());
     return addStmt(B->getLHS());
   }
+  else if (B->isAssignmentOp()) {
+    if (asc.alwaysAdd()) {
+      autoCreateBlock();
+      AppendStmt(Block, B, asc);
+    }
+    
+    addStmt(B->getRHS());
+    return addStmt(B->getLHS(), AddStmtChoice::AsLValueNotAlwaysAdd);
+  }
 
   return VisitStmt(B, asc);
 }
@@ -612,8 +621,12 @@
   if (!CanThrow(C->getCallee()))
     AddEHEdge = false;
 
-  if (!NoReturn && !AddEHEdge)
-    return VisitStmt(C, AddStmtChoice::AlwaysAdd);
+  if (!NoReturn && !AddEHEdge) {
+    if (asc.asLValue())
+      return VisitStmt(C, AddStmtChoice::AlwaysAddAsLValue);
+    else
+      return VisitStmt(C, AddStmtChoice::AlwaysAdd);
+  }
 
   if (Block) {
     Succ = Block;
Index: lib/Analysis/LiveVariables.cpp
===================================================================
--- lib/Analysis/LiveVariables.cpp	(revision 105255)
+++ lib/Analysis/LiveVariables.cpp	(working copy)
@@ -256,17 +256,21 @@
 
   // Assigning to a variable?
   if (DeclRefExpr* DR = dyn_cast<DeclRefExpr>(LHS->IgnoreParens())) {
+    // Assignments to references don't kill the ref's address
+    if (DR->getDecl()->getType()->isReferenceType()) {
+      VisitDeclRefExpr(DR);
+    } else {
+      // Update liveness inforamtion.
+      unsigned bit = AD.getIdx(DR->getDecl());
+      LiveState.getDeclBit(bit) = Dead | AD.AlwaysLive.getDeclBit(bit);
 
-    // Update liveness inforamtion.
-    unsigned bit = AD.getIdx(DR->getDecl());
-    LiveState.getDeclBit(bit) = Dead | AD.AlwaysLive.getDeclBit(bit);
+      if (AD.Observer) { AD.Observer->ObserverKill(DR); }
 
-    if (AD.Observer) { AD.Observer->ObserverKill(DR); }
-
-    // Handle things like +=, etc., which also generate "uses"
-    // of a variable.  Do this just by visiting the subexpression.
-    if (B->getOpcode() != BinaryOperator::Assign)
-      VisitDeclRefExpr(DR);
+      // Handle things like +=, etc., which also generate "uses"
+      // of a variable.  Do this just by visiting the subexpression.
+      if (B->getOpcode() != BinaryOperator::Assign)
+        VisitDeclRefExpr(DR);
+    }
   }
   else // Not assigning to a variable.  Process LHS as usual.
     Visit(LHS);
@@ -290,8 +294,14 @@
       // transfer function for this expression after the DeclStmt.  If the
       // initializer references the variable (which is bad) then we extend
       // its liveness.
-      if (Expr* Init = VD->getInit())
+      if (Expr* Init = VD->getInit()) {
+        // if (VD->getType()->isReferenceType()) {
+        //   if (ImplicitCastExpr* Cast = dyn_cast<ImplicitCastExpr>(Init))
+        //     Cast->setLvalueCast(true);
+        // }
+
         Visit(Init);
+      }
 
       if (const VariableArrayType* VT =
             AD.getContext().getAsVariableArrayType(VD->getType())) {
Index: test/Analysis/reference.cpp
===================================================================
--- test/Analysis/reference.cpp	(revision 105255)
+++ test/Analysis/reference.cpp	(working copy)
@@ -9,3 +9,48 @@
   if (b != 3)
     *p = 1; // no-warning
 }
+
+typedef typeof(sizeof(int)) size_t;
+void malloc (size_t);
+
+char* ptr();
+char& ref();
+
+// Each of the tests below is repeated with pointers as well as references.
+// This is mostly a sanity check, but then again, both should work!
+char t1 () {
+  char& r = ref();
+  r = 'c'; // no-warning
+  if (r) return r;
+  return *(char*)0; // no-warning
+}
+
+char t2 () {
+  char* p = ptr();
+  *p = 'c'; // no-warning
+  if (*p) return *p;
+  return *(char*)0; // no-warning
+}
+
+char t3 (char& r) {
+  r = 'c'; // no-warning
+  if (r) return r;
+  return *(char*)0; // no-warning
+}
+
+char t4 (char* p) {
+  *p = 'c'; // no-warning
+  if (*p) return *p;
+  return *(char*)0; // no-warning
+}
+
+// These next two tests just shouldn't crash.
+char t5 () {
+  ref() = 'c';
+  return '0';
+}
+
+char t6 () {
+  *ptr() = 'c';
+  return '0';
+}