forked from OSchip/llvm-project
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
This commit is contained in:
Richard Trieu
parent
653457831b
commit
3e1d483e0e
|
|
@ -222,7 +222,10 @@ def GNULabelsAsValue : DiagGroup<"gnu-label-as-value">;
|
|||
def LiteralRange : DiagGroup<"literal-range">;
|
||||
def LocalTypeTemplateArgs : DiagGroup<"local-type-template-args",
|
||||
[CXX98CompatLocalTypeTemplateArgs]>;
|
||||
def LoopAnalysis : DiagGroup<"loop-analysis">;
|
||||
def RangeLoopAnalysis : DiagGroup<"range-loop-analysis">;
|
||||
def ForLoopAnalysis : DiagGroup<"for-loop-analysis">;
|
||||
def LoopAnalysis : DiagGroup<"loop-analysis", [ForLoopAnalysis,
|
||||
RangeLoopAnalysis]>;
|
||||
def MalformedWarningCheck : DiagGroup<"malformed-warning-check">;
|
||||
def Main : DiagGroup<"main">;
|
||||
def MainReturnType : DiagGroup<"main-return-type">;
|
||||
|
|
|
|||
|
|
@ -24,13 +24,30 @@ def note_defined_here : Note<"%0 defined here">;
|
|||
def warn_variables_not_in_loop_body : Warning<
|
||||
"variable%select{s| %1|s %1 and %2|s %1, %2, and %3|s %1, %2, %3, and %4}0 "
|
||||
"used in loop condition not modified in loop body">,
|
||||
InGroup<LoopAnalysis>, DefaultIgnore;
|
||||
InGroup<ForLoopAnalysis>, DefaultIgnore;
|
||||
def warn_redundant_loop_iteration : Warning<
|
||||
"variable %0 is %select{decremented|incremented}1 both in the loop header "
|
||||
"and in the loop body">,
|
||||
InGroup<LoopAnalysis>, DefaultIgnore;
|
||||
InGroup<ForLoopAnalysis>, DefaultIgnore;
|
||||
def note_loop_iteration_here : Note<"%select{decremented|incremented}0 here">;
|
||||
|
||||
def warn_for_range_const_reference_copy : Warning<
|
||||
"loop variable %0 "
|
||||
"%diff{has type $ but is initialized with type $"
|
||||
"| is initialized with a value of a different type}1,2 resulting in a copy">,
|
||||
InGroup<RangeLoopAnalysis>, DefaultIgnore;
|
||||
def note_use_type_or_non_reference : Note<
|
||||
"use non-reference type %0 to keep the copy or type %1 to prevent copying">;
|
||||
def warn_for_range_variable_always_copy : Warning<
|
||||
"loop variable %0 is always a copy because the range of type %1 does not "
|
||||
"return a reference">,
|
||||
InGroup<RangeLoopAnalysis>, DefaultIgnore;
|
||||
def note_use_non_reference_type : Note<"use non-reference type %0">;
|
||||
def warn_for_range_copy : Warning<
|
||||
"loop variable %0 of type %1 creates a copy from type %2">,
|
||||
InGroup<RangeLoopAnalysis>, DefaultIgnore;
|
||||
def note_use_reference_type : Note<"use reference type %0 to prevent copying">;
|
||||
|
||||
def warn_duplicate_enum_values : Warning<
|
||||
"element %0 has been implicitly assigned %1 which another element has "
|
||||
"been assigned">, InGroup<DiagGroup<"duplicate-enum">>, DefaultIgnore;
|
||||
|
|
|
|||
|
|
@ -2364,6 +2364,156 @@ StmtResult Sema::FinishObjCForCollectionStmt(Stmt *S, Stmt *B) {
|
|||
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 @@ StmtResult Sema::FinishCXXForRangeStmt(Stmt *S, Stmt *B) {
|
|||
DiagnoseEmptyStmtBody(ForStmt->getRParenLoc(), B,
|
||||
diag::warn_empty_range_based_for_body);
|
||||
|
||||
DiagnoseForRangeVariableCopies(*this, ForStmt);
|
||||
|
||||
return S;
|
||||
}
|
||||
|
||||
|
|
|
|||
|
|
@ -0,0 +1,299 @@
|
|||
// RUN: %clang_cc1 -fsyntax-only -std=c++11 -Wloop-analysis -verify %s
|
||||
// RUN: %clang_cc1 -fsyntax-only -std=c++11 -Wrange-loop-analysis -verify %s
|
||||
|
||||
template <typename return_type>
|
||||
struct Iterator {
|
||||
return_type operator*();
|
||||
Iterator operator++();
|
||||
bool operator!=(const Iterator);
|
||||
};
|
||||
|
||||
template <typename T>
|
||||
struct Container {
|
||||
typedef Iterator<T> I;
|
||||
|
||||
I begin();
|
||||
I end();
|
||||
};
|
||||
|
||||
struct Foo {};
|
||||
struct Bar {
|
||||
Bar(Foo);
|
||||
Bar(int);
|
||||
operator int();
|
||||
};
|
||||
|
||||
// Testing notes:
|
||||
// test0 checks that the full text of the warnings and notes is correct. The
|
||||
// rest of the tests checks a smaller portion of the text.
|
||||
// test1-6 are set in pairs, the odd numbers are the non-reference returning
|
||||
// versions of the even numbers.
|
||||
// test7-9 use an array instead of a range object
|
||||
// tests use all four versions of the loop varaible, const &T, const T, T&, and
|
||||
// T. Versions producing errors and are commented out.
|
||||
//
|
||||
// Conversion chart:
|
||||
// double <=> int
|
||||
// int <=> Bar
|
||||
// double => Bar
|
||||
// Foo => Bar
|
||||
//
|
||||
// Conversions during tests:
|
||||
// test1-2
|
||||
// int => int
|
||||
// int => double
|
||||
// int => Bar
|
||||
// test3-4
|
||||
// Bar => Bar
|
||||
// Bar => int
|
||||
// test5-6
|
||||
// Foo => Bar
|
||||
// test7
|
||||
// double => double
|
||||
// double => int
|
||||
// double => Bar
|
||||
// test8
|
||||
// Foo => Foo
|
||||
// Foo => Bar
|
||||
// test9
|
||||
// Bar => Bar
|
||||
// Bar => int
|
||||
|
||||
void test0() {
|
||||
Container<int> int_non_ref_container;
|
||||
Container<int&> int_container;
|
||||
Container<Bar&> bar_container;
|
||||
|
||||
for (const int &x : int_non_ref_container) {}
|
||||
// expected-warning@-1 {{loop variable 'x' is always a copy because the range of type 'Container<int>' does not return a reference}}
|
||||
// expected-note@-2 {{use non-reference type 'int'}}
|
||||
|
||||
for (const double &x : int_container) {}
|
||||
// expected-warning@-1 {{loop variable 'x' has type 'const double &' but is initialized with type 'int' resulting in a copy}}
|
||||
// expected-note@-2 {{use non-reference type 'double' to keep the copy or type 'const int &' to prevent copying}}
|
||||
|
||||
for (const Bar x : bar_container) {}
|
||||
// expected-warning@-1 {{loop variable 'x' of type 'const Bar' creates a copy from type 'const Bar'}}
|
||||
// expected-note@-2 {{use reference type 'const Bar &' to prevent copying}}
|
||||
}
|
||||
|
||||
void test1() {
|
||||
Container<int> A;
|
||||
|
||||
for (const int &x : A) {}
|
||||
// expected-warning@-1 {{always a copy}}
|
||||
// expected-note@-2 {{'int'}}
|
||||
for (const int x : A) {}
|
||||
// No warning, non-reference type indicates copy is made
|
||||
//for (int &x : A) {}
|
||||
// Binding error
|
||||
for (int x : A) {}
|
||||
// No warning, non-reference type indicates copy is made
|
||||
|
||||
for (const double &x : A) {}
|
||||
// expected-warning@-1 {{always a copy}}
|
||||
// expected-note@-2 {{'double'}}
|
||||
for (const double x : A) {}
|
||||
// No warning, non-reference type indicates copy is made
|
||||
//for (double &x : A) {}
|
||||
// Binding error
|
||||
for (double x : A) {}
|
||||
// No warning, non-reference type indicates copy is made
|
||||
|
||||
for (const Bar &x : A) {}
|
||||
// expected-warning@-1 {{always a copy}}
|
||||
// expected-note@-2 {{'Bar'}}
|
||||
for (const Bar x : A) {}
|
||||
// No warning, non-reference type indicates copy is made
|
||||
//for (Bar &x : A) {}
|
||||
// Binding error
|
||||
for (Bar x : A) {}
|
||||
// No warning, non-reference type indicates copy is made
|
||||
}
|
||||
|
||||
void test2() {
|
||||
Container<int&> B;
|
||||
|
||||
for (const int &x : B) {}
|
||||
// No warning, this reference is not a temporary
|
||||
for (const int x : B) {}
|
||||
// No warning on POD copy
|
||||
for (int &x : B) {}
|
||||
// No warning
|
||||
for (int x : B) {}
|
||||
// No warning
|
||||
|
||||
for (const double &x : B) {}
|
||||
// expected-warning@-1 {{resulting in a copy}}
|
||||
// expected-note-re@-2 {{'double'{{.*}}'const int &'}}
|
||||
for (const double x : B) {}
|
||||
//for (double &x : B) {}
|
||||
// Binding error
|
||||
for (double x : B) {}
|
||||
// No warning
|
||||
|
||||
for (const Bar &x : B) {}
|
||||
// expected-warning@-1 {{resulting in a copy}}
|
||||
// expected-note@-2 {{'Bar'}}
|
||||
for (const Bar x : B) {}
|
||||
//for (Bar &x : B) {}
|
||||
// Binding error
|
||||
for (Bar x : B) {}
|
||||
// No warning
|
||||
}
|
||||
|
||||
void test3() {
|
||||
Container<Bar> C;
|
||||
|
||||
for (const Bar &x : C) {}
|
||||
// expected-warning@-1 {{always a copy}}
|
||||
// expected-note@-2 {{'Bar'}}
|
||||
for (const Bar x : C) {}
|
||||
// No warning, non-reference type indicates copy is made
|
||||
//for (Bar &x : C) {}
|
||||
// Binding error
|
||||
for (Bar x : C) {}
|
||||
// No warning, non-reference type indicates copy is made
|
||||
|
||||
for (const int &x : C) {}
|
||||
// expected-warning@-1 {{always a copy}}
|
||||
// expected-note@-2 {{'int'}}
|
||||
for (const int x : C) {}
|
||||
// No warning, copy made
|
||||
//for (int &x : C) {}
|
||||
// Binding error
|
||||
for (int x : C) {}
|
||||
// No warning, copy made
|
||||
}
|
||||
|
||||
void test4() {
|
||||
Container<Bar&> D;
|
||||
|
||||
for (const Bar &x : D) {}
|
||||
// No warning, this reference is not a temporary
|
||||
for (const Bar x : D) {}
|
||||
// expected-warning@-1 {{creates a copy}}
|
||||
// expected-note@-2 {{'const Bar &'}}
|
||||
for (Bar &x : D) {}
|
||||
// No warning
|
||||
for (Bar x : D) {}
|
||||
// No warning
|
||||
|
||||
for (const int &x : D) {}
|
||||
// expected-warning@-1 {{resulting in a copy}}
|
||||
// expected-note-re@-2 {{'int'{{.*}}'const Bar &'}}
|
||||
for (const int x : D) {}
|
||||
// No warning
|
||||
//for (int &x : D) {}
|
||||
// Binding error
|
||||
for (int x : D) {}
|
||||
// No warning
|
||||
}
|
||||
|
||||
void test5() {
|
||||
Container<Foo> E;
|
||||
|
||||
for (const Bar &x : E) {}
|
||||
// expected-warning@-1 {{always a copy}}
|
||||
// expected-note@-2 {{'Bar'}}
|
||||
for (const Bar x : E) {}
|
||||
// No warning, non-reference type indicates copy is made
|
||||
//for (Bar &x : E) {}
|
||||
// Binding error
|
||||
for (Bar x : E) {}
|
||||
// No warning, non-reference type indicates copy is made
|
||||
}
|
||||
|
||||
void test6() {
|
||||
Container<Foo&> F;
|
||||
|
||||
for (const Bar &x : F) {}
|
||||
// expected-warning@-1 {{resulting in a copy}}
|
||||
// expected-note-re@-2 {{'Bar'{{.*}}'const Foo &'}}
|
||||
for (const Bar x : F) {}
|
||||
// No warning.
|
||||
//for (Bar &x : F) {}
|
||||
// Binding error
|
||||
for (Bar x : F) {}
|
||||
// No warning
|
||||
}
|
||||
|
||||
void test7() {
|
||||
double G[2];
|
||||
|
||||
for (const double &x : G) {}
|
||||
// No warning
|
||||
for (const double x : G) {}
|
||||
// No warning on POD copy
|
||||
for (double &x : G) {}
|
||||
// No warning
|
||||
for (double x : G) {}
|
||||
// No warning
|
||||
|
||||
for (const int &x : G) {}
|
||||
// expected-warning@-1 {{resulting in a copy}}
|
||||
// expected-note-re@-2 {{'int'{{.*}}'const double &'}}
|
||||
for (const int x : G) {}
|
||||
// No warning
|
||||
//for (int &x : G) {}
|
||||
// Binding error
|
||||
for (int x : G) {}
|
||||
// No warning
|
||||
|
||||
for (const Bar &x : G) {}
|
||||
// expected-warning@-1 {{resulting in a copy}}
|
||||
// expected-note-re@-2 {{'Bar'{{.*}}'const double &'}}
|
||||
for (const Bar x : G) {}
|
||||
// No warning
|
||||
//for (int &Bar : G) {}
|
||||
// Binding error
|
||||
for (int Bar : G) {}
|
||||
// No warning
|
||||
}
|
||||
|
||||
void test8() {
|
||||
Foo H[2];
|
||||
|
||||
for (const Foo &x : H) {}
|
||||
// No warning
|
||||
for (const Foo x : H) {}
|
||||
// No warning on POD copy
|
||||
for (Foo &x : H) {}
|
||||
// No warning
|
||||
for (Foo x : H) {}
|
||||
// No warning
|
||||
|
||||
for (const Bar &x : H) {}
|
||||
// expected-warning@-1 {{resulting in a copy}}
|
||||
// expected-note-re@-2 {{'Bar'{{.*}}'const Foo &'}}
|
||||
for (const Bar x : H) {}
|
||||
// No warning
|
||||
//for (Bar &x: H) {}
|
||||
// Binding error
|
||||
for (Bar x: H) {}
|
||||
// No warning
|
||||
}
|
||||
|
||||
void test9() {
|
||||
Bar I[2] = {1,2};
|
||||
|
||||
for (const Bar &x : I) {}
|
||||
// No warning
|
||||
for (const Bar x : I) {}
|
||||
// expected-warning@-1 {{creates a copy}}
|
||||
// expected-note@-2 {{'const Bar &'}}
|
||||
for (Bar &x : I) {}
|
||||
// No warning
|
||||
for (Bar x : I) {}
|
||||
// No warning
|
||||
|
||||
for (const int &x : I) {}
|
||||
// expected-warning@-1 {{resulting in a copy}}
|
||||
// expected-note-re@-2 {{'int'{{.*}}'const Bar &'}}
|
||||
for (const int x : I) {}
|
||||
// No warning
|
||||
//for (int &x : I) {}
|
||||
// Binding error
|
||||
for (int x : I) {}
|
||||
// No warning
|
||||
}
|
||||
Loading…
Reference in New Issue