Add new warning -Wrange-loop-analysis to warn on copies during loops.
-Wrange-loop-analysis is a subgroup of -Wloop-analysis and will warn when
a range-based for-loop makes copies of the elements in the range. If possible,
suggest the proper type to prevent copies, or the non-reference to help
distinguish copy versus non-copy forms. Existing warnings in -Wloop-analysis
are moved to -Wfor-loop-analysis, also a subgroup of -Wloop-analysis.
Differential Revision: http://reviews.llvm.org/D4169
llvm-svn: 234804
diff --git a/clang/lib/Sema/SemaStmt.cpp b/clang/lib/Sema/SemaStmt.cpp
index 8b174f8..ed5da43 100644
--- a/clang/lib/Sema/SemaStmt.cpp
+++ b/clang/lib/Sema/SemaStmt.cpp
@@ -2364,6 +2364,156 @@
return S;
}
+// Warn when the loop variable is a const reference that creates a copy.
+// Suggest using the non-reference type for copies. If a copy can be prevented
+// suggest the const reference type that would do so.
+// For instance, given "for (const &Foo : Range)", suggest
+// "for (const Foo : Range)" to denote a copy is made for the loop. If
+// possible, also suggest "for (const &Bar : Range)" if this type prevents
+// the copy altogether.
+static void DiagnoseForRangeReferenceVariableCopies(Sema &SemaRef,
+ const VarDecl *VD,
+ QualType RangeInitType) {
+ const Expr *InitExpr = VD->getInit();
+ if (!InitExpr)
+ return;
+
+ QualType VariableType = VD->getType();
+
+ const MaterializeTemporaryExpr *MTE =
+ dyn_cast<MaterializeTemporaryExpr>(InitExpr);
+
+ // No copy made.
+ if (!MTE)
+ return;
+
+ const Expr *E = MTE->GetTemporaryExpr()->IgnoreImpCasts();
+
+ // Searching for either UnaryOperator for dereference of a pointer or
+ // CXXOperatorCallExpr for handling iterators.
+ while (!isa<CXXOperatorCallExpr>(E) && !isa<UnaryOperator>(E)) {
+ if (const CXXConstructExpr *CCE = dyn_cast<CXXConstructExpr>(E)) {
+ E = CCE->getArg(0);
+ } else if (const CXXMemberCallExpr *Call = dyn_cast<CXXMemberCallExpr>(E)) {
+ const MemberExpr *ME = cast<MemberExpr>(Call->getCallee());
+ E = ME->getBase();
+ } else {
+ const MaterializeTemporaryExpr *MTE = cast<MaterializeTemporaryExpr>(E);
+ E = MTE->GetTemporaryExpr();
+ }
+ E = E->IgnoreImpCasts();
+ }
+
+ bool ReturnsReference = false;
+ if (isa<UnaryOperator>(E)) {
+ ReturnsReference = true;
+ } else {
+ const CXXOperatorCallExpr *Call = cast<CXXOperatorCallExpr>(E);
+ const FunctionDecl *FD = Call->getDirectCallee();
+ QualType ReturnType = FD->getReturnType();
+ ReturnsReference = ReturnType->isReferenceType();
+ }
+
+ if (ReturnsReference) {
+ // Loop variable creates a temporary. Suggest either to go with
+ // non-reference loop variable to indiciate a copy is made, or
+ // the correct time to bind a const reference.
+ SemaRef.Diag(VD->getLocation(), diag::warn_for_range_const_reference_copy)
+ << VD << VariableType << E->getType();
+ QualType NonReferenceType = VariableType.getNonReferenceType();
+ NonReferenceType.removeLocalConst();
+ QualType NewReferenceType =
+ SemaRef.Context.getLValueReferenceType(E->getType().withConst());
+ SemaRef.Diag(VD->getLocStart(), diag::note_use_type_or_non_reference)
+ << NonReferenceType << NewReferenceType << VD->getSourceRange();
+ } else {
+ // The range always returns a copy, so a temporary is always created.
+ // Suggest removing the reference from the loop variable.
+ SemaRef.Diag(VD->getLocation(), diag::warn_for_range_variable_always_copy)
+ << VD << RangeInitType;
+ QualType NonReferenceType = VariableType.getNonReferenceType();
+ NonReferenceType.removeLocalConst();
+ SemaRef.Diag(VD->getLocStart(), diag::note_use_non_reference_type)
+ << NonReferenceType << VD->getSourceRange();
+ }
+}
+
+// Warns when the loop variable can be changed to a reference type to
+// prevent a copy. For instance, if given "for (const Foo x : Range)" suggest
+// "for (const Foo &x : Range)" if this form does not make a copy.
+static void DiagnoseForRangeConstVariableCopies(Sema &SemaRef,
+ const VarDecl *VD) {
+ const Expr *InitExpr = VD->getInit();
+ if (!InitExpr)
+ return;
+
+ QualType VariableType = VD->getType();
+
+ if (const CXXConstructExpr *CE = dyn_cast<CXXConstructExpr>(InitExpr)) {
+ if (!CE->getConstructor()->isCopyConstructor())
+ return;
+ } else if (const CastExpr *CE = dyn_cast<CastExpr>(InitExpr)) {
+ if (CE->getCastKind() != CK_LValueToRValue)
+ return;
+ } else {
+ return;
+ }
+
+ // TODO: Determine a maximum size that a POD type can be before a diagnostic
+ // should be emitted. Also, only ignore POD types with trivial copy
+ // constructors.
+ if (VariableType.isPODType(SemaRef.Context))
+ return;
+
+ // Suggest changing from a const variable to a const reference variable
+ // if doing so will prevent a copy.
+ SemaRef.Diag(VD->getLocation(), diag::warn_for_range_copy)
+ << VD << VariableType << InitExpr->getType();
+ SemaRef.Diag(VD->getLocStart(), diag::note_use_reference_type)
+ << SemaRef.Context.getLValueReferenceType(VariableType)
+ << VD->getSourceRange();
+}
+
+/// DiagnoseForRangeVariableCopies - Diagnose three cases and fixes for them.
+/// 1) for (const foo &x : foos) where foos only returns a copy. Suggest
+/// using "const foo x" to show that a copy is made
+/// 2) for (const bar &x : foos) where bar is a temporary intialized by bar.
+/// Suggest either "const bar x" to keep the copying or "const foo& x" to
+/// prevent the copy.
+/// 3) for (const foo x : foos) where x is constructed from a reference foo.
+/// Suggest "const foo &x" to prevent the copy.
+static void DiagnoseForRangeVariableCopies(Sema &SemaRef,
+ const CXXForRangeStmt *ForStmt) {
+ if (SemaRef.Diags.isIgnored(diag::warn_for_range_const_reference_copy,
+ ForStmt->getLocStart()) &&
+ SemaRef.Diags.isIgnored(diag::warn_for_range_variable_always_copy,
+ ForStmt->getLocStart()) &&
+ SemaRef.Diags.isIgnored(diag::warn_for_range_copy,
+ ForStmt->getLocStart())) {
+ return;
+ }
+
+ const VarDecl *VD = ForStmt->getLoopVariable();
+ if (!VD)
+ return;
+
+ QualType VariableType = VD->getType();
+
+ if (VariableType->isIncompleteType())
+ return;
+
+ const Expr *InitExpr = VD->getInit();
+ if (!InitExpr)
+ return;
+
+ if (VariableType->isReferenceType()) {
+ DiagnoseForRangeReferenceVariableCopies(SemaRef, VD,
+ ForStmt->getRangeInit()->getType());
+ } else if (VariableType.isConstQualified()) {
+ DiagnoseForRangeConstVariableCopies(SemaRef, VD);
+ }
+}
+
/// FinishCXXForRangeStmt - Attach the body to a C++0x for-range statement.
/// This is a separate step from ActOnCXXForRangeStmt because analysis of the
/// body cannot be performed until after the type of the range variable is
@@ -2381,6 +2531,8 @@
DiagnoseEmptyStmtBody(ForStmt->getRParenLoc(), B,
diag::warn_empty_range_based_for_body);
+ DiagnoseForRangeVariableCopies(*this, ForStmt);
+
return S;
}