[clang] 85ca7e4 - Revert "[clang] NRVO: Improvements and handling of more cases."

Arthur Eubanks via cfe-commits cfe-commits at lists.llvm.org
Thu Jun 10 20:46:19 PDT 2021


Author: Arthur Eubanks
Date: 2021-06-10T20:37:01-07:00
New Revision: 85ca7e424fd050582026a299906c9e8397043c52

URL: https://github.com/llvm/llvm-project/commit/85ca7e424fd050582026a299906c9e8397043c52
DIFF: https://github.com/llvm/llvm-project/commit/85ca7e424fd050582026a299906c9e8397043c52.diff

LOG: Revert "[clang] NRVO: Improvements and handling of more cases."

This reverts commit 667fbcdd0b2ee5e78f5ce9789b862e3bbca94644.

Causes crashes on a stage 2 build on Windows.

Added: 
    

Modified: 
    clang/include/clang/Sema/Sema.h
    clang/lib/Sema/Sema.cpp
    clang/lib/Sema/SemaCoroutine.cpp
    clang/lib/Sema/SemaExprCXX.cpp
    clang/lib/Sema/SemaStmt.cpp
    clang/lib/Sema/SemaTemplateInstantiateDecl.cpp
    clang/test/CodeGen/nrvo-tracking.cpp

Removed: 
    


################################################################################
diff  --git a/clang/include/clang/Sema/Sema.h b/clang/include/clang/Sema/Sema.h
index f7ec89a33e00c..6ade9d7691266 100644
--- a/clang/include/clang/Sema/Sema.h
+++ b/clang/include/clang/Sema/Sema.h
@@ -3455,6 +3455,12 @@ class Sema final {
   bool DiagnoseMultipleUserDefinedConversion(Expr *From, QualType ToType);
   bool isSameOrCompatibleFunctionType(CanQualType Param, CanQualType Arg);
 
+  ExprResult PerformMoveOrCopyInitialization(const InitializedEntity &Entity,
+                                             const VarDecl *NRVOCandidate,
+                                             QualType ResultType,
+                                             Expr *Value,
+                                             bool AllowNRVO = true);
+
   bool CanPerformAggregateInitializationForOverloadResolution(
       const InitializedEntity &Entity, InitListExpr *From);
 
@@ -4754,30 +4760,28 @@ class Sema final {
                                            SourceLocation Loc,
                                            unsigned NumParams);
 
-  struct NamedReturnInfo {
-    const VarDecl *Candidate;
-
-    enum Status : uint8_t { None, MoveEligible, MoveEligibleAndCopyElidable };
-    Status S;
-
-    bool isMoveEligible() const { return S != None; };
-    bool isCopyElidable() const { return S == MoveEligibleAndCopyElidable; }
+  enum CopyElisionSemanticsKind {
+    CES_Strict = 0,
+    CES_AllowParameters = 1,
+    CES_AllowDifferentTypes = 2,
+    CES_AllowExceptionVariables = 4,
+    CES_AllowRValueReferenceType = 8,
+    CES_ImplicitlyMovableCXX11CXX14CXX17 =
+        (CES_AllowParameters | CES_AllowDifferentTypes),
+    CES_ImplicitlyMovableCXX20 =
+        (CES_AllowParameters | CES_AllowDifferentTypes |
+         CES_AllowExceptionVariables | CES_AllowRValueReferenceType),
   };
-  NamedReturnInfo getNamedReturnInfo(const Expr *E, bool ForceCXX20 = false);
-  NamedReturnInfo getNamedReturnInfo(const VarDecl *VD,
-                                     bool ForceCXX20 = false);
-  const VarDecl *getCopyElisionCandidate(NamedReturnInfo &Info,
-                                         QualType ReturnType);
 
-  ExprResult PerformMoveOrCopyInitialization(const InitializedEntity &Entity,
-                                             const NamedReturnInfo &NRInfo,
-                                             Expr *Value);
+  VarDecl *getCopyElisionCandidate(QualType ReturnType, Expr *E,
+                                   CopyElisionSemanticsKind CESK);
+  bool isCopyElisionCandidate(QualType ReturnType, const VarDecl *VD,
+                              CopyElisionSemanticsKind CESK);
 
   StmtResult ActOnReturnStmt(SourceLocation ReturnLoc, Expr *RetValExp,
                              Scope *CurScope);
   StmtResult BuildReturnStmt(SourceLocation ReturnLoc, Expr *RetValExp);
-  StmtResult ActOnCapScopeReturnStmt(SourceLocation ReturnLoc, Expr *RetValExp,
-                                     NamedReturnInfo &NRInfo);
+  StmtResult ActOnCapScopeReturnStmt(SourceLocation ReturnLoc, Expr *RetValExp);
 
   StmtResult ActOnGCCAsmStmt(SourceLocation AsmLoc, bool IsSimple,
                              bool IsVolatile, unsigned NumOutputs,

diff  --git a/clang/lib/Sema/Sema.cpp b/clang/lib/Sema/Sema.cpp
index 9af247e0ab4fb..8fd4c680d3bff 100644
--- a/clang/lib/Sema/Sema.cpp
+++ b/clang/lib/Sema/Sema.cpp
@@ -1949,10 +1949,9 @@ static void checkEscapingByref(VarDecl *VD, Sema &S) {
   SourceLocation Loc = VD->getLocation();
   Expr *VarRef =
       new (S.Context) DeclRefExpr(S.Context, VD, false, T, VK_LValue, Loc);
-  ExprResult Result = S.PerformCopyInitialization(
-      InitializedEntity::InitializeBlock(Loc, T, false), SourceLocation(),
-      VarRef);
-
+  ExprResult Result = S.PerformMoveOrCopyInitialization(
+      InitializedEntity::InitializeBlock(Loc, T, false), VD, VD->getType(),
+      VarRef, /*AllowNRVO=*/true);
   if (!Result.isInvalid()) {
     Result = S.MaybeCreateExprWithCleanups(Result);
     Expr *Init = Result.getAs<Expr>();

diff  --git a/clang/lib/Sema/SemaCoroutine.cpp b/clang/lib/Sema/SemaCoroutine.cpp
index 187e7a0516d10..67c64782a0e82 100644
--- a/clang/lib/Sema/SemaCoroutine.cpp
+++ b/clang/lib/Sema/SemaCoroutine.cpp
@@ -995,13 +995,17 @@ StmtResult Sema::BuildCoreturnStmt(SourceLocation Loc, Expr *E,
   }
 
   // Move the return value if we can
-  NamedReturnInfo NRInfo = getNamedReturnInfo(E, /*ForceCXX20=*/true);
-  if (NRInfo.isMoveEligible()) {
-    InitializedEntity Entity = InitializedEntity::InitializeResult(
-        Loc, E->getType(), NRInfo.Candidate);
-    ExprResult MoveResult = PerformMoveOrCopyInitialization(Entity, NRInfo, E);
-    if (MoveResult.get())
-      E = MoveResult.get();
+  if (E) {
+    const VarDecl *NRVOCandidate = this->getCopyElisionCandidate(
+        E->getType(), E, CES_ImplicitlyMovableCXX20);
+    if (NRVOCandidate) {
+      InitializedEntity Entity =
+          InitializedEntity::InitializeResult(Loc, E->getType(), NRVOCandidate);
+      ExprResult MoveResult = this->PerformMoveOrCopyInitialization(
+          Entity, NRVOCandidate, E->getType(), E);
+      if (MoveResult.get())
+        E = MoveResult.get();
+    }
   }
 
   // FIXME: If the operand is a reference to a variable that's about to go out
@@ -1566,7 +1570,7 @@ bool CoroutineStmtBuilder::makeGroDeclAndReturnStmt() {
     // Trigger a nice error message.
     InitializedEntity Entity =
         InitializedEntity::InitializeResult(Loc, FnRetType, false);
-    S.PerformCopyInitialization(Entity, SourceLocation(), ReturnValue);
+    S.PerformMoveOrCopyInitialization(Entity, nullptr, FnRetType, ReturnValue);
     noteMemberDeclaredHere(S, ReturnValue, Fn);
     return false;
   }
@@ -1582,8 +1586,8 @@ bool CoroutineStmtBuilder::makeGroDeclAndReturnStmt() {
     return false;
 
   InitializedEntity Entity = InitializedEntity::InitializeVariable(GroDecl);
-  ExprResult Res =
-      S.PerformCopyInitialization(Entity, SourceLocation(), ReturnValue);
+  ExprResult Res = S.PerformMoveOrCopyInitialization(Entity, nullptr, GroType,
+                                                     this->ReturnValue);
   if (Res.isInvalid())
     return false;
 

diff  --git a/clang/lib/Sema/SemaExprCXX.cpp b/clang/lib/Sema/SemaExprCXX.cpp
index 784da78890911..2f02cb41212ca 100644
--- a/clang/lib/Sema/SemaExprCXX.cpp
+++ b/clang/lib/Sema/SemaExprCXX.cpp
@@ -873,13 +873,15 @@ ExprResult Sema::BuildCXXThrow(SourceLocation OpLoc, Expr *Ex,
     //       operation from the operand to the exception object (15.1) can be
     //       omitted by constructing the automatic object directly into the
     //       exception object
-    NamedReturnInfo NRInfo =
-        IsThrownVarInScope ? getNamedReturnInfo(Ex) : NamedReturnInfo();
+    const VarDecl *NRVOVariable = nullptr;
+    if (IsThrownVarInScope)
+      NRVOVariable = getCopyElisionCandidate(QualType(), Ex, CES_Strict);
 
     InitializedEntity Entity = InitializedEntity::InitializeException(
         OpLoc, ExceptionObjectTy,
-        /*NRVO=*/NRInfo.isCopyElidable());
-    ExprResult Res = PerformMoveOrCopyInitialization(Entity, NRInfo, Ex);
+        /*NRVO=*/NRVOVariable != nullptr);
+    ExprResult Res = PerformMoveOrCopyInitialization(
+        Entity, NRVOVariable, QualType(), Ex, IsThrownVarInScope);
     if (Res.isInvalid())
       return ExprError();
     Ex = Res.get();

diff  --git a/clang/lib/Sema/SemaStmt.cpp b/clang/lib/Sema/SemaStmt.cpp
index 35d29b8f12dd0..da87d01de8e89 100644
--- a/clang/lib/Sema/SemaStmt.cpp
+++ b/clang/lib/Sema/SemaStmt.cpp
@@ -3307,153 +3307,99 @@ Sema::ActOnBreakStmt(SourceLocation BreakLoc, Scope *CurScope) {
   return new (Context) BreakStmt(BreakLoc);
 }
 
-/// Determine whether the given expression might be move-eligible or
-/// copy-elidable in either a (co_)return statement or throw expression,
-/// without considering function return type, if applicable.
+/// Determine whether the given expression is a candidate for
+/// copy elision in either a return statement or a throw expression.
 ///
-/// \param E The expression being returned from the function or block,
-/// being thrown, or being co_returned from a coroutine.
+/// \param ReturnType If we're determining the copy elision candidate for
+/// a return statement, this is the return type of the function. If we're
+/// determining the copy elision candidate for a throw expression, this will
+/// be a NULL type.
 ///
-/// \param ForceCXX20 Overrides detection of current language mode
-/// and uses the rules for C++20.
+/// \param E The expression being returned from the function or block, or
+/// being thrown.
 ///
-/// \returns An aggregate which contains the Candidate and isMoveEligible
-/// and isCopyElidable methods. If Candidate is non-null, it means
-/// isMoveEligible() would be true under the most permissive language standard.
-Sema::NamedReturnInfo Sema::getNamedReturnInfo(const Expr *E, bool ForceCXX20) {
-  if (!E)
-    return NamedReturnInfo();
+/// \param CESK Whether we allow function parameters or
+/// id-expressions that could be moved out of the function to be considered NRVO
+/// candidates. C++ prohibits these for NRVO itself, but we re-use this logic to
+/// determine whether we should try to move as part of a return or throw (which
+/// does allow function parameters).
+///
+/// \returns The NRVO candidate variable, if the return statement may use the
+/// NRVO, or NULL if there is no such candidate.
+VarDecl *Sema::getCopyElisionCandidate(QualType ReturnType, Expr *E,
+                                       CopyElisionSemanticsKind CESK) {
   // - in a return statement in a function [where] ...
   // ... the expression is the name of a non-volatile automatic object ...
-  const auto *DR = dyn_cast<DeclRefExpr>(E->IgnoreParens());
+  DeclRefExpr *DR = dyn_cast<DeclRefExpr>(E->IgnoreParens());
   if (!DR || DR->refersToEnclosingVariableOrCapture())
-    return NamedReturnInfo();
-  const auto *VD = dyn_cast<VarDecl>(DR->getDecl());
+    return nullptr;
+  VarDecl *VD = dyn_cast<VarDecl>(DR->getDecl());
   if (!VD)
-    return NamedReturnInfo();
-  return getNamedReturnInfo(VD, ForceCXX20);
-}
+    return nullptr;
 
-/// Updates the status in the given NamedReturnInfo object to disallow
-/// copy elision, and optionally also implicit move.
-///
-/// \param Info The NamedReturnInfo object to update.
-///
-/// \param CanMove If true, disallow only copy elision.
-/// If false, also disallow implcit move.
-static void disallowNRVO(Sema::NamedReturnInfo &Info, bool CanMove) {
-  Info.S = std::min(Info.S, CanMove ? Sema::NamedReturnInfo::MoveEligible
-                                    : Sema::NamedReturnInfo::None);
+  if (isCopyElisionCandidate(ReturnType, VD, CESK))
+    return VD;
+  return nullptr;
 }
 
-/// Determine whether the given NRVO candidate variable is move-eligible or
-/// copy-elidable, without considering function return type.
-///
-/// \param VD The NRVO candidate variable.
-///
-/// \param ForceCXX20 Overrides detection of current language mode
-/// and uses the rules for C++20.
-///
-/// \returns An aggregate which contains the Candidate and isMoveEligible
-/// and isCopyElidable methods. If Candidate is non-null, it means
-/// isMoveEligible() would be true under the most permissive language standard.
-Sema::NamedReturnInfo Sema::getNamedReturnInfo(const VarDecl *VD,
-                                               bool ForceCXX20) {
-  bool hasCXX11 = getLangOpts().CPlusPlus11 || ForceCXX20;
-  bool hasCXX20 = getLangOpts().CPlusPlus20 || ForceCXX20;
-  NamedReturnInfo Info{VD, NamedReturnInfo::MoveEligibleAndCopyElidable};
-
-  // C++20 [class.copy.elision]p3:
+bool Sema::isCopyElisionCandidate(QualType ReturnType, const VarDecl *VD,
+                                  CopyElisionSemanticsKind CESK) {
+  QualType VDType = VD->getType();
   // - in a return statement in a function with ...
-  // (other than a function ... parameter)
-  if (VD->getKind() == Decl::ParmVar)
-    disallowNRVO(Info, hasCXX11);
-  else if (VD->getKind() != Decl::Var)
-    return NamedReturnInfo();
+  // ... a class return type ...
+  if (!ReturnType.isNull() && !ReturnType->isDependentType()) {
+    if (!ReturnType->isRecordType())
+      return false;
+    // ... the same cv-unqualified type as the function return type ...
+    // When considering moving this expression out, allow dissimilar types.
+    if (!(CESK & CES_AllowDifferentTypes) && !VDType->isDependentType() &&
+        !Context.hasSameUnqualifiedType(ReturnType, VDType))
+      return false;
+  }
 
-  // (other than ... a catch-clause parameter)
-  if (VD->isExceptionVariable())
-    disallowNRVO(Info, hasCXX20);
+  // ...object (other than a function or catch-clause parameter)...
+  if (VD->getKind() != Decl::Var &&
+      !((CESK & CES_AllowParameters) && VD->getKind() == Decl::ParmVar))
+    return false;
+  if (!(CESK & CES_AllowExceptionVariables) && VD->isExceptionVariable())
+    return false;
 
   // ...automatic...
-  if (!VD->hasLocalStorage())
-    return NamedReturnInfo();
+  if (!VD->hasLocalStorage()) return false;
 
-  // We don't want to implicitly move out of a __block variable during a return
-  // because we cannot assume the variable will no longer be used.
+  // Return false if VD is a __block variable. We don't want to implicitly move
+  // out of a __block variable during a return because we cannot assume the
+  // variable will no longer be used.
   if (VD->hasAttr<BlocksAttr>())
-    return NamedReturnInfo();
+    return false;
 
-  QualType VDType = VD->getType();
   if (VDType->isObjectType()) {
     // C++17 [class.copy.elision]p3:
     // ...non-volatile automatic object...
     if (VDType.isVolatileQualified())
-      return NamedReturnInfo();
+      return false;
   } else if (VDType->isRValueReferenceType()) {
     // C++20 [class.copy.elision]p3:
-    // ...either a non-volatile object or an rvalue reference to a non-volatile
-    // object type...
+    // ...either a non-volatile object or an rvalue reference to a non-volatile object type...
+    if (!(CESK & CES_AllowRValueReferenceType))
+      return false;
     QualType VDReferencedType = VDType.getNonReferenceType();
-    if (VDReferencedType.isVolatileQualified() ||
-        !VDReferencedType->isObjectType())
-      return NamedReturnInfo();
-    disallowNRVO(Info, hasCXX20);
+    if (VDReferencedType.isVolatileQualified() || !VDReferencedType->isObjectType())
+      return false;
   } else {
-    return NamedReturnInfo();
+    return false;
   }
 
+  if (CESK & CES_AllowDifferentTypes)
+    return true;
+
   // Variables with higher required alignment than their type's ABI
   // alignment cannot use NRVO.
   if (!VDType->isDependentType() && VD->hasAttr<AlignedAttr>() &&
       Context.getDeclAlign(VD) > Context.getTypeAlignInChars(VDType))
-    disallowNRVO(Info, hasCXX11);
-
-  return Info;
-}
-
-/// Updates given NamedReturnInfo's move-eligible and
-/// copy-elidable statuses, considering the function
-/// return type criteria as applicable to return statements.
-///
-/// \param Info The NamedReturnInfo object to update.
-///
-/// \param ReturnType This is the return type of the function.
-/// \returns The copy elision candidate, in case the initial return expression
-/// was copy elidable, or nullptr otherwise.
-const VarDecl *Sema::getCopyElisionCandidate(NamedReturnInfo &Info,
-                                             QualType ReturnType) {
-  if (!Info.Candidate)
-    return nullptr;
-
-  auto invalidNRVO = [&] {
-    Info = NamedReturnInfo();
-    return nullptr;
-  };
-
-  // If we got a non-deduced auto ReturnType, we are in a dependent context and
-  // there is no point in allowing copy elision since we won't have it deduced
-  // by the point the VardDecl is instantiated, which is the last chance we have
-  // of deciding if the candidate is really copy elidable.
-  if ((ReturnType->getTypeClass() == Type::TypeClass::Auto &&
-       ReturnType->isCanonicalUnqualified()) ||
-      ReturnType->isSpecificBuiltinType(BuiltinType::Dependent))
-    return invalidNRVO();
-
-  if (!ReturnType->isDependentType()) {
-    // - in a return statement in a function with ...
-    // ... a class return type ...
-    if (!ReturnType->isRecordType())
-      return invalidNRVO();
+    return false;
 
-    QualType VDType = Info.Candidate->getType();
-    // ... the same cv-unqualified type as the function return type ...
-    // When considering moving this expression out, allow dissimilar types.
-    if (!VDType->isDependentType() &&
-        !Context.hasSameUnqualifiedType(ReturnType, VDType))
-      disallowNRVO(Info, getLangOpts().CPlusPlus11);
-  }
-  return Info.isCopyElidable() ? Info.Candidate : nullptr;
+  return true;
 }
 
 /// Try to perform the initialization of a potentially-movable value,
@@ -3478,7 +3424,8 @@ const VarDecl *Sema::getCopyElisionCandidate(NamedReturnInfo &Info,
 /// the selected constructor/operator doesn't match the additional criteria, we
 /// need to do the second overload resolution.
 static bool TryMoveInitialization(Sema &S, const InitializedEntity &Entity,
-                                  const VarDecl *NRVOCandidate, Expr *&Value,
+                                  const VarDecl *NRVOCandidate,
+                                  QualType ResultType, Expr *&Value,
                                   bool ConvertingConstructorsOnly,
                                   bool IsDiagnosticsCheck, ExprResult &Res) {
   ImplicitCastExpr AsRvalue(ImplicitCastExpr::OnStack, Value->getType(),
@@ -3561,41 +3508,63 @@ static bool TryMoveInitialization(Sema &S, const InitializedEntity &Entity,
 /// This routine implements C++20 [class.copy.elision]p3, which attempts to
 /// treat returned lvalues as rvalues in certain cases (to prefer move
 /// construction), then falls back to treating them as lvalues if that failed.
-ExprResult
-Sema::PerformMoveOrCopyInitialization(const InitializedEntity &Entity,
-                                      const NamedReturnInfo &NRInfo,
-                                      Expr *Value) {
-
-  if (NRInfo.Candidate) {
-    if (NRInfo.isMoveEligible()) {
-      ExprResult Res;
-      if (!TryMoveInitialization(*this, Entity, NRInfo.Candidate, Value,
-                                 !getLangOpts().CPlusPlus20, false, Res))
-        return Res;
+ExprResult Sema::PerformMoveOrCopyInitialization(
+    const InitializedEntity &Entity, const VarDecl *NRVOCandidate,
+    QualType ResultType, Expr *Value, bool AllowNRVO) {
+  ExprResult Res = ExprError();
+  bool NeedSecondOverloadResolution = true;
+
+  if (AllowNRVO) {
+    CopyElisionSemanticsKind CESK = CES_Strict;
+    if (getLangOpts().CPlusPlus20) {
+      CESK = CES_ImplicitlyMovableCXX20;
+    } else if (getLangOpts().CPlusPlus11) {
+      CESK = CES_ImplicitlyMovableCXX11CXX14CXX17;
+    }
+
+    if (!NRVOCandidate) {
+      NRVOCandidate = getCopyElisionCandidate(ResultType, Value, CESK);
+    }
+
+    if (NRVOCandidate) {
+      NeedSecondOverloadResolution =
+          TryMoveInitialization(*this, Entity, NRVOCandidate, ResultType, Value,
+                                !getLangOpts().CPlusPlus20, false, Res);
     }
-    if (!getDiagnostics().isIgnored(diag::warn_return_std_move,
+
+    if (!getLangOpts().CPlusPlus20 && NeedSecondOverloadResolution &&
+        !getDiagnostics().isIgnored(diag::warn_return_std_move,
                                     Value->getExprLoc())) {
-      QualType QT = NRInfo.Candidate->getType();
-      if (QT.getNonReferenceType().getUnqualifiedType().isTriviallyCopyableType(
-              Context)) {
-        // Adding 'std::move' around a trivially copyable variable is probably
-        // pointless. Don't suggest it.
-      } else {
-        ExprResult FakeRes = ExprError();
-        Expr *FakeValue = Value;
-        TryMoveInitialization(*this, Entity, NRInfo.Candidate, FakeValue, false,
-                              true, FakeRes);
-        if (!FakeRes.isInvalid()) {
-          bool IsThrow = (Entity.getKind() == InitializedEntity::EK_Exception);
-          SmallString<32> Str;
-          Str += "std::move(";
-          Str += NRInfo.Candidate->getDeclName().getAsString();
-          Str += ")";
-          Diag(Value->getExprLoc(), diag::warn_return_std_move)
-              << Value->getSourceRange() << NRInfo.Candidate->getDeclName()
-              << IsThrow;
-          Diag(Value->getExprLoc(), diag::note_add_std_move)
-              << FixItHint::CreateReplacement(Value->getSourceRange(), Str);
+      const VarDecl *FakeNRVOCandidate = getCopyElisionCandidate(
+          QualType(), Value, CES_ImplicitlyMovableCXX20);
+      if (FakeNRVOCandidate) {
+        QualType QT = FakeNRVOCandidate->getType();
+        if (QT->isLValueReferenceType()) {
+          // Adding 'std::move' around an lvalue reference variable's name is
+          // dangerous. Don't suggest it.
+        } else if (QT.getNonReferenceType()
+                       .getUnqualifiedType()
+                       .isTriviallyCopyableType(Context)) {
+          // Adding 'std::move' around a trivially copyable variable is probably
+          // pointless. Don't suggest it.
+        } else {
+          ExprResult FakeRes = ExprError();
+          Expr *FakeValue = Value;
+          TryMoveInitialization(*this, Entity, FakeNRVOCandidate, ResultType,
+                                FakeValue, false, true, FakeRes);
+          if (!FakeRes.isInvalid()) {
+            bool IsThrow =
+                (Entity.getKind() == InitializedEntity::EK_Exception);
+            SmallString<32> Str;
+            Str += "std::move(";
+            Str += FakeNRVOCandidate->getDeclName().getAsString();
+            Str += ")";
+            Diag(Value->getExprLoc(), diag::warn_return_std_move)
+                << Value->getSourceRange()
+                << FakeNRVOCandidate->getDeclName() << IsThrow;
+            Diag(Value->getExprLoc(), diag::note_add_std_move)
+                << FixItHint::CreateReplacement(Value->getSourceRange(), Str);
+          }
         }
       }
     }
@@ -3604,7 +3573,10 @@ Sema::PerformMoveOrCopyInitialization(const InitializedEntity &Entity,
   // Either we didn't meet the criteria for treating an lvalue as an rvalue,
   // above, or overload resolution failed. Either way, we need to try
   // (again) now with the return value expression as written.
-  return PerformCopyInitialization(Entity, SourceLocation(), Value);
+  if (NeedSecondOverloadResolution)
+    Res = PerformCopyInitialization(Entity, SourceLocation(), Value);
+
+  return Res;
 }
 
 /// Determine whether the declared return type of the specified function
@@ -3618,9 +3590,8 @@ static bool hasDeducedReturnType(FunctionDecl *FD) {
 /// ActOnCapScopeReturnStmt - Utility routine to type-check return statements
 /// for capturing scopes.
 ///
-StmtResult Sema::ActOnCapScopeReturnStmt(SourceLocation ReturnLoc,
-                                         Expr *RetValExp,
-                                         NamedReturnInfo &NRInfo) {
+StmtResult
+Sema::ActOnCapScopeReturnStmt(SourceLocation ReturnLoc, Expr *RetValExp) {
   // If this is the first return we've seen, infer the return type.
   // [expr.prim.lambda]p4 in C++11; block literals follow the same rules.
   CapturingScopeInfo *CurCap = cast<CapturingScopeInfo>(getCurFunction());
@@ -3699,7 +3670,7 @@ StmtResult Sema::ActOnCapScopeReturnStmt(SourceLocation ReturnLoc,
     if (CurCap->ReturnType.isNull())
       CurCap->ReturnType = FnRetType;
   }
-  const VarDecl *NRVOCandidate = getCopyElisionCandidate(NRInfo, FnRetType);
+  assert(!FnRetType.isNull());
 
   if (auto *CurBlock = dyn_cast<BlockScopeInfo>(CurCap)) {
     if (CurBlock->FunctionType->castAs<FunctionType>()->getNoReturnAttr()) {
@@ -3722,6 +3693,7 @@ StmtResult Sema::ActOnCapScopeReturnStmt(SourceLocation ReturnLoc,
   // Otherwise, verify that this result type matches the previous one.  We are
   // pickier with blocks than for normal functions because we don't have GCC
   // compatibility to worry about here.
+  const VarDecl *NRVOCandidate = nullptr;
   if (FnRetType->isDependentType()) {
     // Delay processing for now.  TODO: there are lots of dependent
     // types we can conclusively prove aren't void.
@@ -3749,15 +3721,20 @@ StmtResult Sema::ActOnCapScopeReturnStmt(SourceLocation ReturnLoc,
 
     // In C++ the return statement is handled via a copy initialization.
     // the C version of which boils down to CheckSingleAssignmentConstraints.
-    InitializedEntity Entity = InitializedEntity::InitializeResult(
-        ReturnLoc, FnRetType, NRVOCandidate != nullptr);
-    ExprResult Res = PerformMoveOrCopyInitialization(Entity, NRInfo, RetValExp);
+    NRVOCandidate = getCopyElisionCandidate(FnRetType, RetValExp, CES_Strict);
+    InitializedEntity Entity = InitializedEntity::InitializeResult(ReturnLoc,
+                                                                   FnRetType,
+                                                      NRVOCandidate != nullptr);
+    ExprResult Res = PerformMoveOrCopyInitialization(Entity, NRVOCandidate,
+                                                     FnRetType, RetValExp);
     if (Res.isInvalid()) {
       // FIXME: Cleanup temporaries here, anyway?
       return StmtError();
     }
     RetValExp = Res.get();
     CheckReturnValExpr(RetValExp, FnRetType, ReturnLoc);
+  } else {
+    NRVOCandidate = getCopyElisionCandidate(FnRetType, RetValExp, CES_Strict);
   }
 
   if (RetValExp) {
@@ -3966,10 +3943,8 @@ StmtResult Sema::BuildReturnStmt(SourceLocation ReturnLoc, Expr *RetValExp) {
   if (RetValExp && DiagnoseUnexpandedParameterPack(RetValExp))
     return StmtError();
 
-  NamedReturnInfo NRInfo = getNamedReturnInfo(RetValExp);
-
   if (isa<CapturingScopeInfo>(getCurFunction()))
-    return ActOnCapScopeReturnStmt(ReturnLoc, RetValExp, NRInfo);
+    return ActOnCapScopeReturnStmt(ReturnLoc, RetValExp);
 
   QualType FnRetType;
   QualType RelatedRetType;
@@ -4041,7 +4016,6 @@ StmtResult Sema::BuildReturnStmt(SourceLocation ReturnLoc, Expr *RetValExp) {
       }
     }
   }
-  const VarDecl *NRVOCandidate = getCopyElisionCandidate(NRInfo, FnRetType);
 
   bool HasDependentReturnType = FnRetType->isDependentType();
 
@@ -4148,6 +4122,8 @@ StmtResult Sema::BuildReturnStmt(SourceLocation ReturnLoc, Expr *RetValExp) {
                                 /* NRVOCandidate=*/nullptr);
   } else {
     assert(RetValExp || HasDependentReturnType);
+    const VarDecl *NRVOCandidate = nullptr;
+
     QualType RetType = RelatedRetType.isNull() ? FnRetType : RelatedRetType;
 
     // C99 6.8.6.4p3(136): The return statement is not an assignment. The
@@ -4156,12 +4132,15 @@ StmtResult Sema::BuildReturnStmt(SourceLocation ReturnLoc, Expr *RetValExp) {
 
     // In C++ the return statement is handled via a copy initialization,
     // the C version of which boils down to CheckSingleAssignmentConstraints.
+    if (RetValExp)
+      NRVOCandidate = getCopyElisionCandidate(FnRetType, RetValExp, CES_Strict);
     if (!HasDependentReturnType && !RetValExp->isTypeDependent()) {
       // we have a non-void function with an expression, continue checking
-      InitializedEntity Entity = InitializedEntity::InitializeResult(
-          ReturnLoc, RetType, NRVOCandidate != nullptr);
-      ExprResult Res =
-          PerformMoveOrCopyInitialization(Entity, NRInfo, RetValExp);
+      InitializedEntity Entity = InitializedEntity::InitializeResult(ReturnLoc,
+                                                                     RetType,
+                                                      NRVOCandidate != nullptr);
+      ExprResult Res = PerformMoveOrCopyInitialization(Entity, NRVOCandidate,
+                                                       RetType, RetValExp);
       if (Res.isInvalid()) {
         // FIXME: Clean up temporaries here anyway?
         return StmtError();

diff  --git a/clang/lib/Sema/SemaTemplateInstantiateDecl.cpp b/clang/lib/Sema/SemaTemplateInstantiateDecl.cpp
index 2f9b56fed5864..8de09def4d52a 100644
--- a/clang/lib/Sema/SemaTemplateInstantiateDecl.cpp
+++ b/clang/lib/Sema/SemaTemplateInstantiateDecl.cpp
@@ -23,7 +23,6 @@
 #include "clang/Basic/TargetInfo.h"
 #include "clang/Sema/Initialization.h"
 #include "clang/Sema/Lookup.h"
-#include "clang/Sema/ScopeInfo.h"
 #include "clang/Sema/SemaInternal.h"
 #include "clang/Sema/Template.h"
 #include "clang/Sema/TemplateInstCallback.h"
@@ -1086,30 +1085,11 @@ Decl *TemplateDeclInstantiator::VisitVarDecl(VarDecl *D,
 
   SemaRef.BuildVariableInstantiation(Var, D, TemplateArgs, LateAttrs, Owner,
                                      StartingScope, InstantiatingVarTemplate);
+
   if (D->isNRVOVariable()) {
-    QualType FT;
-    if (auto *F = dyn_cast<FunctionDecl>(DC))
-      FT = F->getType();
-    else if (isa<BlockDecl>(DC))
-      FT = SemaRef.getCurBlock()->FunctionType;
-    else
-      llvm_unreachable("Unknown context type");
-
-    // This is the last chance we have of checking copy elision eligibility
-    // for functions in depdendent contexts. The sema actions for building
-    // the return statement during template instantiation will have no effect
-    // regarding copy elision, since NRVO propagation runs on the scope exit
-    // actions, and these are not run on instantiation.
-    // This might run through some VarDecls which were returned from non-taken
-    // 'if constexpr' branches, and these will end up being constructed on the
-    // return slot even if they will never be returned, as a sort of accidental
-    // 'optimization'. Notably, functions with 'auto' return types won't have it
-    // deduced by this point. Coupled with the limitation described
-    // previously, this makes it very hard to support copy elision for these.
-    Sema::NamedReturnInfo Info = SemaRef.getNamedReturnInfo(Var);
-    bool NRVO = SemaRef.getCopyElisionCandidate(
-                    Info, cast<FunctionType>(FT)->getReturnType()) != nullptr;
-    Var->setNRVOVariable(NRVO);
+    QualType ReturnType = cast<FunctionDecl>(DC)->getReturnType();
+    if (SemaRef.isCopyElisionCandidate(ReturnType, Var, Sema::CES_Strict))
+      Var->setNRVOVariable(true);
   }
 
   Var->setImplicit(D->isImplicit());

diff  --git a/clang/test/CodeGen/nrvo-tracking.cpp b/clang/test/CodeGen/nrvo-tracking.cpp
index dd97469368e77..db5aa198621ae 100644
--- a/clang/test/CodeGen/nrvo-tracking.cpp
+++ b/clang/test/CodeGen/nrvo-tracking.cpp
@@ -29,6 +29,8 @@ L(2, t, X&);
 
 // CHECK-LABEL: define{{.*}} void @_Z2l3v
 // CHECK:       call {{.*}} @_ZN1XC1Ev
+// CHECK-NEXT:  call {{.*}} @_ZN1XC1EOS_
+// CHECK-NEXT:  call void @llvm.lifetime.end
 // CHECK-NEXT:  call void @llvm.lifetime.end
 // CHECK-NEXT:  ret void
 L(3, t, T);
@@ -150,11 +152,7 @@ F(8, (t), decltype(auto));
   }; }()();                                    \
 }
 
-// CHECK-LABEL: define{{.*}} void @_Z2b1v
-// CHECK:       call {{.*}} @_ZN1XC1Ev
-// CHECK-NEXT:  call void @llvm.lifetime.end
-// CHECK-NEXT:  ret void
-B(1, X);
+//B(1, X); // Uncomment this line at your own peril ;)
 
 // CHECK-LABEL: define{{.*}} void @_Z2b2v
 // CHECK:       call {{.*}} @_ZN1XC1Ev
@@ -166,6 +164,8 @@ B(2, X&);
 
 // CHECK-LABEL: define{{.*}} void @_Z2b3v
 // CHECK:       call {{.*}} @_ZN1XC1Ev
+// CHECK-NEXT:  call {{.*}} @_ZN1XC1EOS_
+// CHECK-NEXT:  call void @llvm.lifetime.end
 // CHECK-NEXT:  call void @llvm.lifetime.end
 // CHECK-NEXT:  ret void
 B(3, T);


        


More information about the cfe-commits mailing list