* [PATCH] c++: Extend -Wrange-loop-construct for binding-to-temp [PR94695]
@ 2020-11-16 3:34 Marek Polacek
2020-11-16 22:02 ` Jason Merrill
0 siblings, 1 reply; 4+ messages in thread
From: Marek Polacek @ 2020-11-16 3:34 UTC (permalink / raw)
To: GCC Patches, Jason Merrill
[ This year's end-of-stage1 I'm working virtually from American Samoa. ]
This patch finishes the second half of -Wrange-loop-construct I promised
to implement: it warns when a loop variable in a range-based for-loop is
initialized with a value of a different type resulting in a copy. For
instance:
int arr[10];
for (const double &x : arr) { ... }
where in every iteration we have to create and destroy a temporary value
of type double, to which we bind the reference. This could negatively
impact performance.
As per Clang, this doesn't warn when the range returns a copy, hence the
glvalue_p check.
Bootstrapped/regtested on x86_64-pc-linux-gnu, ok for trunk?
gcc/ChangeLog:
PR c++/94695
* doc/invoke.texi: Update the -Wrange-loop-construct description.
gcc/cp/ChangeLog:
PR c++/94695
* parser.c (warn_for_range_copy): Warn when the loop variable is
initialized with a value of a different type resulting in a copy.
gcc/testsuite/ChangeLog:
PR c++/94695
* g++.dg/warn/Wrange-loop-construct2.C: New test.
---
gcc/cp/parser.c | 30 ++-
gcc/doc/invoke.texi | 18 +-
.../g++.dg/warn/Wrange-loop-construct2.C | 212 ++++++++++++++++++
3 files changed, 256 insertions(+), 4 deletions(-)
create mode 100644 gcc/testsuite/g++.dg/warn/Wrange-loop-construct2.C
diff --git a/gcc/cp/parser.c b/gcc/cp/parser.c
index 42f705266bb..e96d0d94c76 100644
--- a/gcc/cp/parser.c
+++ b/gcc/cp/parser.c
@@ -12681,8 +12681,15 @@ do_range_for_auto_deduction (tree decl, tree range_expr)
for (const auto &x : range)
- if this version doesn't make a copy. DECL is the RANGE_DECL; EXPR is the
- *__for_begin expression.
+ if this version doesn't make a copy.
+
+ This function also warns when the loop variable is initialized with
+ a value of a different type resulting in a copy:
+
+ int arr[10];
+ for (const double &x : arr)
+
+ DECL is the RANGE_DECL; EXPR is the *__for_begin expression.
This function is never called when processing_template_decl is on. */
static void
@@ -12700,7 +12707,24 @@ warn_for_range_copy (tree decl, tree expr)
if (TYPE_REF_P (type))
{
- /* TODO: Implement reference warnings. */
+ if (!reference_compatible_p (non_reference (type), TREE_TYPE (expr))
+ && !reference_related_p (non_reference (type), TREE_TYPE (expr))
+ && glvalue_p (expr))
+ {
+ auto_diagnostic_group d;
+ if (warning_at (loc, OPT_Wrange_loop_construct,
+ "loop variable %qD of type %qT binds to a temporary "
+ "constructed from type %qT", decl, type,
+ TREE_TYPE (expr)))
+ {
+ tree ref = cp_build_qualified_type (TREE_TYPE (expr),
+ TYPE_QUAL_CONST);
+ ref = cp_build_reference_type (ref, /*rval*/false);
+ inform (loc, "use non-reference type %qT to make the copy "
+ "explicit or %qT to prevent copying",
+ non_reference (type), ref);
+ }
+ }
return;
}
else if (!CP_TYPE_CONST_P (type))
diff --git a/gcc/doc/invoke.texi b/gcc/doc/invoke.texi
index b3a2c7ce51d..5a95cddc80f 100644
--- a/gcc/doc/invoke.texi
+++ b/gcc/doc/invoke.texi
@@ -3680,7 +3680,23 @@ void fn () @{
@end smallexample
It does not warn when the type being copied is a trivially-copyable type whose
-size is less than 64 bytes. This warning is enabled by @option{-Wall}.
+size is less than 64 bytes.
+
+This warning also warns when a loop variable in a range-based for-loop is
+initialized with a value of a different type resulting in a copy. For example:
+
+@smallexample
+void fn() @{
+ int arr[10];
+ for (const double &x : arr) @{ @dots{} @}
+@}
+@end smallexample
+
+In the example above, in every iteration of the loop a temporary value of
+type @code{double} is created and destroyed, to which the reference
+@code{const double &} is bound.
+
+This warning is enabled by @option{-Wall}.
@item -Wredundant-tags @r{(C++ and Objective-C++ only)}
@opindex Wredundant-tags
diff --git a/gcc/testsuite/g++.dg/warn/Wrange-loop-construct2.C b/gcc/testsuite/g++.dg/warn/Wrange-loop-construct2.C
new file mode 100644
index 00000000000..5bb08cc5a1d
--- /dev/null
+++ b/gcc/testsuite/g++.dg/warn/Wrange-loop-construct2.C
@@ -0,0 +1,212 @@
+// PR c++/94695
+// { dg-do compile { target c++11 } }
+// { dg-options "-Wrange-loop-construct" }
+
+struct Foo { };
+struct Bar {
+ char arr[100];
+ Bar(Foo);
+ Bar(int);
+ operator int();
+};
+
+template<typename T>
+struct It {
+ T operator*();
+ It operator++();
+ bool operator!=(const It);
+};
+
+template<typename T>
+struct Cont {
+ using I = It<T>;
+ I begin();
+ I end();
+};
+
+void
+fn1 ()
+{
+ int arr[10];
+ Cont<int> cont_int;
+
+ for (const double &x : arr) { (void) x; } // { dg-warning "binds to a temporary constructed from type" }
+ for (const double x : arr) { (void) x; }
+ for (const int &x : arr) { (void) x; }
+ for (double &&x : arr) { (void) x; } // { dg-warning "binds to a temporary constructed from type" }
+ for (double x : arr) { (void) x; }
+
+ for (const int &&x : cont_int) { (void) x; }
+ for (const int &x : cont_int) { (void) x; }
+ for (const int x : cont_int) { (void) x; }
+ for (int&& x : cont_int) { (void) x; }
+ for (int x : cont_int) { (void) x; }
+
+ for (const double &&x : cont_int) { (void) x; }
+ for (const double &x : cont_int) { (void) x; }
+ for (const double x : cont_int) { (void) x; }
+
+ for (double &&x : cont_int) { (void) x; }
+ for (double x : cont_int) { (void) x; }
+
+ for (const Bar &&x : cont_int) { (void) x; }
+ for (const Bar x : cont_int) { (void) x; }
+}
+
+void
+fn2 ()
+{
+ Cont<int &> cont_int_ref;
+
+ for (const int &x : cont_int_ref) { (void) x; }
+ for (const int x : cont_int_ref) { (void) x; }
+ for (int &x : cont_int_ref) { (void) x; }
+ for (int x : cont_int_ref) { (void) x; }
+
+ for (const double &&x : cont_int_ref) { (void) x; } // { dg-warning "binds to a temporary constructed from type" }
+ for (const double &x : cont_int_ref) { (void) x; } // { dg-warning "binds to a temporary constructed from type" }
+ for (const double x : cont_int_ref) { (void) x; }
+ for (double &&x : cont_int_ref) { (void) x; } // { dg-warning "binds to a temporary constructed from type" }
+ for (double x : cont_int_ref) { (void) x; }
+
+ for (const Bar &&x : cont_int_ref) { (void) x; } // { dg-warning "binds to a temporary constructed from type" }
+ for (const Bar &x : cont_int_ref) { (void) x; } // { dg-warning "binds to a temporary constructed from type" }
+ for (const Bar x : cont_int_ref) { (void) x; }
+ for (Bar &&x : cont_int_ref) { (void) x; } // { dg-warning "binds to a temporary constructed from type" }
+ for (Bar x : cont_int_ref) { (void) x; }
+}
+
+void
+fn3 ()
+{
+ Cont<Bar> cont_bar;
+
+ for (const Bar &&x : cont_bar) { (void) x; }
+ for (const Bar &x : cont_bar) { (void) x; }
+ for (const Bar x : cont_bar) { (void) x; }
+ for (Bar &&x : cont_bar) { (void) x; }
+ for (Bar x : cont_bar) { (void) x; }
+
+ for (const int &&x : cont_bar) { (void) x; }
+ for (const int &x : cont_bar) { (void) x; }
+ for (const int x : cont_bar) { (void) x; }
+ for (int &&x : cont_bar) { (void) x; }
+ for (int x : cont_bar) { (void) x; }
+}
+
+void
+fn4 ()
+{
+ Cont<Bar &> cont_bar_ref;
+
+ for (const Bar &x : cont_bar_ref) { (void) x; }
+ for (Bar &x : cont_bar_ref) { (void) x; }
+ for (Bar x : cont_bar_ref) { (void) x; }
+
+ for (const int &&x : cont_bar_ref) { (void) x; } // { dg-warning "binds to a temporary constructed from type" }
+ for (const int &x : cont_bar_ref) { (void) x; } // { dg-warning "binds to a temporary constructed from type" }
+ for (const int x : cont_bar_ref) { (void) x; }
+ for (int &&x : cont_bar_ref) { (void) x; } // { dg-warning "binds to a temporary constructed from type" }
+ for (int x : cont_bar_ref) { (void) x; }
+}
+
+void
+fn5 ()
+{
+ Cont<Foo> cont_foo;
+
+ for (const Bar &&x : cont_foo) { (void) x; }
+ for (const Bar &x : cont_foo) { (void) x; }
+ for (const Bar x : cont_foo) { (void) x; }
+ for (Bar &&x : cont_foo) { (void) x; }
+ for (Bar x : cont_foo) { (void) x; }
+}
+
+void
+fn6 ()
+{
+ Cont<Foo &> cont_foo_ref;
+
+ for (const Bar &&x : cont_foo_ref) { (void) x; } // { dg-warning "binds to a temporary constructed from type" }
+ for (const Bar &x : cont_foo_ref) { (void) x; } // { dg-warning "binds to a temporary constructed from type" }
+ for (const Bar x : cont_foo_ref) { (void) x; }
+ for (Bar &&x : cont_foo_ref) { (void) x; } // { dg-warning "binds to a temporary constructed from type" }
+ for (Bar x : cont_foo_ref) { (void) x; }
+}
+
+void
+fn7 ()
+{
+ double arr[2];
+
+ for (const double &x : arr) { (void) x; }
+ for (const double x : arr) { (void) x; }
+ for (double &x : arr) { (void) x; }
+ for (double x : arr) { (void) x; }
+
+ for (const int &&x : arr) { (void) x; } // { dg-warning "binds to a temporary constructed from type" }
+ for (const int &x : arr) { (void) x; } // { dg-warning "binds to a temporary constructed from type" }
+ for (const int x : arr) { (void) x; }
+ for (int &&x : arr) { (void) x; } // { dg-warning "binds to a temporary constructed from type" }
+ for (int x : arr) { (void) x; }
+
+ for (const Bar &&x : arr) { (void) x; } // { dg-warning "binds to a temporary constructed from type" }
+ for (const Bar &x : arr) { (void) x; } // { dg-warning "binds to a temporary constructed from type" }
+ for (const Bar x : arr) { (void) x; }
+ for (Bar &&x : arr) { (void) x; } // { dg-warning "binds to a temporary constructed from type" }
+ for (Bar x : arr) { (void) x; }
+}
+
+void
+fn8 ()
+{
+ Foo arr[2];
+
+ for (const Foo &x : arr) { (void) x; }
+ for (const Foo x : arr) { (void) x; }
+ for (Foo &x : arr) { (void) x; }
+ for (Foo x : arr) { (void) x; }
+
+ for (const Bar &&x : arr) { (void) x; } // { dg-warning "binds to a temporary constructed from type" }
+ for (const Bar &x : arr) { (void) x; } // { dg-warning "binds to a temporary constructed from type" }
+ for (const Bar x : arr) { (void) x; }
+ for (Bar &&x : arr) { (void) x; } // { dg-warning "binds to a temporary constructed from type" }
+ for (Bar x : arr) { (void) x; }
+}
+
+void
+fn9 ()
+{
+ Bar arr[2] = { 1, 2 };
+
+ for (const Bar &x : arr) { (void) x; }
+ for (Bar &x : arr) { (void) x; }
+ for (Bar x : arr) { (void) x; }
+
+ for (const int &&x : arr) { (void) x; } // { dg-warning "binds to a temporary constructed from type" }
+ for (const int &x : arr) { (void) x; } // { dg-warning "binds to a temporary constructed from type" }
+ for (const int x : arr) { (void) x; }
+ for (int &&x : arr) { (void) x; } // { dg-warning "binds to a temporary constructed from type" }
+ for (int x : arr) { (void) x; }
+}
+
+template<typename T>
+void
+fn10 ()
+{
+ Cont<Bar> cont_bar;
+
+ for (const Bar &x : cont_bar) { (void) x; }
+
+ Cont<T> cont_dep;
+ for (const T &x : cont_dep) { (void) x; }
+}
+template void fn10<Bar>();
+
+struct S {
+ void fn()
+ {
+ Cont<Bar> cont_bar;
+ for (const Bar &x : cont_bar) { (void) x; }
+ }
+};
base-commit: cba306519ce803c55d298c8d972109de602b19c6
--
2.28.0
^ permalink raw reply [flat|nested] 4+ messages in thread
* Re: [PATCH] c++: Extend -Wrange-loop-construct for binding-to-temp [PR94695]
2020-11-16 3:34 [PATCH] c++: Extend -Wrange-loop-construct for binding-to-temp [PR94695] Marek Polacek
@ 2020-11-16 22:02 ` Jason Merrill
2020-11-16 23:52 ` [PATCH v2] " Marek Polacek
0 siblings, 1 reply; 4+ messages in thread
From: Jason Merrill @ 2020-11-16 22:02 UTC (permalink / raw)
To: Marek Polacek, GCC Patches
On 11/15/20 10:34 PM, Marek Polacek wrote:
> [ This year's end-of-stage1 I'm working virtually from American Samoa. ]
>
> This patch finishes the second half of -Wrange-loop-construct I promised
> to implement: it warns when a loop variable in a range-based for-loop is
> initialized with a value of a different type resulting in a copy. For
> instance:
>
> int arr[10];
> for (const double &x : arr) { ... }
>
> where in every iteration we have to create and destroy a temporary value
> of type double, to which we bind the reference. This could negatively
> impact performance.
>
> As per Clang, this doesn't warn when the range returns a copy, hence the
> glvalue_p check.
>
> Bootstrapped/regtested on x86_64-pc-linux-gnu, ok for trunk?
>
> gcc/ChangeLog:
>
> PR c++/94695
> * doc/invoke.texi: Update the -Wrange-loop-construct description.
>
> gcc/cp/ChangeLog:
>
> PR c++/94695
> * parser.c (warn_for_range_copy): Warn when the loop variable is
> initialized with a value of a different type resulting in a copy.
>
> gcc/testsuite/ChangeLog:
>
> PR c++/94695
> * g++.dg/warn/Wrange-loop-construct2.C: New test.
> ---
> gcc/cp/parser.c | 30 ++-
> gcc/doc/invoke.texi | 18 +-
> .../g++.dg/warn/Wrange-loop-construct2.C | 212 ++++++++++++++++++
> 3 files changed, 256 insertions(+), 4 deletions(-)
> create mode 100644 gcc/testsuite/g++.dg/warn/Wrange-loop-construct2.C
>
> diff --git a/gcc/cp/parser.c b/gcc/cp/parser.c
> index 42f705266bb..e96d0d94c76 100644
> --- a/gcc/cp/parser.c
> +++ b/gcc/cp/parser.c
> @@ -12681,8 +12681,15 @@ do_range_for_auto_deduction (tree decl, tree range_expr)
>
> for (const auto &x : range)
>
> - if this version doesn't make a copy. DECL is the RANGE_DECL; EXPR is the
> - *__for_begin expression.
> + if this version doesn't make a copy.
> +
> + This function also warns when the loop variable is initialized with
> + a value of a different type resulting in a copy:
> +
> + int arr[10];
> + for (const double &x : arr)
> +
> + DECL is the RANGE_DECL; EXPR is the *__for_begin expression.
> This function is never called when processing_template_decl is on. */
>
> static void
> @@ -12700,7 +12707,24 @@ warn_for_range_copy (tree decl, tree expr)
>
> if (TYPE_REF_P (type))
> {
> - /* TODO: Implement reference warnings. */
> + if (!reference_compatible_p (non_reference (type), TREE_TYPE (expr))
> + && !reference_related_p (non_reference (type), TREE_TYPE (expr))
Is there a reason not to use ref_conv_binds_directly_p for this case as
well?
> + && glvalue_p (expr))
> + {
> + auto_diagnostic_group d;
> + if (warning_at (loc, OPT_Wrange_loop_construct,
> + "loop variable %qD of type %qT binds to a temporary "
> + "constructed from type %qT", decl, type,
> + TREE_TYPE (expr)))
> + {
> + tree ref = cp_build_qualified_type (TREE_TYPE (expr),
> + TYPE_QUAL_CONST);
> + ref = cp_build_reference_type (ref, /*rval*/false);
> + inform (loc, "use non-reference type %qT to make the copy "
> + "explicit or %qT to prevent copying",
> + non_reference (type), ref);
> + }
> + }
> return;
> }
> else if (!CP_TYPE_CONST_P (type))
> diff --git a/gcc/doc/invoke.texi b/gcc/doc/invoke.texi
> index b3a2c7ce51d..5a95cddc80f 100644
> --- a/gcc/doc/invoke.texi
> +++ b/gcc/doc/invoke.texi
> @@ -3680,7 +3680,23 @@ void fn () @{
> @end smallexample
>
> It does not warn when the type being copied is a trivially-copyable type whose
> -size is less than 64 bytes. This warning is enabled by @option{-Wall}.
> +size is less than 64 bytes.
> +
> +This warning also warns when a loop variable in a range-based for-loop is
> +initialized with a value of a different type resulting in a copy. For example:
> +
> +@smallexample
> +void fn() @{
> + int arr[10];
> + for (const double &x : arr) @{ @dots{} @}
> +@}
> +@end smallexample
> +
> +In the example above, in every iteration of the loop a temporary value of
> +type @code{double} is created and destroyed, to which the reference
> +@code{const double &} is bound.
> +
> +This warning is enabled by @option{-Wall}.
>
> @item -Wredundant-tags @r{(C++ and Objective-C++ only)}
> @opindex Wredundant-tags
> diff --git a/gcc/testsuite/g++.dg/warn/Wrange-loop-construct2.C b/gcc/testsuite/g++.dg/warn/Wrange-loop-construct2.C
> new file mode 100644
> index 00000000000..5bb08cc5a1d
> --- /dev/null
> +++ b/gcc/testsuite/g++.dg/warn/Wrange-loop-construct2.C
> @@ -0,0 +1,212 @@
> +// PR c++/94695
> +// { dg-do compile { target c++11 } }
> +// { dg-options "-Wrange-loop-construct" }
> +
> +struct Foo { };
> +struct Bar {
> + char arr[100];
> + Bar(Foo);
> + Bar(int);
> + operator int();
> +};
> +
> +template<typename T>
> +struct It {
> + T operator*();
> + It operator++();
> + bool operator!=(const It);
> +};
> +
> +template<typename T>
> +struct Cont {
> + using I = It<T>;
> + I begin();
> + I end();
> +};
> +
> +void
> +fn1 ()
> +{
> + int arr[10];
> + Cont<int> cont_int;
> +
> + for (const double &x : arr) { (void) x; } // { dg-warning "binds to a temporary constructed from type" }
> + for (const double x : arr) { (void) x; }
> + for (const int &x : arr) { (void) x; }
> + for (double &&x : arr) { (void) x; } // { dg-warning "binds to a temporary constructed from type" }
> + for (double x : arr) { (void) x; }
> +
> + for (const int &&x : cont_int) { (void) x; }
> + for (const int &x : cont_int) { (void) x; }
> + for (const int x : cont_int) { (void) x; }
> + for (int&& x : cont_int) { (void) x; }
> + for (int x : cont_int) { (void) x; }
> +
> + for (const double &&x : cont_int) { (void) x; }
> + for (const double &x : cont_int) { (void) x; }
> + for (const double x : cont_int) { (void) x; }
> +
> + for (double &&x : cont_int) { (void) x; }
> + for (double x : cont_int) { (void) x; }
> +
> + for (const Bar &&x : cont_int) { (void) x; }
> + for (const Bar x : cont_int) { (void) x; }
> +}
> +
> +void
> +fn2 ()
> +{
> + Cont<int &> cont_int_ref;
> +
> + for (const int &x : cont_int_ref) { (void) x; }
> + for (const int x : cont_int_ref) { (void) x; }
> + for (int &x : cont_int_ref) { (void) x; }
> + for (int x : cont_int_ref) { (void) x; }
> +
> + for (const double &&x : cont_int_ref) { (void) x; } // { dg-warning "binds to a temporary constructed from type" }
> + for (const double &x : cont_int_ref) { (void) x; } // { dg-warning "binds to a temporary constructed from type" }
> + for (const double x : cont_int_ref) { (void) x; }
> + for (double &&x : cont_int_ref) { (void) x; } // { dg-warning "binds to a temporary constructed from type" }
> + for (double x : cont_int_ref) { (void) x; }
> +
> + for (const Bar &&x : cont_int_ref) { (void) x; } // { dg-warning "binds to a temporary constructed from type" }
> + for (const Bar &x : cont_int_ref) { (void) x; } // { dg-warning "binds to a temporary constructed from type" }
> + for (const Bar x : cont_int_ref) { (void) x; }
> + for (Bar &&x : cont_int_ref) { (void) x; } // { dg-warning "binds to a temporary constructed from type" }
> + for (Bar x : cont_int_ref) { (void) x; }
> +}
> +
> +void
> +fn3 ()
> +{
> + Cont<Bar> cont_bar;
> +
> + for (const Bar &&x : cont_bar) { (void) x; }
> + for (const Bar &x : cont_bar) { (void) x; }
> + for (const Bar x : cont_bar) { (void) x; }
> + for (Bar &&x : cont_bar) { (void) x; }
> + for (Bar x : cont_bar) { (void) x; }
> +
> + for (const int &&x : cont_bar) { (void) x; }
> + for (const int &x : cont_bar) { (void) x; }
> + for (const int x : cont_bar) { (void) x; }
> + for (int &&x : cont_bar) { (void) x; }
> + for (int x : cont_bar) { (void) x; }
> +}
> +
> +void
> +fn4 ()
> +{
> + Cont<Bar &> cont_bar_ref;
> +
> + for (const Bar &x : cont_bar_ref) { (void) x; }
> + for (Bar &x : cont_bar_ref) { (void) x; }
> + for (Bar x : cont_bar_ref) { (void) x; }
> +
> + for (const int &&x : cont_bar_ref) { (void) x; } // { dg-warning "binds to a temporary constructed from type" }
> + for (const int &x : cont_bar_ref) { (void) x; } // { dg-warning "binds to a temporary constructed from type" }
> + for (const int x : cont_bar_ref) { (void) x; }
> + for (int &&x : cont_bar_ref) { (void) x; } // { dg-warning "binds to a temporary constructed from type" }
> + for (int x : cont_bar_ref) { (void) x; }
> +}
> +
> +void
> +fn5 ()
> +{
> + Cont<Foo> cont_foo;
> +
> + for (const Bar &&x : cont_foo) { (void) x; }
> + for (const Bar &x : cont_foo) { (void) x; }
> + for (const Bar x : cont_foo) { (void) x; }
> + for (Bar &&x : cont_foo) { (void) x; }
> + for (Bar x : cont_foo) { (void) x; }
> +}
> +
> +void
> +fn6 ()
> +{
> + Cont<Foo &> cont_foo_ref;
> +
> + for (const Bar &&x : cont_foo_ref) { (void) x; } // { dg-warning "binds to a temporary constructed from type" }
> + for (const Bar &x : cont_foo_ref) { (void) x; } // { dg-warning "binds to a temporary constructed from type" }
> + for (const Bar x : cont_foo_ref) { (void) x; }
> + for (Bar &&x : cont_foo_ref) { (void) x; } // { dg-warning "binds to a temporary constructed from type" }
> + for (Bar x : cont_foo_ref) { (void) x; }
> +}
> +
> +void
> +fn7 ()
> +{
> + double arr[2];
> +
> + for (const double &x : arr) { (void) x; }
> + for (const double x : arr) { (void) x; }
> + for (double &x : arr) { (void) x; }
> + for (double x : arr) { (void) x; }
> +
> + for (const int &&x : arr) { (void) x; } // { dg-warning "binds to a temporary constructed from type" }
> + for (const int &x : arr) { (void) x; } // { dg-warning "binds to a temporary constructed from type" }
> + for (const int x : arr) { (void) x; }
> + for (int &&x : arr) { (void) x; } // { dg-warning "binds to a temporary constructed from type" }
> + for (int x : arr) { (void) x; }
> +
> + for (const Bar &&x : arr) { (void) x; } // { dg-warning "binds to a temporary constructed from type" }
> + for (const Bar &x : arr) { (void) x; } // { dg-warning "binds to a temporary constructed from type" }
> + for (const Bar x : arr) { (void) x; }
> + for (Bar &&x : arr) { (void) x; } // { dg-warning "binds to a temporary constructed from type" }
> + for (Bar x : arr) { (void) x; }
> +}
> +
> +void
> +fn8 ()
> +{
> + Foo arr[2];
> +
> + for (const Foo &x : arr) { (void) x; }
> + for (const Foo x : arr) { (void) x; }
> + for (Foo &x : arr) { (void) x; }
> + for (Foo x : arr) { (void) x; }
> +
> + for (const Bar &&x : arr) { (void) x; } // { dg-warning "binds to a temporary constructed from type" }
> + for (const Bar &x : arr) { (void) x; } // { dg-warning "binds to a temporary constructed from type" }
> + for (const Bar x : arr) { (void) x; }
> + for (Bar &&x : arr) { (void) x; } // { dg-warning "binds to a temporary constructed from type" }
> + for (Bar x : arr) { (void) x; }
> +}
> +
> +void
> +fn9 ()
> +{
> + Bar arr[2] = { 1, 2 };
> +
> + for (const Bar &x : arr) { (void) x; }
> + for (Bar &x : arr) { (void) x; }
> + for (Bar x : arr) { (void) x; }
> +
> + for (const int &&x : arr) { (void) x; } // { dg-warning "binds to a temporary constructed from type" }
> + for (const int &x : arr) { (void) x; } // { dg-warning "binds to a temporary constructed from type" }
> + for (const int x : arr) { (void) x; }
> + for (int &&x : arr) { (void) x; } // { dg-warning "binds to a temporary constructed from type" }
> + for (int x : arr) { (void) x; }
> +}
> +
> +template<typename T>
> +void
> +fn10 ()
> +{
> + Cont<Bar> cont_bar;
> +
> + for (const Bar &x : cont_bar) { (void) x; }
> +
> + Cont<T> cont_dep;
> + for (const T &x : cont_dep) { (void) x; }
> +}
> +template void fn10<Bar>();
> +
> +struct S {
> + void fn()
> + {
> + Cont<Bar> cont_bar;
> + for (const Bar &x : cont_bar) { (void) x; }
> + }
> +};
>
> base-commit: cba306519ce803c55d298c8d972109de602b19c6
>
^ permalink raw reply [flat|nested] 4+ messages in thread
* Re: [PATCH v2] c++: Extend -Wrange-loop-construct for binding-to-temp [PR94695]
2020-11-16 22:02 ` Jason Merrill
@ 2020-11-16 23:52 ` Marek Polacek
2020-11-20 22:17 ` Jason Merrill
0 siblings, 1 reply; 4+ messages in thread
From: Marek Polacek @ 2020-11-16 23:52 UTC (permalink / raw)
To: Jason Merrill; +Cc: GCC Patches
On Mon, Nov 16, 2020 at 05:02:14PM -0500, Jason Merrill via Gcc-patches wrote:
> On 11/15/20 10:34 PM, Marek Polacek wrote:
> > [ This year's end-of-stage1 I'm working virtually from American Samoa. ]
> >
> > This patch finishes the second half of -Wrange-loop-construct I promised
> > to implement: it warns when a loop variable in a range-based for-loop is
> > initialized with a value of a different type resulting in a copy. For
> > instance:
> >
> > int arr[10];
> > for (const double &x : arr) { ... }
> >
> > where in every iteration we have to create and destroy a temporary value
> > of type double, to which we bind the reference. This could negatively
> > impact performance.
> >
> > As per Clang, this doesn't warn when the range returns a copy, hence the
> > glvalue_p check.
> >
> > Bootstrapped/regtested on x86_64-pc-linux-gnu, ok for trunk?
> >
> > gcc/ChangeLog:
> >
> > PR c++/94695
> > * doc/invoke.texi: Update the -Wrange-loop-construct description.
> >
> > gcc/cp/ChangeLog:
> >
> > PR c++/94695
> > * parser.c (warn_for_range_copy): Warn when the loop variable is
> > initialized with a value of a different type resulting in a copy.
> >
> > gcc/testsuite/ChangeLog:
> >
> > PR c++/94695
> > * g++.dg/warn/Wrange-loop-construct2.C: New test.
> > ---
> > gcc/cp/parser.c | 30 ++-
> > gcc/doc/invoke.texi | 18 +-
> > .../g++.dg/warn/Wrange-loop-construct2.C | 212 ++++++++++++++++++
> > 3 files changed, 256 insertions(+), 4 deletions(-)
> > create mode 100644 gcc/testsuite/g++.dg/warn/Wrange-loop-construct2.C
> >
> > diff --git a/gcc/cp/parser.c b/gcc/cp/parser.c
> > index 42f705266bb..e96d0d94c76 100644
> > --- a/gcc/cp/parser.c
> > +++ b/gcc/cp/parser.c
> > @@ -12681,8 +12681,15 @@ do_range_for_auto_deduction (tree decl, tree range_expr)
> > for (const auto &x : range)
> > - if this version doesn't make a copy. DECL is the RANGE_DECL; EXPR is the
> > - *__for_begin expression.
> > + if this version doesn't make a copy.
> > +
> > + This function also warns when the loop variable is initialized with
> > + a value of a different type resulting in a copy:
> > +
> > + int arr[10];
> > + for (const double &x : arr)
> > +
> > + DECL is the RANGE_DECL; EXPR is the *__for_begin expression.
> > This function is never called when processing_template_decl is on. */
> > static void
> > @@ -12700,7 +12707,24 @@ warn_for_range_copy (tree decl, tree expr)
> > if (TYPE_REF_P (type))
> > {
> > - /* TODO: Implement reference warnings. */
> > + if (!reference_compatible_p (non_reference (type), TREE_TYPE (expr))
> > + && !reference_related_p (non_reference (type), TREE_TYPE (expr))
>
> Is there a reason not to use ref_conv_binds_directly_p for this case as
> well?
No reason. reference_compatible_p also builds up a conversion, so it's not
cheaper than ref_conv_binds_directly_p as I mistakenly assumed.
Bootstrapped/regtested on x86_64-pc-linux-gnu, ok for trunk?
-- >8 --
This patch finishes the second half of -Wrange-loop-construct I promised
to implement: it warns when a loop variable in a range-based for-loop is
initialized with a value of a different type resulting in a copy. For
instance:
int arr[10];
for (const double &x : arr) { ... }
where in every iteration we have to create and destroy a temporary value
of type double, to which we bind the reference. This could negatively
impact performance.
As per Clang, this doesn't warn when the range returns a copy, hence the
glvalue_p check.
gcc/ChangeLog:
PR c++/94695
* doc/invoke.texi: Update the -Wrange-loop-construct description.
gcc/cp/ChangeLog:
PR c++/94695
* parser.c (warn_for_range_copy): Warn when the loop variable is
initialized with a value of a different type resulting in a copy.
gcc/testsuite/ChangeLog:
PR c++/94695
* g++.dg/warn/Wrange-loop-construct2.C: New test.
---
gcc/cp/parser.c | 28 ++-
gcc/doc/invoke.texi | 18 +-
.../g++.dg/warn/Wrange-loop-construct2.C | 212 ++++++++++++++++++
3 files changed, 254 insertions(+), 4 deletions(-)
create mode 100644 gcc/testsuite/g++.dg/warn/Wrange-loop-construct2.C
diff --git a/gcc/cp/parser.c b/gcc/cp/parser.c
index 42f705266bb..74584501d11 100644
--- a/gcc/cp/parser.c
+++ b/gcc/cp/parser.c
@@ -12681,8 +12681,15 @@ do_range_for_auto_deduction (tree decl, tree range_expr)
for (const auto &x : range)
- if this version doesn't make a copy. DECL is the RANGE_DECL; EXPR is the
- *__for_begin expression.
+ if this version doesn't make a copy.
+
+ This function also warns when the loop variable is initialized with
+ a value of a different type resulting in a copy:
+
+ int arr[10];
+ for (const double &x : arr)
+
+ DECL is the RANGE_DECL; EXPR is the *__for_begin expression.
This function is never called when processing_template_decl is on. */
static void
@@ -12700,7 +12707,22 @@ warn_for_range_copy (tree decl, tree expr)
if (TYPE_REF_P (type))
{
- /* TODO: Implement reference warnings. */
+ if (!ref_conv_binds_directly_p (type, expr) && glvalue_p (expr))
+ {
+ auto_diagnostic_group d;
+ if (warning_at (loc, OPT_Wrange_loop_construct,
+ "loop variable %qD of type %qT binds to a temporary "
+ "constructed from type %qT", decl, type,
+ TREE_TYPE (expr)))
+ {
+ tree ref = cp_build_qualified_type (TREE_TYPE (expr),
+ TYPE_QUAL_CONST);
+ ref = cp_build_reference_type (ref, /*rval*/false);
+ inform (loc, "use non-reference type %qT to make the copy "
+ "explicit or %qT to prevent copying",
+ non_reference (type), ref);
+ }
+ }
return;
}
else if (!CP_TYPE_CONST_P (type))
diff --git a/gcc/doc/invoke.texi b/gcc/doc/invoke.texi
index e84a3cf422e..a8d5aa50d82 100644
--- a/gcc/doc/invoke.texi
+++ b/gcc/doc/invoke.texi
@@ -3681,7 +3681,23 @@ void fn () @{
@end smallexample
It does not warn when the type being copied is a trivially-copyable type whose
-size is less than 64 bytes. This warning is enabled by @option{-Wall}.
+size is less than 64 bytes.
+
+This warning also warns when a loop variable in a range-based for-loop is
+initialized with a value of a different type resulting in a copy. For example:
+
+@smallexample
+void fn() @{
+ int arr[10];
+ for (const double &x : arr) @{ @dots{} @}
+@}
+@end smallexample
+
+In the example above, in every iteration of the loop a temporary value of
+type @code{double} is created and destroyed, to which the reference
+@code{const double &} is bound.
+
+This warning is enabled by @option{-Wall}.
@item -Wredundant-tags @r{(C++ and Objective-C++ only)}
@opindex Wredundant-tags
diff --git a/gcc/testsuite/g++.dg/warn/Wrange-loop-construct2.C b/gcc/testsuite/g++.dg/warn/Wrange-loop-construct2.C
new file mode 100644
index 00000000000..5bb08cc5a1d
--- /dev/null
+++ b/gcc/testsuite/g++.dg/warn/Wrange-loop-construct2.C
@@ -0,0 +1,212 @@
+// PR c++/94695
+// { dg-do compile { target c++11 } }
+// { dg-options "-Wrange-loop-construct" }
+
+struct Foo { };
+struct Bar {
+ char arr[100];
+ Bar(Foo);
+ Bar(int);
+ operator int();
+};
+
+template<typename T>
+struct It {
+ T operator*();
+ It operator++();
+ bool operator!=(const It);
+};
+
+template<typename T>
+struct Cont {
+ using I = It<T>;
+ I begin();
+ I end();
+};
+
+void
+fn1 ()
+{
+ int arr[10];
+ Cont<int> cont_int;
+
+ for (const double &x : arr) { (void) x; } // { dg-warning "binds to a temporary constructed from type" }
+ for (const double x : arr) { (void) x; }
+ for (const int &x : arr) { (void) x; }
+ for (double &&x : arr) { (void) x; } // { dg-warning "binds to a temporary constructed from type" }
+ for (double x : arr) { (void) x; }
+
+ for (const int &&x : cont_int) { (void) x; }
+ for (const int &x : cont_int) { (void) x; }
+ for (const int x : cont_int) { (void) x; }
+ for (int&& x : cont_int) { (void) x; }
+ for (int x : cont_int) { (void) x; }
+
+ for (const double &&x : cont_int) { (void) x; }
+ for (const double &x : cont_int) { (void) x; }
+ for (const double x : cont_int) { (void) x; }
+
+ for (double &&x : cont_int) { (void) x; }
+ for (double x : cont_int) { (void) x; }
+
+ for (const Bar &&x : cont_int) { (void) x; }
+ for (const Bar x : cont_int) { (void) x; }
+}
+
+void
+fn2 ()
+{
+ Cont<int &> cont_int_ref;
+
+ for (const int &x : cont_int_ref) { (void) x; }
+ for (const int x : cont_int_ref) { (void) x; }
+ for (int &x : cont_int_ref) { (void) x; }
+ for (int x : cont_int_ref) { (void) x; }
+
+ for (const double &&x : cont_int_ref) { (void) x; } // { dg-warning "binds to a temporary constructed from type" }
+ for (const double &x : cont_int_ref) { (void) x; } // { dg-warning "binds to a temporary constructed from type" }
+ for (const double x : cont_int_ref) { (void) x; }
+ for (double &&x : cont_int_ref) { (void) x; } // { dg-warning "binds to a temporary constructed from type" }
+ for (double x : cont_int_ref) { (void) x; }
+
+ for (const Bar &&x : cont_int_ref) { (void) x; } // { dg-warning "binds to a temporary constructed from type" }
+ for (const Bar &x : cont_int_ref) { (void) x; } // { dg-warning "binds to a temporary constructed from type" }
+ for (const Bar x : cont_int_ref) { (void) x; }
+ for (Bar &&x : cont_int_ref) { (void) x; } // { dg-warning "binds to a temporary constructed from type" }
+ for (Bar x : cont_int_ref) { (void) x; }
+}
+
+void
+fn3 ()
+{
+ Cont<Bar> cont_bar;
+
+ for (const Bar &&x : cont_bar) { (void) x; }
+ for (const Bar &x : cont_bar) { (void) x; }
+ for (const Bar x : cont_bar) { (void) x; }
+ for (Bar &&x : cont_bar) { (void) x; }
+ for (Bar x : cont_bar) { (void) x; }
+
+ for (const int &&x : cont_bar) { (void) x; }
+ for (const int &x : cont_bar) { (void) x; }
+ for (const int x : cont_bar) { (void) x; }
+ for (int &&x : cont_bar) { (void) x; }
+ for (int x : cont_bar) { (void) x; }
+}
+
+void
+fn4 ()
+{
+ Cont<Bar &> cont_bar_ref;
+
+ for (const Bar &x : cont_bar_ref) { (void) x; }
+ for (Bar &x : cont_bar_ref) { (void) x; }
+ for (Bar x : cont_bar_ref) { (void) x; }
+
+ for (const int &&x : cont_bar_ref) { (void) x; } // { dg-warning "binds to a temporary constructed from type" }
+ for (const int &x : cont_bar_ref) { (void) x; } // { dg-warning "binds to a temporary constructed from type" }
+ for (const int x : cont_bar_ref) { (void) x; }
+ for (int &&x : cont_bar_ref) { (void) x; } // { dg-warning "binds to a temporary constructed from type" }
+ for (int x : cont_bar_ref) { (void) x; }
+}
+
+void
+fn5 ()
+{
+ Cont<Foo> cont_foo;
+
+ for (const Bar &&x : cont_foo) { (void) x; }
+ for (const Bar &x : cont_foo) { (void) x; }
+ for (const Bar x : cont_foo) { (void) x; }
+ for (Bar &&x : cont_foo) { (void) x; }
+ for (Bar x : cont_foo) { (void) x; }
+}
+
+void
+fn6 ()
+{
+ Cont<Foo &> cont_foo_ref;
+
+ for (const Bar &&x : cont_foo_ref) { (void) x; } // { dg-warning "binds to a temporary constructed from type" }
+ for (const Bar &x : cont_foo_ref) { (void) x; } // { dg-warning "binds to a temporary constructed from type" }
+ for (const Bar x : cont_foo_ref) { (void) x; }
+ for (Bar &&x : cont_foo_ref) { (void) x; } // { dg-warning "binds to a temporary constructed from type" }
+ for (Bar x : cont_foo_ref) { (void) x; }
+}
+
+void
+fn7 ()
+{
+ double arr[2];
+
+ for (const double &x : arr) { (void) x; }
+ for (const double x : arr) { (void) x; }
+ for (double &x : arr) { (void) x; }
+ for (double x : arr) { (void) x; }
+
+ for (const int &&x : arr) { (void) x; } // { dg-warning "binds to a temporary constructed from type" }
+ for (const int &x : arr) { (void) x; } // { dg-warning "binds to a temporary constructed from type" }
+ for (const int x : arr) { (void) x; }
+ for (int &&x : arr) { (void) x; } // { dg-warning "binds to a temporary constructed from type" }
+ for (int x : arr) { (void) x; }
+
+ for (const Bar &&x : arr) { (void) x; } // { dg-warning "binds to a temporary constructed from type" }
+ for (const Bar &x : arr) { (void) x; } // { dg-warning "binds to a temporary constructed from type" }
+ for (const Bar x : arr) { (void) x; }
+ for (Bar &&x : arr) { (void) x; } // { dg-warning "binds to a temporary constructed from type" }
+ for (Bar x : arr) { (void) x; }
+}
+
+void
+fn8 ()
+{
+ Foo arr[2];
+
+ for (const Foo &x : arr) { (void) x; }
+ for (const Foo x : arr) { (void) x; }
+ for (Foo &x : arr) { (void) x; }
+ for (Foo x : arr) { (void) x; }
+
+ for (const Bar &&x : arr) { (void) x; } // { dg-warning "binds to a temporary constructed from type" }
+ for (const Bar &x : arr) { (void) x; } // { dg-warning "binds to a temporary constructed from type" }
+ for (const Bar x : arr) { (void) x; }
+ for (Bar &&x : arr) { (void) x; } // { dg-warning "binds to a temporary constructed from type" }
+ for (Bar x : arr) { (void) x; }
+}
+
+void
+fn9 ()
+{
+ Bar arr[2] = { 1, 2 };
+
+ for (const Bar &x : arr) { (void) x; }
+ for (Bar &x : arr) { (void) x; }
+ for (Bar x : arr) { (void) x; }
+
+ for (const int &&x : arr) { (void) x; } // { dg-warning "binds to a temporary constructed from type" }
+ for (const int &x : arr) { (void) x; } // { dg-warning "binds to a temporary constructed from type" }
+ for (const int x : arr) { (void) x; }
+ for (int &&x : arr) { (void) x; } // { dg-warning "binds to a temporary constructed from type" }
+ for (int x : arr) { (void) x; }
+}
+
+template<typename T>
+void
+fn10 ()
+{
+ Cont<Bar> cont_bar;
+
+ for (const Bar &x : cont_bar) { (void) x; }
+
+ Cont<T> cont_dep;
+ for (const T &x : cont_dep) { (void) x; }
+}
+template void fn10<Bar>();
+
+struct S {
+ void fn()
+ {
+ Cont<Bar> cont_bar;
+ for (const Bar &x : cont_bar) { (void) x; }
+ }
+};
base-commit: 814e016318646d06b1662219cc716d502b76d8ce
--
2.28.0
^ permalink raw reply [flat|nested] 4+ messages in thread
* Re: [PATCH v2] c++: Extend -Wrange-loop-construct for binding-to-temp [PR94695]
2020-11-16 23:52 ` [PATCH v2] " Marek Polacek
@ 2020-11-20 22:17 ` Jason Merrill
0 siblings, 0 replies; 4+ messages in thread
From: Jason Merrill @ 2020-11-20 22:17 UTC (permalink / raw)
To: Marek Polacek; +Cc: GCC Patches
On 11/16/20 6:52 PM, Marek Polacek wrote:
> On Mon, Nov 16, 2020 at 05:02:14PM -0500, Jason Merrill via Gcc-patches wrote:
>> On 11/15/20 10:34 PM, Marek Polacek wrote:
>>> [ This year's end-of-stage1 I'm working virtually from American Samoa. ]
>>>
>>> This patch finishes the second half of -Wrange-loop-construct I promised
>>> to implement: it warns when a loop variable in a range-based for-loop is
>>> initialized with a value of a different type resulting in a copy. For
>>> instance:
>>>
>>> int arr[10];
>>> for (const double &x : arr) { ... }
>>>
>>> where in every iteration we have to create and destroy a temporary value
>>> of type double, to which we bind the reference. This could negatively
>>> impact performance.
>>>
>>> As per Clang, this doesn't warn when the range returns a copy, hence the
>>> glvalue_p check.
>>>
>>> Bootstrapped/regtested on x86_64-pc-linux-gnu, ok for trunk?
>>>
>>> gcc/ChangeLog:
>>>
>>> PR c++/94695
>>> * doc/invoke.texi: Update the -Wrange-loop-construct description.
>>>
>>> gcc/cp/ChangeLog:
>>>
>>> PR c++/94695
>>> * parser.c (warn_for_range_copy): Warn when the loop variable is
>>> initialized with a value of a different type resulting in a copy.
>>>
>>> gcc/testsuite/ChangeLog:
>>>
>>> PR c++/94695
>>> * g++.dg/warn/Wrange-loop-construct2.C: New test.
>>> ---
>>> gcc/cp/parser.c | 30 ++-
>>> gcc/doc/invoke.texi | 18 +-
>>> .../g++.dg/warn/Wrange-loop-construct2.C | 212 ++++++++++++++++++
>>> 3 files changed, 256 insertions(+), 4 deletions(-)
>>> create mode 100644 gcc/testsuite/g++.dg/warn/Wrange-loop-construct2.C
>>>
>>> diff --git a/gcc/cp/parser.c b/gcc/cp/parser.c
>>> index 42f705266bb..e96d0d94c76 100644
>>> --- a/gcc/cp/parser.c
>>> +++ b/gcc/cp/parser.c
>>> @@ -12681,8 +12681,15 @@ do_range_for_auto_deduction (tree decl, tree range_expr)
>>> for (const auto &x : range)
>>> - if this version doesn't make a copy. DECL is the RANGE_DECL; EXPR is the
>>> - *__for_begin expression.
>>> + if this version doesn't make a copy.
>>> +
>>> + This function also warns when the loop variable is initialized with
>>> + a value of a different type resulting in a copy:
>>> +
>>> + int arr[10];
>>> + for (const double &x : arr)
>>> +
>>> + DECL is the RANGE_DECL; EXPR is the *__for_begin expression.
>>> This function is never called when processing_template_decl is on. */
>>> static void
>>> @@ -12700,7 +12707,24 @@ warn_for_range_copy (tree decl, tree expr)
>>> if (TYPE_REF_P (type))
>>> {
>>> - /* TODO: Implement reference warnings. */
>>> + if (!reference_compatible_p (non_reference (type), TREE_TYPE (expr))
>>> + && !reference_related_p (non_reference (type), TREE_TYPE (expr))
>>
>> Is there a reason not to use ref_conv_binds_directly_p for this case as
>> well?
>
> No reason. reference_compatible_p also builds up a conversion, so it's not
> cheaper than ref_conv_binds_directly_p as I mistakenly assumed.
>
> Bootstrapped/regtested on x86_64-pc-linux-gnu, ok for trunk?
>
> -- >8 --
> This patch finishes the second half of -Wrange-loop-construct I promised
> to implement: it warns when a loop variable in a range-based for-loop is
> initialized with a value of a different type resulting in a copy. For
> instance:
>
> int arr[10];
> for (const double &x : arr) { ... }
>
> where in every iteration we have to create and destroy a temporary value
> of type double, to which we bind the reference. This could negatively
> impact performance.
>
> As per Clang, this doesn't warn when the range returns a copy, hence the
> glvalue_p check.
>
> gcc/ChangeLog:
>
> PR c++/94695
> * doc/invoke.texi: Update the -Wrange-loop-construct description.
>
> gcc/cp/ChangeLog:
>
> PR c++/94695
> * parser.c (warn_for_range_copy): Warn when the loop variable is
> initialized with a value of a different type resulting in a copy.
>
> gcc/testsuite/ChangeLog:
>
> PR c++/94695
> * g++.dg/warn/Wrange-loop-construct2.C: New test.
> ---
> gcc/cp/parser.c | 28 ++-
> gcc/doc/invoke.texi | 18 +-
> .../g++.dg/warn/Wrange-loop-construct2.C | 212 ++++++++++++++++++
> 3 files changed, 254 insertions(+), 4 deletions(-)
> create mode 100644 gcc/testsuite/g++.dg/warn/Wrange-loop-construct2.C
>
> diff --git a/gcc/cp/parser.c b/gcc/cp/parser.c
> index 42f705266bb..74584501d11 100644
> --- a/gcc/cp/parser.c
> +++ b/gcc/cp/parser.c
> @@ -12681,8 +12681,15 @@ do_range_for_auto_deduction (tree decl, tree range_expr)
>
> for (const auto &x : range)
>
> - if this version doesn't make a copy. DECL is the RANGE_DECL; EXPR is the
> - *__for_begin expression.
> + if this version doesn't make a copy.
> +
> + This function also warns when the loop variable is initialized with
> + a value of a different type resulting in a copy:
> +
> + int arr[10];
> + for (const double &x : arr)
> +
> + DECL is the RANGE_DECL; EXPR is the *__for_begin expression.
> This function is never called when processing_template_decl is on. */
>
> static void
> @@ -12700,7 +12707,22 @@ warn_for_range_copy (tree decl, tree expr)
>
> if (TYPE_REF_P (type))
> {
> - /* TODO: Implement reference warnings. */
> + if (!ref_conv_binds_directly_p (type, expr) && glvalue_p (expr))
It might be cheaper to switch these tests. OK either way.
> + {
> + auto_diagnostic_group d;
> + if (warning_at (loc, OPT_Wrange_loop_construct,
> + "loop variable %qD of type %qT binds to a temporary "
> + "constructed from type %qT", decl, type,
> + TREE_TYPE (expr)))
> + {
> + tree ref = cp_build_qualified_type (TREE_TYPE (expr),
> + TYPE_QUAL_CONST);
> + ref = cp_build_reference_type (ref, /*rval*/false);
> + inform (loc, "use non-reference type %qT to make the copy "
> + "explicit or %qT to prevent copying",
> + non_reference (type), ref);
> + }
> + }
> return;
> }
> else if (!CP_TYPE_CONST_P (type))
> diff --git a/gcc/doc/invoke.texi b/gcc/doc/invoke.texi
> index e84a3cf422e..a8d5aa50d82 100644
> --- a/gcc/doc/invoke.texi
> +++ b/gcc/doc/invoke.texi
> @@ -3681,7 +3681,23 @@ void fn () @{
> @end smallexample
>
> It does not warn when the type being copied is a trivially-copyable type whose
> -size is less than 64 bytes. This warning is enabled by @option{-Wall}.
> +size is less than 64 bytes.
> +
> +This warning also warns when a loop variable in a range-based for-loop is
> +initialized with a value of a different type resulting in a copy. For example:
> +
> +@smallexample
> +void fn() @{
> + int arr[10];
> + for (const double &x : arr) @{ @dots{} @}
> +@}
> +@end smallexample
> +
> +In the example above, in every iteration of the loop a temporary value of
> +type @code{double} is created and destroyed, to which the reference
> +@code{const double &} is bound.
> +
> +This warning is enabled by @option{-Wall}.
>
> @item -Wredundant-tags @r{(C++ and Objective-C++ only)}
> @opindex Wredundant-tags
> diff --git a/gcc/testsuite/g++.dg/warn/Wrange-loop-construct2.C b/gcc/testsuite/g++.dg/warn/Wrange-loop-construct2.C
> new file mode 100644
> index 00000000000..5bb08cc5a1d
> --- /dev/null
> +++ b/gcc/testsuite/g++.dg/warn/Wrange-loop-construct2.C
> @@ -0,0 +1,212 @@
> +// PR c++/94695
> +// { dg-do compile { target c++11 } }
> +// { dg-options "-Wrange-loop-construct" }
> +
> +struct Foo { };
> +struct Bar {
> + char arr[100];
> + Bar(Foo);
> + Bar(int);
> + operator int();
> +};
> +
> +template<typename T>
> +struct It {
> + T operator*();
> + It operator++();
> + bool operator!=(const It);
> +};
> +
> +template<typename T>
> +struct Cont {
> + using I = It<T>;
> + I begin();
> + I end();
> +};
> +
> +void
> +fn1 ()
> +{
> + int arr[10];
> + Cont<int> cont_int;
> +
> + for (const double &x : arr) { (void) x; } // { dg-warning "binds to a temporary constructed from type" }
> + for (const double x : arr) { (void) x; }
> + for (const int &x : arr) { (void) x; }
> + for (double &&x : arr) { (void) x; } // { dg-warning "binds to a temporary constructed from type" }
> + for (double x : arr) { (void) x; }
> +
> + for (const int &&x : cont_int) { (void) x; }
> + for (const int &x : cont_int) { (void) x; }
> + for (const int x : cont_int) { (void) x; }
> + for (int&& x : cont_int) { (void) x; }
> + for (int x : cont_int) { (void) x; }
> +
> + for (const double &&x : cont_int) { (void) x; }
> + for (const double &x : cont_int) { (void) x; }
> + for (const double x : cont_int) { (void) x; }
> +
> + for (double &&x : cont_int) { (void) x; }
> + for (double x : cont_int) { (void) x; }
> +
> + for (const Bar &&x : cont_int) { (void) x; }
> + for (const Bar x : cont_int) { (void) x; }
> +}
> +
> +void
> +fn2 ()
> +{
> + Cont<int &> cont_int_ref;
> +
> + for (const int &x : cont_int_ref) { (void) x; }
> + for (const int x : cont_int_ref) { (void) x; }
> + for (int &x : cont_int_ref) { (void) x; }
> + for (int x : cont_int_ref) { (void) x; }
> +
> + for (const double &&x : cont_int_ref) { (void) x; } // { dg-warning "binds to a temporary constructed from type" }
> + for (const double &x : cont_int_ref) { (void) x; } // { dg-warning "binds to a temporary constructed from type" }
> + for (const double x : cont_int_ref) { (void) x; }
> + for (double &&x : cont_int_ref) { (void) x; } // { dg-warning "binds to a temporary constructed from type" }
> + for (double x : cont_int_ref) { (void) x; }
> +
> + for (const Bar &&x : cont_int_ref) { (void) x; } // { dg-warning "binds to a temporary constructed from type" }
> + for (const Bar &x : cont_int_ref) { (void) x; } // { dg-warning "binds to a temporary constructed from type" }
> + for (const Bar x : cont_int_ref) { (void) x; }
> + for (Bar &&x : cont_int_ref) { (void) x; } // { dg-warning "binds to a temporary constructed from type" }
> + for (Bar x : cont_int_ref) { (void) x; }
> +}
> +
> +void
> +fn3 ()
> +{
> + Cont<Bar> cont_bar;
> +
> + for (const Bar &&x : cont_bar) { (void) x; }
> + for (const Bar &x : cont_bar) { (void) x; }
> + for (const Bar x : cont_bar) { (void) x; }
> + for (Bar &&x : cont_bar) { (void) x; }
> + for (Bar x : cont_bar) { (void) x; }
> +
> + for (const int &&x : cont_bar) { (void) x; }
> + for (const int &x : cont_bar) { (void) x; }
> + for (const int x : cont_bar) { (void) x; }
> + for (int &&x : cont_bar) { (void) x; }
> + for (int x : cont_bar) { (void) x; }
> +}
> +
> +void
> +fn4 ()
> +{
> + Cont<Bar &> cont_bar_ref;
> +
> + for (const Bar &x : cont_bar_ref) { (void) x; }
> + for (Bar &x : cont_bar_ref) { (void) x; }
> + for (Bar x : cont_bar_ref) { (void) x; }
> +
> + for (const int &&x : cont_bar_ref) { (void) x; } // { dg-warning "binds to a temporary constructed from type" }
> + for (const int &x : cont_bar_ref) { (void) x; } // { dg-warning "binds to a temporary constructed from type" }
> + for (const int x : cont_bar_ref) { (void) x; }
> + for (int &&x : cont_bar_ref) { (void) x; } // { dg-warning "binds to a temporary constructed from type" }
> + for (int x : cont_bar_ref) { (void) x; }
> +}
> +
> +void
> +fn5 ()
> +{
> + Cont<Foo> cont_foo;
> +
> + for (const Bar &&x : cont_foo) { (void) x; }
> + for (const Bar &x : cont_foo) { (void) x; }
> + for (const Bar x : cont_foo) { (void) x; }
> + for (Bar &&x : cont_foo) { (void) x; }
> + for (Bar x : cont_foo) { (void) x; }
> +}
> +
> +void
> +fn6 ()
> +{
> + Cont<Foo &> cont_foo_ref;
> +
> + for (const Bar &&x : cont_foo_ref) { (void) x; } // { dg-warning "binds to a temporary constructed from type" }
> + for (const Bar &x : cont_foo_ref) { (void) x; } // { dg-warning "binds to a temporary constructed from type" }
> + for (const Bar x : cont_foo_ref) { (void) x; }
> + for (Bar &&x : cont_foo_ref) { (void) x; } // { dg-warning "binds to a temporary constructed from type" }
> + for (Bar x : cont_foo_ref) { (void) x; }
> +}
> +
> +void
> +fn7 ()
> +{
> + double arr[2];
> +
> + for (const double &x : arr) { (void) x; }
> + for (const double x : arr) { (void) x; }
> + for (double &x : arr) { (void) x; }
> + for (double x : arr) { (void) x; }
> +
> + for (const int &&x : arr) { (void) x; } // { dg-warning "binds to a temporary constructed from type" }
> + for (const int &x : arr) { (void) x; } // { dg-warning "binds to a temporary constructed from type" }
> + for (const int x : arr) { (void) x; }
> + for (int &&x : arr) { (void) x; } // { dg-warning "binds to a temporary constructed from type" }
> + for (int x : arr) { (void) x; }
> +
> + for (const Bar &&x : arr) { (void) x; } // { dg-warning "binds to a temporary constructed from type" }
> + for (const Bar &x : arr) { (void) x; } // { dg-warning "binds to a temporary constructed from type" }
> + for (const Bar x : arr) { (void) x; }
> + for (Bar &&x : arr) { (void) x; } // { dg-warning "binds to a temporary constructed from type" }
> + for (Bar x : arr) { (void) x; }
> +}
> +
> +void
> +fn8 ()
> +{
> + Foo arr[2];
> +
> + for (const Foo &x : arr) { (void) x; }
> + for (const Foo x : arr) { (void) x; }
> + for (Foo &x : arr) { (void) x; }
> + for (Foo x : arr) { (void) x; }
> +
> + for (const Bar &&x : arr) { (void) x; } // { dg-warning "binds to a temporary constructed from type" }
> + for (const Bar &x : arr) { (void) x; } // { dg-warning "binds to a temporary constructed from type" }
> + for (const Bar x : arr) { (void) x; }
> + for (Bar &&x : arr) { (void) x; } // { dg-warning "binds to a temporary constructed from type" }
> + for (Bar x : arr) { (void) x; }
> +}
> +
> +void
> +fn9 ()
> +{
> + Bar arr[2] = { 1, 2 };
> +
> + for (const Bar &x : arr) { (void) x; }
> + for (Bar &x : arr) { (void) x; }
> + for (Bar x : arr) { (void) x; }
> +
> + for (const int &&x : arr) { (void) x; } // { dg-warning "binds to a temporary constructed from type" }
> + for (const int &x : arr) { (void) x; } // { dg-warning "binds to a temporary constructed from type" }
> + for (const int x : arr) { (void) x; }
> + for (int &&x : arr) { (void) x; } // { dg-warning "binds to a temporary constructed from type" }
> + for (int x : arr) { (void) x; }
> +}
> +
> +template<typename T>
> +void
> +fn10 ()
> +{
> + Cont<Bar> cont_bar;
> +
> + for (const Bar &x : cont_bar) { (void) x; }
> +
> + Cont<T> cont_dep;
> + for (const T &x : cont_dep) { (void) x; }
> +}
> +template void fn10<Bar>();
> +
> +struct S {
> + void fn()
> + {
> + Cont<Bar> cont_bar;
> + for (const Bar &x : cont_bar) { (void) x; }
> + }
> +};
>
> base-commit: 814e016318646d06b1662219cc716d502b76d8ce
>
^ permalink raw reply [flat|nested] 4+ messages in thread
end of thread, other threads:[~2020-11-20 22:17 UTC | newest]
Thread overview: 4+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2020-11-16 3:34 [PATCH] c++: Extend -Wrange-loop-construct for binding-to-temp [PR94695] Marek Polacek
2020-11-16 22:02 ` Jason Merrill
2020-11-16 23:52 ` [PATCH v2] " Marek Polacek
2020-11-20 22:17 ` Jason Merrill
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for read-only IMAP folder(s) and NNTP newsgroup(s).