[cfe-commits] r134548 - in /cfe/trunk: include/clang/AST/ExprCXX.h include/clang/Parse/Parser.h include/clang/Sema/Scope.h include/clang/Sema/Sema.h lib/Parse/ParseExprCXX.cpp lib/Parse/ParseStmt.cpp lib/Sema/SemaExprCXX.cpp lib/Sema/SemaStmt.cpp lib/Sema/TreeTransform.h lib/Serialization/ASTReaderStmt.cpp lib/Serialization/ASTWriterStmt.cpp test/CXX/special/class.copy/p33-0x.cpp

Douglas Gregor dgregor at apple.com
Wed Jul 6 15:04:07 PDT 2011


Author: dgregor
Date: Wed Jul  6 17:04:06 2011
New Revision: 134548

URL: http://llvm.org/viewvc/llvm-project?rev=134548&view=rev
Log:
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>.




Modified:
    cfe/trunk/include/clang/AST/ExprCXX.h
    cfe/trunk/include/clang/Parse/Parser.h
    cfe/trunk/include/clang/Sema/Scope.h
    cfe/trunk/include/clang/Sema/Sema.h
    cfe/trunk/lib/Parse/ParseExprCXX.cpp
    cfe/trunk/lib/Parse/ParseStmt.cpp
    cfe/trunk/lib/Sema/SemaExprCXX.cpp
    cfe/trunk/lib/Sema/SemaStmt.cpp
    cfe/trunk/lib/Sema/TreeTransform.h
    cfe/trunk/lib/Serialization/ASTReaderStmt.cpp
    cfe/trunk/lib/Serialization/ASTWriterStmt.cpp
    cfe/trunk/test/CXX/special/class.copy/p33-0x.cpp

Modified: cfe/trunk/include/clang/AST/ExprCXX.h
URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/include/clang/AST/ExprCXX.h?rev=134548&r1=134547&r2=134548&view=diff
==============================================================================
--- cfe/trunk/include/clang/AST/ExprCXX.h (original)
+++ cfe/trunk/include/clang/AST/ExprCXX.h Wed Jul  6 17:04:06 2011
@@ -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);

Modified: cfe/trunk/include/clang/Parse/Parser.h
URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/include/clang/Parse/Parser.h?rev=134548&r1=134547&r2=134548&view=diff
==============================================================================
--- cfe/trunk/include/clang/Parse/Parser.h (original)
+++ cfe/trunk/include/clang/Parse/Parser.h Wed Jul  6 17:04:06 2011
@@ -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,

Modified: cfe/trunk/include/clang/Sema/Scope.h
URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/include/clang/Sema/Scope.h?rev=134548&r1=134547&r2=134548&view=diff
==============================================================================
--- cfe/trunk/include/clang/Sema/Scope.h (original)
+++ cfe/trunk/include/clang/Sema/Scope.h Wed Jul  6 17:04:06 2011
@@ -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;

Modified: cfe/trunk/include/clang/Sema/Sema.h
URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/include/clang/Sema/Sema.h?rev=134548&r1=134547&r2=134548&view=diff
==============================================================================
--- cfe/trunk/include/clang/Sema/Sema.h (original)
+++ cfe/trunk/include/clang/Sema/Sema.h Wed Jul  6 17:04:06 2011
@@ -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)")

Modified: cfe/trunk/lib/Parse/ParseExprCXX.cpp
URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/Parse/ParseExprCXX.cpp?rev=134548&r1=134547&r2=134548&view=diff
==============================================================================
--- cfe/trunk/lib/Parse/ParseExprCXX.cpp (original)
+++ cfe/trunk/lib/Parse/ParseExprCXX.cpp Wed Jul  6 17:04:06 2011
@@ -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());
   }
 }
 

Modified: cfe/trunk/lib/Parse/ParseStmt.cpp
URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/Parse/ParseStmt.cpp?rev=134548&r1=134547&r2=134548&view=diff
==============================================================================
--- cfe/trunk/lib/Parse/ParseStmt.cpp (original)
+++ cfe/trunk/lib/Parse/ParseStmt.cpp Wed Jul  6 17:04:06 2011
@@ -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);
 

Modified: cfe/trunk/lib/Sema/SemaExprCXX.cpp
URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/Sema/SemaExprCXX.cpp?rev=134548&r1=134547&r2=134548&view=diff
==============================================================================
--- cfe/trunk/lib/Sema/SemaExprCXX.cpp (original)
+++ cfe/trunk/lib/Sema/SemaExprCXX.cpp Wed Jul  6 17:04:06 2011
@@ -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();

Modified: cfe/trunk/lib/Sema/SemaStmt.cpp
URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/Sema/SemaStmt.cpp?rev=134548&r1=134547&r2=134548&view=diff
==============================================================================
--- cfe/trunk/lib/Sema/SemaStmt.cpp (original)
+++ cfe/trunk/lib/Sema/SemaStmt.cpp Wed Jul  6 17:04:06 2011
@@ -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);

Modified: cfe/trunk/lib/Sema/TreeTransform.h
URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/Sema/TreeTransform.h?rev=134548&r1=134547&r2=134548&view=diff
==============================================================================
--- cfe/trunk/lib/Sema/TreeTransform.h (original)
+++ cfe/trunk/lib/Sema/TreeTransform.h Wed Jul  6 17:04:06 2011
@@ -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>

Modified: cfe/trunk/lib/Serialization/ASTReaderStmt.cpp
URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/Serialization/ASTReaderStmt.cpp?rev=134548&r1=134547&r2=134548&view=diff
==============================================================================
--- cfe/trunk/lib/Serialization/ASTReaderStmt.cpp (original)
+++ cfe/trunk/lib/Serialization/ASTReaderStmt.cpp Wed Jul  6 17:04:06 2011
@@ -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) {

Modified: cfe/trunk/lib/Serialization/ASTWriterStmt.cpp
URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/Serialization/ASTWriterStmt.cpp?rev=134548&r1=134547&r2=134548&view=diff
==============================================================================
--- cfe/trunk/lib/Serialization/ASTWriterStmt.cpp (original)
+++ cfe/trunk/lib/Serialization/ASTWriterStmt.cpp Wed Jul  6 17:04:06 2011
@@ -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;
 }
 

Modified: cfe/trunk/test/CXX/special/class.copy/p33-0x.cpp
URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/test/CXX/special/class.copy/p33-0x.cpp?rev=134548&r1=134547&r2=134548&view=diff
==============================================================================
--- cfe/trunk/test/CXX/special/class.copy/p33-0x.cpp (original)
+++ cfe/trunk/test/CXX/special/class.copy/p33-0x.cpp Wed Jul  6 17:04:06 2011
@@ -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}}
+}





More information about the cfe-commits mailing list