c++: Extend -Wrange-loop-construct for binding-to-temp [PR94695]
authorMarek Polacek <polacek@redhat.com>
Tue, 10 Nov 2020 02:15:33 +0000 (21:15 -0500)
committerMarek Polacek <polacek@redhat.com>
Sat, 21 Nov 2020 21:20:16 +0000 (16:20 -0500)
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
gcc/doc/invoke.texi
gcc/testsuite/g++.dg/warn/Wrange-loop-construct2.C [new file with mode: 0644]

index cea7cb0bd17df7be5df02c82ecd323c5e85b4213..88021243ee4cd220440b710e2679b31402c8853e 100644 (file)
@@ -12679,8 +12679,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
@@ -12698,7 +12705,22 @@ warn_for_range_copy (tree decl, tree expr)
 
   if (TYPE_REF_P (type))
     {
-      /* TODO: Implement reference warnings.  */
+      if (glvalue_p (expr) && !ref_conv_binds_directly_p (type, 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))
index 02abac39de86a50c01f8570131bc6cc42f16a144..26372a2435a4d28cefc83eb01bbc842045101e3c 100644 (file)
@@ -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 (file)
index 0000000..5bb08cc
--- /dev/null
@@ -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; }
+  }
+};