Update patch to be based on a newer revision.
http://reviews.llvm.org/D4169
Files:
include/clang/Basic/DiagnosticGroups.td
include/clang/Basic/DiagnosticSemaKinds.td
lib/Sema/SemaStmt.cpp
test/SemaCXX/warn-range-loop-analysis.cpp
EMAIL PREFERENCES
http://reviews.llvm.org/settings/panel/emailpreferences/
Index: include/clang/Basic/DiagnosticGroups.td
===================================================================
--- include/clang/Basic/DiagnosticGroups.td
+++ include/clang/Basic/DiagnosticGroups.td
@@ -219,7 +219,10 @@
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">;
Index: include/clang/Basic/DiagnosticSemaKinds.td
===================================================================
--- include/clang/Basic/DiagnosticSemaKinds.td
+++ include/clang/Basic/DiagnosticSemaKinds.td
@@ -24,13 +24,30 @@
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;
Index: lib/Sema/SemaStmt.cpp
===================================================================
--- lib/Sema/SemaStmt.cpp
+++ lib/Sema/SemaStmt.cpp
@@ -2373,6 +2373,157 @@
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 if (const MaterializeTemporaryExpr *MTE =
+ dyn_cast<MaterializeTemporaryExpr>(E)) {
+ E = MTE->GetTemporaryExpr();
+ } else {
+ llvm_unreachable("Unexpected expression encountered.");
+ }
+ E = E->IgnoreImpCasts();
+ }
+
+ bool ReturnsReference = false;
+ if (isa<UnaryOperator>(E)) {
+ ReturnsReference = true;
+ } else if (const CXXOperatorCallExpr *Call =
+ dyn_cast<CXXOperatorCallExpr>(E)) {
+ const FunctionDecl *FD = Call->getDirectCallee();
+ QualType ReturnType = FD->getReturnType();
+ ReturnsReference = ReturnType->isReferenceType();
+ } else {
+ llvm_unreachable("Unexpected expression encountered.");
+ }
+
+ 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 NewType =
+ SemaRef.Context.getLValueReferenceType(E->getType().withConst());
+ SemaRef.Diag(VD->getLocStart(), diag::note_use_type_or_non_reference)
+ << VariableType.getNonReferenceType() << NewType
+ << 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;
+ SemaRef.Diag(VD->getLocStart(), diag::note_use_non_reference_type)
+ << VariableType.getNonReferenceType() << 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
@@ -2390,6 +2541,8 @@
DiagnoseEmptyStmtBody(ForStmt->getRParenLoc(), B,
diag::warn_empty_range_based_for_body);
+ DiagnoseForRangeVariableCopies(*this, ForStmt);
+
return S;
}
Index: test/SemaCXX/warn-range-loop-analysis.cpp
===================================================================
--- test/SemaCXX/warn-range-loop-analysis.cpp
+++ test/SemaCXX/warn-range-loop-analysis.cpp
@@ -0,0 +1,313 @@
+// 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>
+class Iterator {
+public:
+ return_type operator*();
+ Iterator operator++();
+ bool operator!=(const Iterator);
+};
+
+template <typename T>
+class Container {
+ typedef Iterator<T&> I;
+
+public:
+ // These notes are from errors, which this test doesn't care much about.
+ I begin(); // expected-note 4{{}}
+ I end();
+};
+
+template <typename T>
+class ContainerNonReference {
+ typedef Iterator<T> I;
+
+public:
+ // These notes are from errors, which this test doesn't care much about.
+ I begin(); // expected-note 6{{}}
+ I end();
+};
+
+class Foo {};
+class Bar {
+public:
+ 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. Some of these versions produce errors and are marked a such
+//
+// 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() {
+ ContainerNonReference<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 'ContainerNonReference<int>' does not return a reference}}
+ // expected-note@-2 {{use non-reference type 'const 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 'const 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() {
+ ContainerNonReference<int> A;
+
+ for (const int &x : A) {}
+ // expected-warning@-1 {{always a copy}}
+ // expected-note@-2 {{'const int'}}
+ for (const int x : A) {}
+ // No warning, non-reference type indicates copy is made
+ for (int &x : A) {}
+ // expected-error@-1{{cannot bind}}
+ 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 {{'const double'}}
+ for (const double x : A) {}
+ // No warning, non-reference type indicates copy is made
+ for (double &x : A) {}
+ // expected-error@-1{{cannot bind}}
+ 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 {{'const Bar'}}
+ for (const Bar x : A) {}
+ // No warning, non-reference type indicates copy is made
+ for (Bar &x : A) {}
+ // expected-error@-1{{cannot bind}}
+ 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 {{'const double'{{.*}}'const int &'}}
+ for (const double x : B) {}
+ for (double &x : B) {}
+ // expected-error@-1 {{cannot bind}}
+ for (double x : B) {}
+ // No warning
+
+ for (const Bar &x : B) {}
+ // expected-warning@-1 {{resulting in a copy}}
+ // expected-note@-2 {{'const Bar'}}
+ for (const Bar x : B) {}
+ for (Bar &x : B) {}
+ // expected-error@-1 {{cannot bind}}
+ for (Bar x : B) {}
+ // No warning
+}
+
+void test3() {
+ ContainerNonReference<Bar> C;
+
+ for (const Bar &x : C) {}
+ // expected-warning@-1 {{always a copy}}
+ // expected-note@-2 {{'const Bar'}}
+ for (const Bar x : C) {}
+ // No warning, non-reference type indicates copy is made
+ for (Bar &x : C) {}
+ // expected-error@-1 {{cannot bind}}
+ 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 {{'const int'}}
+ for (const int x : C) {}
+ // Now warning, copy made
+ for (int &x : C) {}
+ // expected-error@-1 {{cannot bind}}
+ for (int x : C) {}
+ // Now 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 {{'const int'{{.*}}'const Bar &'}}
+ for (const int x : D) {}
+ // No warning
+ for (int &x : D) {}
+ // expected-error@-1 {{cannot bind}}
+ for (int x : D) {}
+ // No warning
+}
+
+void test5() {
+ ContainerNonReference<Foo> E;
+
+ for (const Bar &x : E) {}
+ // expected-warning@-1 {{always a copy}}
+ // expected-note@-2 {{'const Bar'}}
+ for (const Bar x : E) {}
+ // No warning, non-reference type indicates copy is made
+ for (Bar &x : E) {}
+ // expected-error@-1 {{cannot bind}}
+ 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 {{'const Bar'{{.*}}'const Foo &'}}
+ for (const Bar x : F) {}
+ // No warning.
+ for (Bar &x : F) {}
+ // expected-error@-1 {{cannot bind}}
+ 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 {{'const int'{{.*}}'const double &'}}
+ for (const int x : G) {}
+ // No warning
+ for (int &x : G) {}
+ // expected-error@-1 {{cannot bind}}
+ for (int x : G) {}
+ // No warning
+
+ for (const Bar &x : G) {}
+ // expected-warning@-1 {{resulting in a copy}}
+ // expected-note-re@-2 {{'const Bar'{{.*}}'const double &'}}
+ for (const Bar x : G) {}
+ // No warning
+ for (int &Bar : G) {}
+ // expected-error@-1 {{cannot bind}}
+ 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 {{'const Bar'{{.*}}'const Foo &'}}
+ for (const Bar x : H) {}
+ // No warning
+ for (Bar &x: H) {}
+ // expected-error@-1 {{cannot bind}}
+ 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 {{'const int'{{.*}}'const Bar &'}}
+ for (const int x : I) {}
+ // No warning
+ for (int &x : I) {}
+ // expected-error@-1 {{cannot bind}}
+ for (int x : I) {}
+ // No warning
+}
_______________________________________________
cfe-commits mailing list
[email protected]
http://lists.cs.uiuc.edu/mailman/listinfo/cfe-commits
