aaronpuchert created this revision.
aaronpuchert added reviewers: aaron.ballman, Mordante, rtrieu.
Herald added a project: clang.
Herald added a subscriber: cfe-commits.
aaronpuchert added a comment.
This addresses my earlier comment D73007#1853563
<https://reviews.llvm.org/D73007#1853563>.
The messages for two of the warnings are misleading:
- warn_for_range_const_reference_copy suggests that the initialization of the
loop variable results in a copy. But that's not always true, we just know that
some conversion happens, potentially invoking a constructor or conversion
operator. The constructor might copy, as in the example that lead to this
message [1], but it might also not. However, the constructed object is bound to
a reference, which is potentially misleading, so we rewrite the message to
emphasize that.
- warn_for_range_variable_always_copy suggests that a reference type loop
variable initialized from a temporary "is always a copy". But we don't know
this, the range might just return temporary objects which aren't copies of
anything. (Assuming RVO a copy constructor might never have been called.)
The message for warn_for_range_copy is a bit repetitive: the type of a
VarDecl and its initialization Expr are the same up to cv-qualifiers,
because Sema will insert implicit casts or constructor calls to make
them match.
[1] https://bugs.llvm.org/show_bug.cgi?id=32823
Repository:
rG LLVM Github Monorepo
https://reviews.llvm.org/D75613
Files:
clang/include/clang/Basic/DiagnosticSemaKinds.td
clang/lib/Sema/SemaStmt.cpp
clang/test/SemaCXX/warn-range-loop-analysis-trivially-copyable.cpp
clang/test/SemaCXX/warn-range-loop-analysis.cpp
Index: clang/test/SemaCXX/warn-range-loop-analysis.cpp
===================================================================
--- clang/test/SemaCXX/warn-range-loop-analysis.cpp
+++ clang/test/SemaCXX/warn-range-loop-analysis.cpp
@@ -71,17 +71,17 @@
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-warning@-1 {{loop variable 'x' binds to a temporary value produced by a range of type 'Container<int>'}}
// expected-note@-2 {{use non-reference type 'int'}}
// CHECK: fix-it:"{{.*}}":{[[@LINE-3]]:18-[[@LINE-3]]:19}:""
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-warning@-1 {{loop variable 'x' of type 'const double &' binds to a temporary constructed from type 'int'}}
// expected-note@-2 {{use non-reference type 'double' to keep the copy or type 'const int &' to prevent copying}}
// CHECK: fix-it:"{{.*}}":{[[@LINE-3]]:21-[[@LINE-3]]:22}:""
for (const Bar x : bar_container) {}
- // expected-warning@-1 {{loop variable 'x' of type 'const Bar' creates a copy from type 'const Bar'}}
+ // expected-warning@-1 {{loop variable 'x' creates a copy from type 'const Bar'}}
// expected-note@-2 {{use reference type 'const Bar &' to prevent copying}}
// CHECK: fix-it:"{{.*}}":{[[@LINE-3]]:18-[[@LINE-3]]:18}:"&"
}
@@ -92,7 +92,7 @@
for (const int &&x : A) {}
// No warning, rvalue-reference to the temporary
for (const int &x : A) {}
- // expected-warning@-1 {{always a copy}}
+ // expected-warning@-1 {{binds to a temporary value produced by a range}}
// expected-note@-2 {{'int'}}
// CHECK: fix-it:"{{.*}}":{[[@LINE-3]]:18-[[@LINE-3]]:19}:""
for (const int x : A) {}
@@ -107,7 +107,7 @@
for (const double &&x : A) {}
// No warning, rvalue-reference to the temporary
for (const double &x : A) {}
- // expected-warning@-1 {{always a copy}}
+ // expected-warning@-1 {{binds to a temporary value produced by a range}}
// expected-note@-2 {{'double'}}
// CHECK: fix-it:"{{.*}}":{[[@LINE-3]]:21-[[@LINE-3]]:22}:""
for (const double x : A) {}
@@ -122,7 +122,7 @@
for (const Bar &&x : A) {}
// No warning, rvalue-reference to the temporary
for (const Bar &x : A) {}
- // expected-warning@-1 {{always a copy}}
+ // expected-warning@-1 {{binds to a temporary value produced by a range}}
// expected-note@-2 {{'Bar'}}
// CHECK: fix-it:"{{.*}}":{[[@LINE-3]]:18-[[@LINE-3]]:19}:""
for (const Bar x : A) {}
@@ -152,16 +152,16 @@
// No warning
for (const double &&x : B) {}
- // expected-warning@-1 {{resulting in a copy}}
+ // expected-warning@-1 {{binds to a temporary constructed from}}
// expected-note-re@-2 {{'double'{{.*}}'const int &'}}
// CHECK: fix-it:"{{.*}}":{[[@LINE-3]]:21-[[@LINE-3]]:23}:""
for (const double &x : B) {}
- // expected-warning@-1 {{resulting in a copy}}
+ // expected-warning@-1 {{binds to a temporary constructed from}}
// expected-note-re@-2 {{'double'{{.*}}'const int &'}}
// CHECK: fix-it:"{{.*}}":{[[@LINE-3]]:21-[[@LINE-3]]:22}:""
for (const double x : B) {}
for (double &&x : B) {}
- // expected-warning@-1 {{resulting in a copy}}
+ // expected-warning@-1 {{binds to a temporary constructed from}}
// expected-note-re@-2 {{'double'{{.*}}'const int &'}}
// CHECK: fix-it:"{{.*}}":{[[@LINE-3]]:15-[[@LINE-3]]:17}:""
//for (double &x : B) {}
@@ -170,16 +170,16 @@
// No warning
for (const Bar &&x : B) {}
- // expected-warning@-1 {{resulting in a copy}}
+ // expected-warning@-1 {{binds to a temporary constructed from}}
// expected-note@-2 {{'Bar'}}
// CHECK: fix-it:"{{.*}}":{[[@LINE-3]]:18-[[@LINE-3]]:20}:""
for (const Bar &x : B) {}
- // expected-warning@-1 {{resulting in a copy}}
+ // expected-warning@-1 {{binds to a temporary constructed from}}
// expected-note@-2 {{'Bar'}}
// CHECK: fix-it:"{{.*}}":{[[@LINE-3]]:18-[[@LINE-3]]:19}:""
for (const Bar x : B) {}
for (Bar &&x : B) {}
- // expected-warning@-1 {{resulting in a copy}}
+ // expected-warning@-1 {{binds to a temporary constructed from}}
// expected-note@-2 {{'Bar'}}
// CHECK: fix-it:"{{.*}}":{[[@LINE-3]]:12-[[@LINE-3]]:14}:""
//for (Bar &x : B) {}
@@ -194,7 +194,7 @@
for (const Bar &&x : C) {}
// No warning, rvalue-reference to the temporary
for (const Bar &x : C) {}
- // expected-warning@-1 {{always a copy}}
+ // expected-warning@-1 {{binds to a temporary value produced by a range}}
// expected-note@-2 {{'Bar'}}
// CHECK: fix-it:"{{.*}}":{[[@LINE-3]]:18-[[@LINE-3]]:19}:""
for (const Bar x : C) {}
@@ -209,7 +209,7 @@
for (const int &&x : C) {}
// No warning, rvalue-reference to the temporary
for (const int &x : C) {}
- // expected-warning@-1 {{always a copy}}
+ // expected-warning@-1 {{binds to a temporary value produced by a range}}
// expected-note@-2 {{'int'}}
// CHECK: fix-it:"{{.*}}":{[[@LINE-3]]:18-[[@LINE-3]]:19}:""
for (const int x : C) {}
@@ -241,17 +241,17 @@
// No warning
for (const int &&x : D) {}
- // expected-warning@-1 {{resulting in a copy}}
+ // expected-warning@-1 {{binds to a temporary constructed from}}
// expected-note-re@-2 {{'int'{{.*}}'const Bar &'}}
// CHECK: fix-it:"{{.*}}":{[[@LINE-3]]:18-[[@LINE-3]]:20}:""
for (const int &x : D) {}
- // expected-warning@-1 {{resulting in a copy}}
+ // expected-warning@-1 {{binds to a temporary constructed from}}
// expected-note-re@-2 {{'int'{{.*}}'const Bar &'}}
// CHECK: fix-it:"{{.*}}":{[[@LINE-3]]:18-[[@LINE-3]]:19}:""
for (const int x : D) {}
// No warning
for (int &&x : D) {}
- // expected-warning@-1 {{resulting in a copy}}
+ // expected-warning@-1 {{binds to a temporary constructed from}}
// expected-note-re@-2 {{'int'{{.*}}'const Bar &'}}
// CHECK: fix-it:"{{.*}}":{[[@LINE-3]]:12-[[@LINE-3]]:14}:""
//for (int &x : D) {}
@@ -266,7 +266,7 @@
for (const Bar &&x : E) {}
// No warning, rvalue-reference to the temporary
for (const Bar &x : E) {}
- // expected-warning@-1 {{always a copy}}
+ // expected-warning@-1 {{binds to a temporary value produced by a range}}
// expected-note@-2 {{'Bar'}}
// CHECK: fix-it:"{{.*}}":{[[@LINE-3]]:18-[[@LINE-3]]:19}:""
for (const Bar x : E) {}
@@ -283,17 +283,17 @@
Container<Foo&> F;
for (const Bar &&x : F) {}
- // expected-warning@-1 {{resulting in a copy}}
+ // expected-warning@-1 {{binds to a temporary constructed from}}
// expected-note-re@-2 {{'Bar'{{.*}}'const Foo &'}}
// CHECK: fix-it:"{{.*}}":{[[@LINE-3]]:18-[[@LINE-3]]:20}:""
for (const Bar &x : F) {}
- // expected-warning@-1 {{resulting in a copy}}
+ // expected-warning@-1 {{binds to a temporary constructed from}}
// expected-note-re@-2 {{'Bar'{{.*}}'const Foo &'}}
// CHECK: fix-it:"{{.*}}":{[[@LINE-3]]:18-[[@LINE-3]]:19}:""
for (const Bar x : F) {}
// No warning.
for (Bar &&x : F) {}
- // expected-warning@-1 {{resulting in a copy}}
+ // expected-warning@-1 {{binds to a temporary constructed from}}
// expected-note-re@-2 {{'Bar'{{.*}}'const Foo &'}}
// CHECK: fix-it:"{{.*}}":{[[@LINE-3]]:12-[[@LINE-3]]:14}:""
//for (Bar &x : F) {}
@@ -319,17 +319,17 @@
// No warning
for (const int &&x : G) {}
- // expected-warning@-1 {{resulting in a copy}}
+ // expected-warning@-1 {{binds to a temporary constructed from}}
// expected-note-re@-2 {{'int'{{.*}}'const double &'}}
// CHECK: fix-it:"{{.*}}":{[[@LINE-3]]:18-[[@LINE-3]]:20}:""
for (const int &x : G) {}
- // expected-warning@-1 {{resulting in a copy}}
+ // expected-warning@-1 {{binds to a temporary constructed from}}
// expected-note-re@-2 {{'int'{{.*}}'const double &'}}
// CHECK: fix-it:"{{.*}}":{[[@LINE-3]]:18-[[@LINE-3]]:19}:""
for (const int x : G) {}
// No warning
for (int &&x : G) {}
- // expected-warning@-1 {{resulting in a copy}}
+ // expected-warning@-1 {{binds to a temporary constructed from}}
// expected-note-re@-2 {{'int'{{.*}}'const double &'}}
// CHECK: fix-it:"{{.*}}":{[[@LINE-3]]:12-[[@LINE-3]]:14}:""
//for (int &x : G) {}
@@ -338,17 +338,17 @@
// No warning
for (const Bar &&x : G) {}
- // expected-warning@-1 {{resulting in a copy}}
+ // expected-warning@-1 {{binds to a temporary constructed from}}
// expected-note-re@-2 {{'Bar'{{.*}}'const double &'}}
// CHECK: fix-it:"{{.*}}":{[[@LINE-3]]:18-[[@LINE-3]]:20}:""
for (const Bar &x : G) {}
- // expected-warning@-1 {{resulting in a copy}}
+ // expected-warning@-1 {{binds to a temporary constructed from}}
// expected-note-re@-2 {{'Bar'{{.*}}'const double &'}}
// CHECK: fix-it:"{{.*}}":{[[@LINE-3]]:18-[[@LINE-3]]:19}:""
for (const Bar x : G) {}
// No warning
for (Bar &&x : G) {}
- // expected-warning@-1 {{resulting in a copy}}
+ // expected-warning@-1 {{binds to a temporary constructed from}}
// expected-note-re@-2 {{'Bar'{{.*}}'const double &'}}
// CHECK: fix-it:"{{.*}}":{[[@LINE-3]]:12-[[@LINE-3]]:14}:""
//for (Bar &x : G) {}
@@ -374,17 +374,17 @@
// No warning
for (const Bar &&x : H) {}
- // expected-warning@-1 {{resulting in a copy}}
+ // expected-warning@-1 {{binds to a temporary constructed from}}
// expected-note-re@-2 {{'Bar'{{.*}}'const Foo &'}}
// CHECK: fix-it:"{{.*}}":{[[@LINE-3]]:18-[[@LINE-3]]:20}:""
for (const Bar &x : H) {}
- // expected-warning@-1 {{resulting in a copy}}
+ // expected-warning@-1 {{binds to a temporary constructed from}}
// expected-note-re@-2 {{'Bar'{{.*}}'const Foo &'}}
// CHECK: fix-it:"{{.*}}":{[[@LINE-3]]:18-[[@LINE-3]]:19}:""
for (const Bar x : H) {}
// No warning
for (Bar &&x: H) {}
- // expected-warning@-1 {{resulting in a copy}}
+ // expected-warning@-1 {{binds to a temporary constructed from}}
// expected-note-re@-2 {{'Bar'{{.*}}'const Foo &'}}
// CHECK: fix-it:"{{.*}}":{[[@LINE-3]]:12-[[@LINE-3]]:14}:""
//for (Bar &x: H) {}
@@ -412,17 +412,17 @@
// No warning
for (const int &&x : I) {}
- // expected-warning@-1 {{resulting in a copy}}
+ // expected-warning@-1 {{binds to a temporary constructed from}}
// expected-note-re@-2 {{'int'{{.*}}'const Bar &'}}
// CHECK: fix-it:"{{.*}}":{[[@LINE-3]]:18-[[@LINE-3]]:20}:""
for (const int &x : I) {}
- // expected-warning@-1 {{resulting in a copy}}
+ // expected-warning@-1 {{binds to a temporary constructed from}}
// expected-note-re@-2 {{'int'{{.*}}'const Bar &'}}
// CHECK: fix-it:"{{.*}}":{[[@LINE-3]]:18-[[@LINE-3]]:19}:""
for (const int x : I) {}
// No warning
for (int &&x : I) {}
- // expected-warning@-1 {{resulting in a copy}}
+ // expected-warning@-1 {{binds to a temporary constructed from}}
// expected-note-re@-2 {{'int'{{.*}}'const Bar &'}}
// CHECK: fix-it:"{{.*}}":{[[@LINE-3]]:12-[[@LINE-3]]:14}:""
//for (int &x : I) {}
@@ -435,22 +435,22 @@
Container<Bar> C;
for (const Bar &x : C) {}
- // expected-warning@-1 {{always a copy}}
+ // expected-warning@-1 {{binds to a temporary value produced by a range}}
// expected-note@-2 {{'Bar'}}
// CHECK: fix-it:"{{.*}}":{[[@LINE-3]]:18-[[@LINE-3]]:19}:""
for (const Bar& x : C) {}
- // expected-warning@-1 {{always a copy}}
+ // expected-warning@-1 {{binds to a temporary value produced by a range}}
// expected-note@-2 {{'Bar'}}
// CHECK: fix-it:"{{.*}}":{[[@LINE-3]]:17-[[@LINE-3]]:18}:""
for (const Bar & x : C) {}
- // expected-warning@-1 {{always a copy}}
+ // expected-warning@-1 {{binds to a temporary value produced by a range}}
// expected-note@-2 {{'Bar'}}
// CHECK: fix-it:"{{.*}}":{[[@LINE-3]]:18-[[@LINE-3]]:20}:""
for (const Bar&x : C) {}
- // expected-warning@-1 {{always a copy}}
+ // expected-warning@-1 {{binds to a temporary value produced by a range}}
// expected-note@-2 {{'Bar'}}
// CHECK: fix-it:"{{.*}}":{[[@LINE-3]]:17-[[@LINE-3]]:18}:" "
}
@@ -461,7 +461,7 @@
// loops with dependent types.
Container<Bar> C;
for (const Bar &x : C) {}
- // expected-warning@-1 {{always a copy}}
+ // expected-warning@-1 {{binds to a temporary value produced by a range}}
// expected-note@-2 {{'Bar'}}
// CHECK: fix-it:"{{.*}}":{[[@LINE-3]]:18-[[@LINE-3]]:19}:""
@@ -475,7 +475,7 @@
static void static_member() {
Container<Bar> C;
for (const Bar &x : C) {}
- // expected-warning@-1 {{always a copy}}
+ // expected-warning@-1 {{binds to a temporary value produced by a range}}
// expected-note@-2 {{'Bar'}}
// CHECK: fix-it:"{{.*}}":{[[@LINE-3]]:20-[[@LINE-3]]:21}:""
@@ -486,7 +486,7 @@
void member() {
Container<Bar> C;
for (const Bar &x : C) {}
- // expected-warning@-1 {{always a copy}}
+ // expected-warning@-1 {{binds to a temporary value produced by a range}}
// expected-note@-2 {{'Bar'}}
// CHECK: fix-it:"{{.*}}":{[[@LINE-3]]:20-[[@LINE-3]]:21}:""
@@ -500,7 +500,7 @@
void member() {
Container<Bar> C;
for (const Bar &x : C) {}
- // expected-warning@-1 {{always a copy}}
+ // expected-warning@-1 {{binds to a temporary value produced by a range}}
// expected-note@-2 {{'Bar'}}
// CHECK: fix-it:"{{.*}}":{[[@LINE-3]]:20-[[@LINE-3]]:21}:""
}
@@ -509,7 +509,7 @@
void template_member() {
Container<Bar> C;
for (const Bar &x : C) {}
- // expected-warning@-1 {{always a copy}}
+ // expected-warning@-1 {{binds to a temporary value produced by a range}}
// expected-note@-2 {{'Bar'}}
// CHECK: fix-it:"{{.*}}":{[[@LINE-3]]:20-[[@LINE-3]]:21}:""
Index: clang/test/SemaCXX/warn-range-loop-analysis-trivially-copyable.cpp
===================================================================
--- clang/test/SemaCXX/warn-range-loop-analysis-trivially-copyable.cpp
+++ clang/test/SemaCXX/warn-range-loop-analysis-trivially-copyable.cpp
@@ -16,7 +16,7 @@
char a[65];
};
- // expected-warning@+3 {{loop variable 'r' of type 'const Record' creates a copy from type 'const Record'}}
+ // expected-warning@+3 {{loop variable 'r' creates a copy from type 'const Record'}}
// expected-note@+2 {{use reference type 'const Record &' to prevent copying}}
Record records[8];
for (const auto r : records)
@@ -40,7 +40,7 @@
char a[65];
};
- // expected-warning@+3 {{loop variable 'r' of type 'const Record' creates a copy from type 'const Record'}}
+ // expected-warning@+3 {{loop variable 'r' creates a copy from type 'const Record'}}
// expected-note@+2 {{use reference type 'const Record &' to prevent copying}}
Record records[8];
for (const auto r : records)
@@ -55,7 +55,7 @@
int b;
};
- // expected-warning@+3 {{loop variable 'r' of type 'const Record' creates a copy from type 'const Record'}}
+ // expected-warning@+3 {{loop variable 'r' creates a copy from type 'const Record'}}
// expected-note@+2 {{use reference type 'const Record &' to prevent copying}}
Record records[8];
for (const auto r : records)
@@ -81,7 +81,7 @@
char a[65];
};
- // expected-warning@+3 {{loop variable 'r' of type 'const Record' creates a copy from type 'const Record'}}
+ // expected-warning@+3 {{loop variable 'r' creates a copy from type 'const Record'}}
// expected-note@+2 {{use reference type 'const Record &' to prevent copying}}
Record records[8];
for (const auto r : records)
Index: clang/lib/Sema/SemaStmt.cpp
===================================================================
--- clang/lib/Sema/SemaStmt.cpp
+++ clang/lib/Sema/SemaStmt.cpp
@@ -2821,7 +2821,7 @@
// 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();
+ << VD << VariableType;
SemaRef.Diag(VD->getBeginLoc(), diag::note_use_reference_type)
<< SemaRef.Context.getLValueReferenceType(VariableType)
<< VD->getSourceRange()
Index: clang/include/clang/Basic/DiagnosticSemaKinds.td
===================================================================
--- clang/include/clang/Basic/DiagnosticSemaKinds.td
+++ clang/include/clang/Basic/DiagnosticSemaKinds.td
@@ -2426,18 +2426,17 @@
"selected '%select{begin|end}0' %select{function|template }1%2 with iterator type %3">;
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">,
+ "%diff{of type $ binds to a temporary constructed from type $"
+ "|binds to a temporary constructed from a different type}1,2">,
InGroup<RangeLoopConstruct>, 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">,
+ "loop variable %0 binds to a temporary value produced by a range of type %1">,
InGroup<RangeLoopBindReference>, 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">,
+ "loop variable %0 creates a copy from type %1">,
InGroup<RangeLoopConstruct>, DefaultIgnore;
def note_use_reference_type : Note<"use reference type %0 to prevent copying">;
def err_objc_for_range_init_stmt : Error<
_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
