Author: Matheus Izvekov Date: 2021-06-12T16:43:32+02:00 New Revision: 1e50c3d785f4563873ab1ce86559f2a1285b5678
URL: https://github.com/llvm/llvm-project/commit/1e50c3d785f4563873ab1ce86559f2a1285b5678 DIFF: https://github.com/llvm/llvm-project/commit/1e50c3d785f4563873ab1ce86559f2a1285b5678.diff LOG: [clang] NRVO: Improvements and handling of more cases. This expands NRVO propagation for more cases: Parse analysis improvement: * Lambdas and Blocks with dependent return type can have their variables marked as NRVO Candidates. Variable instantiation improvements: * Fixes crash when instantiating NRVO variables in Blocks. * Functions, Lambdas, and Blocks which have auto return type have their variables' NRVO status propagated. For Blocks with non-auto return type, as a limitation, this propagation does not consider the actual return type. This also implements exclusion of VarDecls which are references to dependent types. Signed-off-by: Matheus Izvekov <mizve...@gmail.com> Reviewed By: Quuxplusone Differential Revision: https://reviews.llvm.org/D99696 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 6ade9d7691266..f7ec89a33e00c 100644 --- a/clang/include/clang/Sema/Sema.h +++ b/clang/include/clang/Sema/Sema.h @@ -3455,12 +3455,6 @@ 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); @@ -4760,28 +4754,30 @@ class Sema final { SourceLocation Loc, unsigned NumParams); - 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), + 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; } }; + NamedReturnInfo getNamedReturnInfo(const Expr *E, bool ForceCXX20 = false); + NamedReturnInfo getNamedReturnInfo(const VarDecl *VD, + bool ForceCXX20 = false); + const VarDecl *getCopyElisionCandidate(NamedReturnInfo &Info, + QualType ReturnType); - VarDecl *getCopyElisionCandidate(QualType ReturnType, Expr *E, - CopyElisionSemanticsKind CESK); - bool isCopyElisionCandidate(QualType ReturnType, const VarDecl *VD, - CopyElisionSemanticsKind CESK); + ExprResult PerformMoveOrCopyInitialization(const InitializedEntity &Entity, + const NamedReturnInfo &NRInfo, + Expr *Value); StmtResult ActOnReturnStmt(SourceLocation ReturnLoc, Expr *RetValExp, Scope *CurScope); StmtResult BuildReturnStmt(SourceLocation ReturnLoc, Expr *RetValExp); - StmtResult ActOnCapScopeReturnStmt(SourceLocation ReturnLoc, Expr *RetValExp); + StmtResult ActOnCapScopeReturnStmt(SourceLocation ReturnLoc, Expr *RetValExp, + NamedReturnInfo &NRInfo); 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 850c189cc51a3..cdde9a83a6d02 100644 --- a/clang/lib/Sema/Sema.cpp +++ b/clang/lib/Sema/Sema.cpp @@ -1962,9 +1962,10 @@ 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.PerformMoveOrCopyInitialization( - InitializedEntity::InitializeBlock(Loc, T, false), VD, VD->getType(), - VarRef, /*AllowNRVO=*/true); + ExprResult Result = S.PerformCopyInitialization( + InitializedEntity::InitializeBlock(Loc, T, false), SourceLocation(), + VarRef); + 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 67c64782a0e82..187e7a0516d10 100644 --- a/clang/lib/Sema/SemaCoroutine.cpp +++ b/clang/lib/Sema/SemaCoroutine.cpp @@ -995,17 +995,13 @@ StmtResult Sema::BuildCoreturnStmt(SourceLocation Loc, Expr *E, } // Move the return value if we can - 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(); - } + 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(); } // FIXME: If the operand is a reference to a variable that's about to go out @@ -1570,7 +1566,7 @@ bool CoroutineStmtBuilder::makeGroDeclAndReturnStmt() { // Trigger a nice error message. InitializedEntity Entity = InitializedEntity::InitializeResult(Loc, FnRetType, false); - S.PerformMoveOrCopyInitialization(Entity, nullptr, FnRetType, ReturnValue); + S.PerformCopyInitialization(Entity, SourceLocation(), ReturnValue); noteMemberDeclaredHere(S, ReturnValue, Fn); return false; } @@ -1586,8 +1582,8 @@ bool CoroutineStmtBuilder::makeGroDeclAndReturnStmt() { return false; InitializedEntity Entity = InitializedEntity::InitializeVariable(GroDecl); - ExprResult Res = S.PerformMoveOrCopyInitialization(Entity, nullptr, GroType, - this->ReturnValue); + ExprResult Res = + S.PerformCopyInitialization(Entity, SourceLocation(), ReturnValue); if (Res.isInvalid()) return false; diff --git a/clang/lib/Sema/SemaExprCXX.cpp b/clang/lib/Sema/SemaExprCXX.cpp index 81a2e0cb775d0..e9ab2bcd67c2e 100644 --- a/clang/lib/Sema/SemaExprCXX.cpp +++ b/clang/lib/Sema/SemaExprCXX.cpp @@ -873,15 +873,13 @@ 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 - const VarDecl *NRVOVariable = nullptr; - if (IsThrownVarInScope) - NRVOVariable = getCopyElisionCandidate(QualType(), Ex, CES_Strict); + NamedReturnInfo NRInfo = + IsThrownVarInScope ? getNamedReturnInfo(Ex) : NamedReturnInfo(); InitializedEntity Entity = InitializedEntity::InitializeException( OpLoc, ExceptionObjectTy, - /*NRVO=*/NRVOVariable != nullptr); - ExprResult Res = PerformMoveOrCopyInitialization( - Entity, NRVOVariable, QualType(), Ex, IsThrownVarInScope); + /*NRVO=*/NRInfo.isCopyElidable()); + ExprResult Res = PerformMoveOrCopyInitialization(Entity, NRInfo, Ex); if (Res.isInvalid()) return ExprError(); Ex = Res.get(); diff --git a/clang/lib/Sema/SemaStmt.cpp b/clang/lib/Sema/SemaStmt.cpp index 8bb2e0e8c8633..136e39198c728 100644 --- a/clang/lib/Sema/SemaStmt.cpp +++ b/clang/lib/Sema/SemaStmt.cpp @@ -3307,99 +3307,153 @@ Sema::ActOnBreakStmt(SourceLocation BreakLoc, Scope *CurScope) { return new (Context) BreakStmt(BreakLoc); } -/// Determine whether the given expression is a candidate for -/// copy elision in either a return statement or a throw expression. +/// 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. /// -/// \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 E The expression being returned from the function or block, +/// being thrown, or being co_returned from a coroutine. /// -/// \param E The expression being returned from the function or block, or -/// being thrown. +/// \param ForceCXX20 Overrides detection of current language mode +/// and uses the rules for C++20. /// -/// \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) { +/// \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(); // - in a return statement in a function [where] ... // ... the expression is the name of a non-volatile automatic object ... - DeclRefExpr *DR = dyn_cast<DeclRefExpr>(E->IgnoreParens()); + const auto *DR = dyn_cast<DeclRefExpr>(E->IgnoreParens()); if (!DR || DR->refersToEnclosingVariableOrCapture()) - return nullptr; - VarDecl *VD = dyn_cast<VarDecl>(DR->getDecl()); + return NamedReturnInfo(); + const auto *VD = dyn_cast<VarDecl>(DR->getDecl()); if (!VD) - return nullptr; + return NamedReturnInfo(); + return getNamedReturnInfo(VD, ForceCXX20); +} - if (isCopyElisionCandidate(ReturnType, VD, CESK)) - return VD; - 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); } -bool Sema::isCopyElisionCandidate(QualType ReturnType, const VarDecl *VD, - CopyElisionSemanticsKind CESK) { - QualType VDType = VD->getType(); +/// 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: // - in a return statement in a function with ... - // ... 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 function ... parameter) + if (VD->getKind() == Decl::ParmVar) + disallowNRVO(Info, hasCXX11); + else if (VD->getKind() != Decl::Var) + return NamedReturnInfo(); - // ...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; + // (other than ... a catch-clause parameter) + if (VD->isExceptionVariable()) + disallowNRVO(Info, hasCXX20); // ...automatic... - if (!VD->hasLocalStorage()) return false; + if (!VD->hasLocalStorage()) + return NamedReturnInfo(); - // 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. + // 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 false; + return NamedReturnInfo(); + QualType VDType = VD->getType(); if (VDType->isObjectType()) { // C++17 [class.copy.elision]p3: // ...non-volatile automatic object... if (VDType.isVolatileQualified()) - return false; + return NamedReturnInfo(); } 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... - if (!(CESK & CES_AllowRValueReferenceType)) - return false; + // ...either a non-volatile object or an rvalue reference to a non-volatile + // object type... QualType VDReferencedType = VDType.getNonReferenceType(); - if (VDReferencedType.isVolatileQualified() || !VDReferencedType->isObjectType()) - return false; + if (VDReferencedType.isVolatileQualified() || + !VDReferencedType->isObjectType()) + return NamedReturnInfo(); + disallowNRVO(Info, hasCXX20); } else { - return false; + return NamedReturnInfo(); } - 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)) - return false; + disallowNRVO(Info, hasCXX11); - return true; + 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(); + + 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; } /// Try to perform the initialization of a potentially-movable value, @@ -3424,8 +3478,7 @@ bool Sema::isCopyElisionCandidate(QualType ReturnType, const VarDecl *VD, /// 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, - QualType ResultType, Expr *&Value, + const VarDecl *NRVOCandidate, Expr *&Value, bool ConvertingConstructorsOnly, bool IsDiagnosticsCheck, ExprResult &Res) { ImplicitCastExpr AsRvalue(ImplicitCastExpr::OnStack, Value->getType(), @@ -3508,63 +3561,41 @@ 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 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); +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; } - - if (!getLangOpts().CPlusPlus20 && NeedSecondOverloadResolution && - !getDiagnostics().isIgnored(diag::warn_return_std_move, + if (!getDiagnostics().isIgnored(diag::warn_return_std_move, Value->getExprLoc())) { - 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); - } + 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); } } } @@ -3573,10 +3604,7 @@ ExprResult Sema::PerformMoveOrCopyInitialization( // 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. - if (NeedSecondOverloadResolution) - Res = PerformCopyInitialization(Entity, SourceLocation(), Value); - - return Res; + return PerformCopyInitialization(Entity, SourceLocation(), Value); } /// Determine whether the declared return type of the specified function @@ -3590,8 +3618,9 @@ static bool hasDeducedReturnType(FunctionDecl *FD) { /// ActOnCapScopeReturnStmt - Utility routine to type-check return statements /// for capturing scopes. /// -StmtResult -Sema::ActOnCapScopeReturnStmt(SourceLocation ReturnLoc, Expr *RetValExp) { +StmtResult Sema::ActOnCapScopeReturnStmt(SourceLocation ReturnLoc, + Expr *RetValExp, + NamedReturnInfo &NRInfo) { // 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()); @@ -3670,7 +3699,7 @@ Sema::ActOnCapScopeReturnStmt(SourceLocation ReturnLoc, Expr *RetValExp) { if (CurCap->ReturnType.isNull()) CurCap->ReturnType = FnRetType; } - assert(!FnRetType.isNull()); + const VarDecl *NRVOCandidate = getCopyElisionCandidate(NRInfo, FnRetType); if (auto *CurBlock = dyn_cast<BlockScopeInfo>(CurCap)) { if (CurBlock->FunctionType->castAs<FunctionType>()->getNoReturnAttr()) { @@ -3693,7 +3722,6 @@ Sema::ActOnCapScopeReturnStmt(SourceLocation ReturnLoc, Expr *RetValExp) { // 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. @@ -3721,20 +3749,15 @@ Sema::ActOnCapScopeReturnStmt(SourceLocation ReturnLoc, Expr *RetValExp) { // In C++ the return statement is handled via a copy initialization. // the C version of which boils down to CheckSingleAssignmentConstraints. - NRVOCandidate = getCopyElisionCandidate(FnRetType, RetValExp, CES_Strict); - InitializedEntity Entity = InitializedEntity::InitializeResult(ReturnLoc, - FnRetType, - NRVOCandidate != nullptr); - ExprResult Res = PerformMoveOrCopyInitialization(Entity, NRVOCandidate, - FnRetType, RetValExp); + InitializedEntity Entity = InitializedEntity::InitializeResult( + ReturnLoc, FnRetType, NRVOCandidate != nullptr); + ExprResult Res = PerformMoveOrCopyInitialization(Entity, NRInfo, 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) { @@ -3943,8 +3966,10 @@ 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); + return ActOnCapScopeReturnStmt(ReturnLoc, RetValExp, NRInfo); QualType FnRetType; QualType RelatedRetType; @@ -4016,6 +4041,7 @@ StmtResult Sema::BuildReturnStmt(SourceLocation ReturnLoc, Expr *RetValExp) { } } } + const VarDecl *NRVOCandidate = getCopyElisionCandidate(NRInfo, FnRetType); bool HasDependentReturnType = FnRetType->isDependentType(); @@ -4122,8 +4148,6 @@ 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 @@ -4132,15 +4156,12 @@ 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, NRVOCandidate, - RetType, RetValExp); + InitializedEntity Entity = InitializedEntity::InitializeResult( + ReturnLoc, RetType, NRVOCandidate != nullptr); + ExprResult Res = + PerformMoveOrCopyInitialization(Entity, NRInfo, 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 8de09def4d52a..1327e0889c6fe 100644 --- a/clang/lib/Sema/SemaTemplateInstantiateDecl.cpp +++ b/clang/lib/Sema/SemaTemplateInstantiateDecl.cpp @@ -23,6 +23,7 @@ #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" @@ -1085,11 +1086,30 @@ Decl *TemplateDeclInstantiator::VisitVarDecl(VarDecl *D, SemaRef.BuildVariableInstantiation(Var, D, TemplateArgs, LateAttrs, Owner, StartingScope, InstantiatingVarTemplate); - if (D->isNRVOVariable()) { - QualType ReturnType = cast<FunctionDecl>(DC)->getReturnType(); - if (SemaRef.isCopyElisionCandidate(ReturnType, Var, Sema::CES_Strict)) - Var->setNRVOVariable(true); + QualType RT; + if (auto *F = dyn_cast<FunctionDecl>(DC)) + RT = F->getReturnType(); + else if (isa<BlockDecl>(DC)) + RT = cast<FunctionType>(SemaRef.getCurBlock()->FunctionType) + ->getReturnType(); + 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, RT) != nullptr; + Var->setNRVOVariable(NRVO); } Var->setImplicit(D->isImplicit()); diff --git a/clang/test/CodeGen/nrvo-tracking.cpp b/clang/test/CodeGen/nrvo-tracking.cpp index db5aa198621ae..b1aa67389e99f 100644 --- a/clang/test/CodeGen/nrvo-tracking.cpp +++ b/clang/test/CodeGen/nrvo-tracking.cpp @@ -29,8 +29,6 @@ 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); @@ -152,7 +150,11 @@ F(8, (t), decltype(auto)); }; }()(); \ } -//B(1, X); // Uncomment this line at your own peril ;) +// CHECK-LABEL: define{{.*}} void @_Z2b1v +// CHECK: call {{.*}} @_ZN1XC1Ev +// CHECK-NEXT: call void @llvm.lifetime.end +// CHECK-NEXT: ret void +B(1, X); // CHECK-LABEL: define{{.*}} void @_Z2b2v // CHECK: call {{.*}} @_ZN1XC1Ev @@ -164,8 +166,6 @@ 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); @@ -187,3 +187,24 @@ B(4, T&); B(5, ); #undef B + +// CHECK-LABEL: define{{.*}} void @_Z6f_attrv +// CHECK: call {{.*}} @_ZN1XC1Ev +// CHECK-NEXT: call void @llvm.lifetime.end +// CHECK-NEXT: ret void +template<class T = X> [[gnu::cdecl]] static inline auto tf_attr() -> X { + T t; + return t; +} +void f_attr() { auto t = tf_attr(); } + +// CHECK-LABEL: define{{.*}} void @_Z6b_attrv +// CHECK: call {{.*}} @_ZN1XC1Ev +// CHECK-NEXT: call void @llvm.lifetime.end +// CHECK-NEXT: ret void +void b_attr() { + auto t = []<class T = X>() { return ^ X () [[clang::vectorcall]] { + T t; + return t; + }; }()(); +} _______________________________________________ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
