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

[Jordy Rose]

On Thu, 3 Jun 2010 12:21:09 +0800, Zhongxing Xu <xuzhongxing at gmail.com>
wrote:
> One small suggestion: It's easier to review if each patch only solves
one
> specific problem. We'd like small patches that fix one well defined and
> isolated problem.

Thanks for the correction/reminder, my mistake. (Once again it's because I
did the non-dependent part second.)



> Comments below:
> 
> 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());
> 
> Is it necessary to add all RHS of assignment expr as block-level expr?

Ah. No, probably not. I was trying to imitate the default VisitStmt()
behavior and change things as little as possible, but I must have gotten
addStmt() and Visit() crossed in my mind. I'm not even sure the LHS needs
to be addStmt()ed.

Just so my understanding is clear: addStmt() / "always add" is only for
statements that affect control-flow, correct?

Revised patch attached, just for the crasher on "ref() = 0". Per advice
I'll re-submit the second one once the first one's committed.

Jordy
-------------- 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);
+    }
+    
+    Visit(B->getRHS());
+    return Visit(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: test/Analysis/reference.cpp
===================================================================
--- test/Analysis/reference.cpp	(revision 105344)
+++ test/Analysis/reference.cpp	(working copy)
@@ -9,3 +9,18 @@
   if (b != 3)
     *p = 1; // no-warning
 }
+
+char* ptr();
+char& ref();
+
+// These next two tests just shouldn't crash.
+char t1 () {
+  ref() = 'c';
+  return '0';
+}
+
+// just a sanity test, the same behavior as t1()
+char t2 () {
+  *ptr() = 'c';
+  return '0';
+}