Properly implement the scope restriction on the NRVO for
throw-expressions, such that we don't consider the NRVO when the
non-volatile automatic object comes from outside the innermost try
scope (C++0x [class.copymove]p13). In C++98/03, our ASTs were
incorrect but it didn't matter because IR generation doesn't actually
apply the NRVO here. In C++0x, however, we were moving from an object
when in fact we should have copied from it. Fixes PR10142 /
<rdar://problem/9714312>.





git-svn-id: https://llvm.org/svn/llvm-project/cfe/trunk@134548 91177308-0d34-0410-b5e6-96231b3b80d8
diff --git a/include/clang/AST/ExprCXX.h b/include/clang/AST/ExprCXX.h
index 8f650f0..e23b02c 100644
--- a/include/clang/AST/ExprCXX.h
+++ b/include/clang/AST/ExprCXX.h
@@ -586,24 +586,35 @@
 class CXXThrowExpr : public Expr {
   Stmt *Op;
   SourceLocation ThrowLoc;
+  /// \brief Whether the thrown variable (if any) is in scope.
+  unsigned IsThrownVariableInScope : 1;
+  
+  friend class ASTStmtReader;
+  
 public:
   // Ty is the void type which is used as the result type of the
   // exepression.  The l is the location of the throw keyword.  expr
   // can by null, if the optional expression to throw isn't present.
-  CXXThrowExpr(Expr *expr, QualType Ty, SourceLocation l) :
+  CXXThrowExpr(Expr *expr, QualType Ty, SourceLocation l,
+               bool IsThrownVariableInScope) :
     Expr(CXXThrowExprClass, Ty, VK_RValue, OK_Ordinary, false, false,
          expr && expr->isInstantiationDependent(),
          expr && expr->containsUnexpandedParameterPack()),
-    Op(expr), ThrowLoc(l) {}
+    Op(expr), ThrowLoc(l), IsThrownVariableInScope(IsThrownVariableInScope) {}
   CXXThrowExpr(EmptyShell Empty) : Expr(CXXThrowExprClass, Empty) {}
 
   const Expr *getSubExpr() const { return cast_or_null<Expr>(Op); }
   Expr *getSubExpr() { return cast_or_null<Expr>(Op); }
-  void setSubExpr(Expr *E) { Op = E; }
 
   SourceLocation getThrowLoc() const { return ThrowLoc; }
-  void setThrowLoc(SourceLocation L) { ThrowLoc = L; }
 
+  /// \brief Determines whether the variable thrown by this expression (if any!)
+  /// is within the innermost try block.
+  ///
+  /// This information is required to determine whether the NRVO can apply to
+  /// this variable.
+  bool isThrownVariableInScope() const { return IsThrownVariableInScope; }
+  
   SourceRange getSourceRange() const {
     if (getSubExpr() == 0)
       return SourceRange(ThrowLoc, ThrowLoc);
diff --git a/include/clang/Parse/Parser.h b/include/clang/Parse/Parser.h
index a0fd4d5..8f49dda 100644
--- a/include/clang/Parse/Parser.h
+++ b/include/clang/Parse/Parser.h
@@ -1314,6 +1314,9 @@
   StmtResult ParseDefaultStatement(ParsedAttributes &Attr);
   StmtResult ParseCompoundStatement(ParsedAttributes &Attr,
                                     bool isStmtExpr = false);
+  StmtResult ParseCompoundStatement(ParsedAttributes &Attr,
+                                    bool isStmtExpr,
+                                    unsigned ScopeFlags);
   StmtResult ParseCompoundStatementBody(bool isStmtExpr = false);
   bool ParseParenExprOrCondition(ExprResult &ExprResult,
                                  Decl *&DeclResult,
diff --git a/include/clang/Sema/Scope.h b/include/clang/Sema/Scope.h
index 7514633..95d2978 100644
--- a/include/clang/Sema/Scope.h
+++ b/include/clang/Sema/Scope.h
@@ -84,7 +84,10 @@
     /// ThisScope - This is the scope of a struct/union/class definition,
     /// outside of any member function definition, where 'this' is nonetheless
     /// usable.
-    ThisScope = 0x1000
+    ThisScope = 0x1000,
+    
+    /// TryScope - This is the scope of a C++ try statement.
+    TryScope = 0x2000
   };
 private:
   /// The parent scope for this scope.  This is null for the translation-unit
@@ -303,6 +306,9 @@
     }
     return false;
   }
+  
+  /// \brief Determine whether this scope is a C++ 'try' block.
+  bool isTryScope() const { return getFlags() & Scope::TryScope; }
 
   typedef UsingDirectivesTy::iterator udir_iterator;
   typedef UsingDirectivesTy::const_iterator const_udir_iterator;
diff --git a/include/clang/Sema/Sema.h b/include/clang/Sema/Sema.h
index 4682404..30719c1 100644
--- a/include/clang/Sema/Sema.h
+++ b/include/clang/Sema/Sema.h
@@ -1336,7 +1336,8 @@
   ExprResult PerformMoveOrCopyInitialization(const InitializedEntity &Entity,
                                              const VarDecl *NRVOCandidate,
                                              QualType ResultType,
-                                             Expr *Value);
+                                             Expr *Value,
+                                             bool AllowNRVO = true);
   
   bool CanPerformCopyInitialization(const InitializedEntity &Entity,
                                     ExprResult Init);
@@ -2885,8 +2886,11 @@
   ExprResult ActOnCXXNullPtrLiteral(SourceLocation Loc);
 
   //// ActOnCXXThrow -  Parse throw expressions.
-  ExprResult ActOnCXXThrow(SourceLocation OpLoc, Expr *expr);
-  ExprResult CheckCXXThrowOperand(SourceLocation ThrowLoc, Expr *E);
+  ExprResult ActOnCXXThrow(Scope *S, SourceLocation OpLoc, Expr *expr);
+  ExprResult BuildCXXThrow(SourceLocation OpLoc, Expr *Ex, 
+                           bool IsThrownVarInScope);
+  ExprResult CheckCXXThrowOperand(SourceLocation ThrowLoc, Expr *E, 
+                                  bool IsThrownVarInScope);
 
   /// ActOnCXXTypeConstructExpr - Parse construction of a specified type.
   /// Can be interpreted either as function-style casting ("int(x)")
diff --git a/lib/Parse/ParseExprCXX.cpp b/lib/Parse/ParseExprCXX.cpp
index 6822681..b32eeda 100644
--- a/lib/Parse/ParseExprCXX.cpp
+++ b/lib/Parse/ParseExprCXX.cpp
@@ -787,12 +787,12 @@
   case tok::r_brace:
   case tok::colon:
   case tok::comma:
-    return Actions.ActOnCXXThrow(ThrowLoc, 0);
+    return Actions.ActOnCXXThrow(getCurScope(), ThrowLoc, 0);
 
   default:
     ExprResult Expr(ParseAssignmentExpression());
     if (Expr.isInvalid()) return move(Expr);
-    return Actions.ActOnCXXThrow(ThrowLoc, Expr.take());
+    return Actions.ActOnCXXThrow(getCurScope(), ThrowLoc, Expr.take());
   }
 }
 
diff --git a/lib/Parse/ParseStmt.cpp b/lib/Parse/ParseStmt.cpp
index 28b34fe..47a9ecc 100644
--- a/lib/Parse/ParseStmt.cpp
+++ b/lib/Parse/ParseStmt.cpp
@@ -647,6 +647,10 @@
                                   SubStmt.get(), getCurScope());
 }
 
+StmtResult Parser::ParseCompoundStatement(ParsedAttributes &Attr,
+                                          bool isStmtExpr) {
+  return ParseCompoundStatement(Attr, isStmtExpr, Scope::DeclScope);
+}
 
 /// ParseCompoundStatement - Parse a "{}" block.
 ///
@@ -676,14 +680,15 @@
 /// [OMP]   flush-directive
 ///
 StmtResult Parser::ParseCompoundStatement(ParsedAttributes &attrs,
-                                                        bool isStmtExpr) {
+                                          bool isStmtExpr,
+                                          unsigned ScopeFlags) {
   //FIXME: Use attributes?
 
   assert(Tok.is(tok::l_brace) && "Not a compount stmt!");
 
   // Enter a scope to hold everything within the compound stmt.  Compound
   // statements can always hold declarations.
-  ParseScope CompoundScope(this, Scope::DeclScope);
+  ParseScope CompoundScope(this, ScopeFlags);
 
   // Parse the statements in the body.
   return ParseCompoundStatementBody(isStmtExpr);
@@ -1910,7 +1915,8 @@
     return StmtError(Diag(Tok, diag::err_expected_lbrace));
   // FIXME: Possible draft standard bug: attribute-specifier should be allowed?
   ParsedAttributesWithRange attrs(AttrFactory);
-  StmtResult TryBlock(ParseCompoundStatement(attrs));
+  StmtResult TryBlock(ParseCompoundStatement(attrs, /*isStmtExpr=*/false,
+                                             Scope::DeclScope|Scope::TryScope));
   if (TryBlock.isInvalid())
     return move(TryBlock);
 
diff --git a/lib/Sema/SemaExprCXX.cpp b/lib/Sema/SemaExprCXX.cpp
index 8dc9cee..6ae48dd 100644
--- a/lib/Sema/SemaExprCXX.cpp
+++ b/lib/Sema/SemaExprCXX.cpp
@@ -480,23 +480,63 @@
 
 /// ActOnCXXThrow - Parse throw expressions.
 ExprResult
-Sema::ActOnCXXThrow(SourceLocation OpLoc, Expr *Ex) {
+Sema::ActOnCXXThrow(Scope *S, SourceLocation OpLoc, Expr *Ex) {
+  bool IsThrownVarInScope = false;
+  if (Ex) {
+    // C++0x [class.copymove]p31:
+    //   When certain criteria are met, an implementation is allowed to omit the 
+    //   copy/move construction of a class object [...]
+    //
+    //     - in a throw-expression, when the operand is the name of a 
+    //       non-volatile automatic object (other than a function or catch-
+    //       clause parameter) whose scope does not extend beyond the end of the 
+    //       innermost enclosing try-block (if there is one), the copy/move 
+    //       operation from the operand to the exception object (15.1) can be 
+    //       omitted by constructing the automatic object directly into the 
+    //       exception object
+    if (DeclRefExpr *DRE = dyn_cast<DeclRefExpr>(Ex->IgnoreParens()))
+      if (VarDecl *Var = dyn_cast<VarDecl>(DRE->getDecl())) {
+        if (Var->hasLocalStorage() && !Var->getType().isVolatileQualified()) {
+          for( ; S; S = S->getParent()) {
+            if (S->isDeclScope(Var)) {
+              IsThrownVarInScope = true;
+              break;
+            }
+            
+            if (S->getFlags() &
+                (Scope::FnScope | Scope::ClassScope | Scope::BlockScope |
+                 Scope::FunctionPrototypeScope | Scope::ObjCMethodScope |
+                 Scope::TryScope))
+              break;
+          }
+        }
+      }
+  }
+  
+  return BuildCXXThrow(OpLoc, Ex, IsThrownVarInScope);
+}
+
+ExprResult Sema::BuildCXXThrow(SourceLocation OpLoc, Expr *Ex, 
+                               bool IsThrownVarInScope) {
   // Don't report an error if 'throw' is used in system headers.
   if (!getLangOptions().CXXExceptions &&
       !getSourceManager().isInSystemHeader(OpLoc))
     Diag(OpLoc, diag::err_exceptions_disabled) << "throw";
-
+  
   if (Ex && !Ex->isTypeDependent()) {
-    ExprResult ExRes = CheckCXXThrowOperand(OpLoc, Ex);
+    ExprResult ExRes = CheckCXXThrowOperand(OpLoc, Ex, IsThrownVarInScope);
     if (ExRes.isInvalid())
       return ExprError();
     Ex = ExRes.take();
   }
-  return Owned(new (Context) CXXThrowExpr(Ex, Context.VoidTy, OpLoc));
+  
+  return Owned(new (Context) CXXThrowExpr(Ex, Context.VoidTy, OpLoc,
+                                          IsThrownVarInScope));
 }
 
 /// CheckCXXThrowOperand - Validate the operand of a throw.
-ExprResult Sema::CheckCXXThrowOperand(SourceLocation ThrowLoc, Expr *E) {
+ExprResult Sema::CheckCXXThrowOperand(SourceLocation ThrowLoc, Expr *E,
+                                      bool IsThrownVarInScope) {
   // C++ [except.throw]p3:
   //   A throw-expression initializes a temporary object, called the exception
   //   object, the type of which is determined by removing any top-level
@@ -535,14 +575,28 @@
 
   // Initialize the exception result.  This implicitly weeds out
   // abstract types or types with inaccessible copy constructors.
-  const VarDecl *NRVOVariable = getCopyElisionCandidate(QualType(), E, false);
-
-  // FIXME: Determine whether we can elide this copy per C++0x [class.copy]p32.
+  
+  // C++0x [class.copymove]p31:
+  //   When certain criteria are met, an implementation is allowed to omit the 
+  //   copy/move construction of a class object [...]
+  //
+  //     - in a throw-expression, when the operand is the name of a 
+  //       non-volatile automatic object (other than a function or catch-clause 
+  //       parameter) whose scope does not extend beyond the end of the 
+  //       innermost enclosing try-block (if there is one), the copy/move 
+  //       operation from the operand to the exception object (15.1) can be 
+  //       omitted by constructing the automatic object directly into the 
+  //       exception object
+  const VarDecl *NRVOVariable = 0;
+  if (IsThrownVarInScope)
+    NRVOVariable = getCopyElisionCandidate(QualType(), E, false);
+  
   InitializedEntity Entity =
       InitializedEntity::InitializeException(ThrowLoc, E->getType(),
-                                             /*NRVO=*/false);
+                                             /*NRVO=*/NRVOVariable != 0);
   Res = PerformMoveOrCopyInitialization(Entity, NRVOVariable,
-                                        QualType(), E);
+                                        QualType(), E,
+                                        IsThrownVarInScope);
   if (Res.isInvalid())
     return ExprError();
   E = Res.take();
diff --git a/lib/Sema/SemaStmt.cpp b/lib/Sema/SemaStmt.cpp
index 164c0ce..65f431d 100644
--- a/lib/Sema/SemaStmt.cpp
+++ b/lib/Sema/SemaStmt.cpp
@@ -1543,7 +1543,8 @@
 Sema::PerformMoveOrCopyInitialization(const InitializedEntity &Entity,
                                       const VarDecl *NRVOCandidate,
                                       QualType ResultType,
-                                      Expr *Value) {
+                                      Expr *Value,
+                                      bool AllowNRVO) {
   // C++0x [class.copy]p33:
   //   When the criteria for elision of a copy operation are met or would
   //   be met save for the fact that the source object is a function
@@ -1551,7 +1552,8 @@
   //   overload resolution to select the constructor for the copy is first
   //   performed as if the object were designated by an rvalue.
   ExprResult Res = ExprError();
-  if (NRVOCandidate || getCopyElisionCandidate(ResultType, Value, true)) {
+  if (AllowNRVO &&
+      (NRVOCandidate || getCopyElisionCandidate(ResultType, Value, true))) {
     ImplicitCastExpr AsRvalue(ImplicitCastExpr::OnStack,
                               Value->getType(), CK_LValueToRValue,
                               Value, VK_XValue);
diff --git a/lib/Sema/TreeTransform.h b/lib/Sema/TreeTransform.h
index 66ea7df..040ba9e 100644
--- a/lib/Sema/TreeTransform.h
+++ b/lib/Sema/TreeTransform.h
@@ -1866,8 +1866,9 @@
   ///
   /// By default, performs semantic analysis to build the new expression.
   /// Subclasses may override this routine to provide different behavior.
-  ExprResult RebuildCXXThrowExpr(SourceLocation ThrowLoc, Expr *Sub) {
-    return getSema().ActOnCXXThrow(ThrowLoc, Sub);
+  ExprResult RebuildCXXThrowExpr(SourceLocation ThrowLoc, Expr *Sub,
+                                 bool IsThrownVariableInScope) {
+    return getSema().BuildCXXThrow(ThrowLoc, Sub, IsThrownVariableInScope);
   }
 
   /// \brief Build a new C++ default-argument expression.
@@ -6793,7 +6794,8 @@
       SubExpr.get() == E->getSubExpr())
     return SemaRef.Owned(E);
 
-  return getDerived().RebuildCXXThrowExpr(E->getThrowLoc(), SubExpr.get());
+  return getDerived().RebuildCXXThrowExpr(E->getThrowLoc(), SubExpr.get(),
+                                          E->isThrownVariableInScope());
 }
 
 template<typename Derived>
diff --git a/lib/Serialization/ASTReaderStmt.cpp b/lib/Serialization/ASTReaderStmt.cpp
index 1697b3b..a14038d 100644
--- a/lib/Serialization/ASTReaderStmt.cpp
+++ b/lib/Serialization/ASTReaderStmt.cpp
@@ -1172,8 +1172,9 @@
 
 void ASTStmtReader::VisitCXXThrowExpr(CXXThrowExpr *E) {
   VisitExpr(E);
-  E->setThrowLoc(ReadSourceLocation(Record, Idx));
-  E->setSubExpr(Reader.ReadSubExpr());
+  E->ThrowLoc = ReadSourceLocation(Record, Idx);
+  E->Op = Reader.ReadSubExpr();
+  E->IsThrownVariableInScope = Record[Idx++];
 }
 
 void ASTStmtReader::VisitCXXDefaultArgExpr(CXXDefaultArgExpr *E) {
diff --git a/lib/Serialization/ASTWriterStmt.cpp b/lib/Serialization/ASTWriterStmt.cpp
index 91c0b3d..d0e09a5 100644
--- a/lib/Serialization/ASTWriterStmt.cpp
+++ b/lib/Serialization/ASTWriterStmt.cpp
@@ -1174,6 +1174,7 @@
   VisitExpr(E);
   Writer.AddSourceLocation(E->getThrowLoc(), Record);
   Writer.AddStmt(E->getSubExpr());
+  Record.push_back(E->isThrownVariableInScope());
   Code = serialization::EXPR_CXX_THROW;
 }
 
diff --git a/test/CXX/special/class.copy/p33-0x.cpp b/test/CXX/special/class.copy/p33-0x.cpp
index 1e6a025..b196865 100644
--- a/test/CXX/special/class.copy/p33-0x.cpp
+++ b/test/CXX/special/class.copy/p33-0x.cpp
@@ -23,3 +23,35 @@
   throw x2;
 }
   
+namespace PR10142 {
+  struct X {
+    X();
+    X(X&&);
+    X(const X&) = delete; // expected-note 2{{function has been explicitly marked deleted here}}
+  };
+
+  void f(int i) {
+    X x;
+    try {
+      X x2;
+      if (i)
+        throw x2; // okay
+      throw x; // expected-error{{call to deleted constructor of 'PR10142::X'}}
+    } catch (...) {
+    }
+  }
+
+  template<typename T>
+  void f2(int i) {
+    T x;
+    try {
+      T x2;
+      if (i)
+        throw x2; // okay
+      throw x; // expected-error{{call to deleted constructor of 'PR10142::X'}}
+    } catch (...) {
+    }
+  }
+
+  template void f2<X>(int); // expected-note{{in instantiation of function template specialization 'PR10142::f2<PR10142::X>' requested here}}
+}