This warning will catch code such as:
class X {};
X foo() {
X x;
return std::move(x);
}
where NRVO could have been applied if a call to std::move was not used. This
will emit one warning per function, with one note per return statement that
needs updated.
class.cc:15:3: warning: return value optimization for function 'foo' prevented
by call to std::move on return statement [-Wpessimizing-move]
X foo() {
^
class.cc:17:7: note: remove call to std::move here to allow return value
optimization
return std::move(x);
^ ~~~~~~~~~~~~
x
1 warning generated.
http://reviews.llvm.org/D7633
Files:
include/clang/AST/Stmt.h
include/clang/Basic/DiagnosticGroups.td
include/clang/Basic/DiagnosticSemaKinds.td
include/clang/Sema/ScopeInfo.h
include/clang/Sema/Sema.h
lib/Sema/ScopeInfo.cpp
lib/Sema/SemaDecl.cpp
lib/Sema/SemaStmt.cpp
EMAIL PREFERENCES
http://reviews.llvm.org/settings/panel/emailpreferences/
Index: include/clang/AST/Stmt.h
===================================================================
--- include/clang/AST/Stmt.h
+++ include/clang/AST/Stmt.h
@@ -1344,15 +1344,16 @@
Stmt *RetExpr;
SourceLocation RetLoc;
const VarDecl *NRVOCandidate;
+ bool PessimizingMoveCandidate;
public:
ReturnStmt(SourceLocation RL)
: Stmt(ReturnStmtClass), RetExpr(nullptr), RetLoc(RL),
- NRVOCandidate(nullptr) {}
+ NRVOCandidate(nullptr), PessimizingMoveCandidate(false) {}
ReturnStmt(SourceLocation RL, Expr *E, const VarDecl *NRVOCandidate)
: Stmt(ReturnStmtClass), RetExpr((Stmt*) E), RetLoc(RL),
- NRVOCandidate(NRVOCandidate) {}
+ NRVOCandidate(NRVOCandidate), PessimizingMoveCandidate(false) {}
/// \brief Build an empty return expression.
explicit ReturnStmt(EmptyShell Empty) : Stmt(ReturnStmtClass, Empty) { }
@@ -1372,6 +1373,9 @@
const VarDecl *getNRVOCandidate() const { return NRVOCandidate; }
void setNRVOCandidate(const VarDecl *Var) { NRVOCandidate = Var; }
+ bool isPessimizingMoveCandidate() { return PessimizingMoveCandidate; }
+ void setPessimizingMoveCandidate() { PessimizingMoveCandidate = true; }
+
SourceLocation getLocStart() const LLVM_READONLY { return RetLoc; }
SourceLocation getLocEnd() const LLVM_READONLY {
return RetExpr ? RetExpr->getLocEnd() : RetLoc;
Index: include/clang/Basic/DiagnosticGroups.td
===================================================================
--- include/clang/Basic/DiagnosticGroups.td
+++ include/clang/Basic/DiagnosticGroups.td
@@ -279,6 +279,7 @@
def ExplicitInitializeCall : DiagGroup<"explicit-initialize-call">;
def Packed : DiagGroup<"packed">;
def Padded : DiagGroup<"padded">;
+def PessimizingMove : DiagGroup<"pessimizing-move">;
def PointerArith : DiagGroup<"pointer-arith">;
def PoundWarning : DiagGroup<"#warnings">;
def PoundPragmaMessage : DiagGroup<"#pragma-messages">,
Index: include/clang/Basic/DiagnosticSemaKinds.td
===================================================================
--- include/clang/Basic/DiagnosticSemaKinds.td
+++ include/clang/Basic/DiagnosticSemaKinds.td
@@ -4741,6 +4741,13 @@
"explicitly moving variable of type %0 to itself">,
InGroup<SelfMove>, DefaultIgnore;
+def warn_pessimizing_move : Warning<
+ "return value optimization for function %0 prevented by call to "
+ "std::move on return statement">,
+ InGroup<PessimizingMove>, DefaultIgnore;
+def note_remove_move : Note<
+ "remove call to std::move here to allow return value optimization">;
+
def warn_string_plus_int : Warning<
"adding %0 to a string does not append to the string">,
InGroup<StringPlusInt>;
Index: include/clang/Sema/ScopeInfo.h
===================================================================
--- include/clang/Sema/ScopeInfo.h
+++ include/clang/Sema/ScopeInfo.h
@@ -124,6 +124,9 @@
/// false if there is an invocation of an initializer on 'self'.
bool ObjCWarnForNoInitDelegation;
+ /// True when function may contain pessimizing moves in return statements.
+ bool HasPossiblePessimizingMove;
+
/// First C++ 'try' statement in the current function.
SourceLocation FirstCXXTryLoc;
@@ -150,7 +153,7 @@
/// current function scope. These diagnostics are vetted for reachability
/// prior to being emitted.
SmallVector<PossiblyUnreachableDiag, 4> PossiblyUnreachableDiags;
-
+
/// \brief A list of parameters which have the nonnull attribute and are
/// modified in the function.
llvm::SmallPtrSet<const ParmVarDecl*, 8> ModifiedNonNullParams;
@@ -342,7 +345,7 @@
(HasIndirectGoto ||
(HasBranchProtectedScope && HasBranchIntoScope));
}
-
+
FunctionScopeInfo(DiagnosticsEngine &Diag)
: Kind(SK_Function),
HasBranchProtectedScope(false),
@@ -354,6 +357,7 @@
ObjCWarnForNoDesignatedInitChain(false),
ObjCIsSecondaryInit(false),
ObjCWarnForNoInitDelegation(false),
+ HasPossiblePessimizingMove(true),
ErrorTrap(Diag) { }
virtual ~FunctionScopeInfo();
Index: include/clang/Sema/Sema.h
===================================================================
--- include/clang/Sema/Sema.h
+++ include/clang/Sema/Sema.h
@@ -1706,7 +1706,8 @@
/// \c constexpr in C++11 or has an 'auto' return type in C++14).
bool canSkipFunctionBody(Decl *D);
- void computeNRVO(Stmt *Body, sema::FunctionScopeInfo *Scope);
+ void computeNRVO(Stmt *Body, sema::FunctionScopeInfo *Scope,
+ FunctionDecl *FD = nullptr);
Decl *ActOnFinishFunctionBody(Decl *Decl, Stmt *Body);
Decl *ActOnFinishFunctionBody(Decl *Decl, Stmt *Body, bool IsInstantiation);
Decl *ActOnSkippedFunctionBody(Decl *Decl);
Index: lib/Sema/ScopeInfo.cpp
===================================================================
--- lib/Sema/ScopeInfo.cpp
+++ lib/Sema/ScopeInfo.cpp
@@ -33,6 +33,7 @@
ObjCWarnForNoDesignatedInitChain = false;
ObjCIsSecondaryInit = false;
ObjCWarnForNoInitDelegation = false;
+ HasPossiblePessimizingMove = true;
FirstCXXTryLoc = SourceLocation();
FirstSEHTryLoc = SourceLocation();
Index: lib/Sema/SemaDecl.cpp
===================================================================
--- lib/Sema/SemaDecl.cpp
+++ lib/Sema/SemaDecl.cpp
@@ -10367,24 +10367,58 @@
}
/// \brief Given the set of return statements within a function body,
-/// compute the variables that are subject to the named return value
+/// compute the variables that are subject to the named return value
/// optimization.
///
-/// Each of the variables that is subject to the named return value
+/// Each of the variables that is subject to the named return value
/// optimization will be marked as NRVO variables in the AST, and any
/// return statement that has a marked NRVO variable as its NRVO candidate can
/// use the named return value optimization.
///
/// This function applies a very simplistic algorithm for NRVO: if every return
/// statement in the scope of a variable has the same NRVO candidate, that
/// candidate is an NRVO variable.
-void Sema::computeNRVO(Stmt *Body, FunctionScopeInfo *Scope) {
+void Sema::computeNRVO(Stmt *Body, FunctionScopeInfo *Scope, FunctionDecl *FD) {
ReturnStmt **Returns = Scope->Returns.data();
+ bool FoundPessimizingMove = false;
+ const VarDecl *VD = nullptr;
+ bool HasVD = false;
for (unsigned I = 0, E = Scope->Returns.size(); I != E; ++I) {
if (const VarDecl *NRVOCandidate = Returns[I]->getNRVOCandidate()) {
- if (!NRVOCandidate->isNRVOVariable())
+ if (!HasVD) {
+ VD = NRVOCandidate;
+ HasVD = true;
+ }
+ if (VD != NRVOCandidate)
+ VD = nullptr;
+ if (!NRVOCandidate->isNRVOVariable()) {
Returns[I]->setNRVOCandidate(nullptr);
+ if (Returns[I]->isPessimizingMoveCandidate()) {
+ FoundPessimizingMove = true;
+ }
+ }
+ }
+ }
+
+ // Only diagnose in functions
+ if (!FD) return;
+ // No pessimizing moves found in return statements
+ if (!FoundPessimizingMove) return;
+ // No single VarDecl across all return statements
+ if (!VD) return;
+ // Check the scope status of pessimizing move
+ if (!Scope->HasPossiblePessimizingMove) return;
+ // Don't warn in template instantiations
+ if (!ActiveTemplateInstantiations.empty()) return;
+
+ Diag(FD->getLocation(), diag::warn_pessimizing_move) << FD;
+
+ for (unsigned I = 0, E = Scope->Returns.size(); I != E; ++I) {
+ if (Returns[I]->isPessimizingMoveCandidate()) {
+ Diag(Returns[I]->getLocStart(), diag::note_remove_move)
+ << FixItHint::CreateReplacement(
+ Returns[I]->getRetValue()->getSourceRange(), VD->getName());
}
}
}
@@ -10498,15 +10532,15 @@
MarkVTableUsed(FD->getLocation(), Constructor->getParent());
else if (CXXDestructorDecl *Destructor = dyn_cast<CXXDestructorDecl>(FD))
MarkVTableUsed(FD->getLocation(), Destructor->getParent());
-
+
// Try to apply the named return value optimization. We have to check
// if we can do this here because lambdas keep return statements around
// to deduce an implicit return type.
if (getLangOpts().CPlusPlus && FD->getReturnType()->isRecordType() &&
!FD->isDependentContext())
- computeNRVO(Body, getCurFunction());
+ computeNRVO(Body, getCurFunction(), FD);
}
-
+
assert((FD == getCurFunctionDecl() || getCurLambda()->CallOperator == FD) &&
"Function parsing confused");
} else if (ObjCMethodDecl *MD = dyn_cast_or_null<ObjCMethodDecl>(dcl)) {
Index: lib/Sema/SemaStmt.cpp
===================================================================
--- lib/Sema/SemaStmt.cpp
+++ lib/Sema/SemaStmt.cpp
@@ -2905,16 +2905,45 @@
return R;
}
- if (VarDecl *VD =
- const_cast<VarDecl*>(cast<ReturnStmt>(R.get())->getNRVOCandidate())) {
+ ReturnStmt *RetStmt = cast<ReturnStmt>(R.get());
+ VarDecl *VD = const_cast<VarDecl *>(RetStmt->getNRVOCandidate());
+ // Only set NRVO on true NRVO candidates. Ignore pessimizing move candidates.
+ if (VD && !RetStmt->isPessimizingMoveCandidate()) {
CurScope->addNRVOCandidate(VD);
} else {
CurScope->setNoNRVO();
}
return R;
}
+// If the RetExpr is a function call in the form of std::move(x), return
+// the VarDecl for x if "return x;" could possibly be an NRVO.
+static VarDecl *GetPessimizingMoveDecl(Sema &SemaRef, QualType RetType,
+ Expr *RetExpr) {
+ if (!RetExpr || RetType->isReferenceType()) return nullptr;
+
+ CallExpr *CE = dyn_cast<CallExpr>(RetExpr);
+ if (!CE || CE->getNumArgs() != 1) return nullptr;
+
+ FunctionDecl *FD = CE->getDirectCallee();
+ if (!FD || !FD->isInStdNamespace() || !FD->getIdentifier() ||
+ !FD->getIdentifier()->isStr("move"))
+ return nullptr;
+
+ DeclRefExpr *DRE = dyn_cast<DeclRefExpr>(CE->getArg(0));
+ if (!DRE || !DRE->getDecl()) return nullptr;
+
+ VarDecl *VD = dyn_cast<VarDecl>(DRE->getDecl());
+ if (!VD) return nullptr;
+
+ if (!SemaRef.isCopyElisionCandidate(RetType, VD, false)) return nullptr;
+
+ if (!SemaRef.Context.hasSameType(RetType, VD->getType())) return nullptr;
+
+ return VD;
+}
+
StmtResult Sema::BuildReturnStmt(SourceLocation ReturnLoc, Expr *RetValExp) {
// Check for unexpanded parameter packs.
if (RetValExp && DiagnoseUnexpandedParameterPack(RetValExp))
@@ -2928,13 +2957,17 @@
const AttrVec *Attrs = nullptr;
bool isObjCMethod = false;
+ // Hold the VarDecl being moved in the return statement.
+ VarDecl* MovedDecl = nullptr;
+
if (const FunctionDecl *FD = getCurFunctionDecl()) {
FnRetType = FD->getReturnType();
if (FD->hasAttrs())
Attrs = &FD->getAttrs();
if (FD->isNoReturn())
Diag(ReturnLoc, diag::warn_noreturn_function_has_return_expr)
<< FD->getDeclName();
+ MovedDecl = GetPessimizingMoveDecl(*this, FnRetType, RetValExp);
} else if (ObjCMethodDecl *MD = getCurMethodDecl()) {
FnRetType = MD->getReturnType();
isObjCMethod = true;
@@ -3120,10 +3153,20 @@
Result = new (Context) ReturnStmt(ReturnLoc, RetValExp, NRVOCandidate);
}
- // If we need to check for the named return value optimization, save the
- // return statement in our scope for later processing.
- if (Result->getNRVOCandidate())
+ if (Result->getNRVOCandidate()) {
+ // If we need to check for the named return value optimization, save the
+ // return statement in our scope for later processing.
FunctionScopes.back()->Returns.push_back(Result);
+ } else if (MovedDecl) {
+ // If the return expression is "return std::move(x);" and "return x;" could
+ // be a NRVO, mark it as a pessimizing move for later diagnostics.
+ Result->setNRVOCandidate(MovedDecl);
+ Result->setPessimizingMoveCandidate();
+ FunctionScopes.back()->Returns.push_back(Result);
+ } else {
+ // Return statement is neither an NRVO candidate or a pessimizing move.
+ FunctionScopes.back()->HasPossiblePessimizingMove = false;
+ }
return Result;
}
_______________________________________________
cfe-commits mailing list
[email protected]
http://lists.cs.uiuc.edu/mailman/listinfo/cfe-commits
