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); _______________________________________________ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
