aboutsummaryrefslogtreecommitdiff
path: root/gcc/cp/parser.c
diff options
context:
space:
mode:
authorMarek Polacek <polacek@redhat.com>2020-09-24 14:30:50 -0400
committerMarek Polacek <polacek@redhat.com>2020-09-29 19:03:04 -0400
commit969baf03acd8124345617cea125b148568c7370a (patch)
tree0188d91dea5c97f873a1f94023f86f91449ec3a9 /gcc/cp/parser.c
parent01852cc865c9c53fa3ba6627c1b7abd2446f48c1 (diff)
downloadgcc-969baf03acd8124345617cea125b148568c7370a.zip
gcc-969baf03acd8124345617cea125b148568c7370a.tar.gz
gcc-969baf03acd8124345617cea125b148568c7370a.tar.bz2
c++: Implement -Wrange-loop-construct [PR94695]
This new warning can be used to prevent expensive copies inside range-based for-loops, for instance: struct S { char arr[128]; }; void fn () { S arr[5]; for (const auto x : arr) { } } where auto deduces to S and then we copy the big S in every iteration. Using "const auto &x" would not incur such a copy. With this patch the compiler will warn: q.C:4:19: warning: loop variable 'x' creates a copy from type 'const S' [-Wrange-loop-construct] 4 | for (const auto x : arr) { } | ^ q.C:4:19: note: use reference type 'const S&' to prevent copying 4 | for (const auto x : arr) { } | ^ | & As per Clang, this warning is suppressed for trivially copyable types whose size does not exceed 64B. The tricky part of the patch was how to figure out if using a reference would have prevented a copy. To that point, I'm using the new function called ref_conv_binds_directly_p. This warning is enabled by -Wall. Further warnings of similar nature should follow soon. gcc/c-family/ChangeLog: PR c++/94695 * c.opt (Wrange-loop-construct): New option. gcc/cp/ChangeLog: PR c++/94695 * call.c (ref_conv_binds_directly_p): New function. * cp-tree.h (ref_conv_binds_directly_p): Declare. * parser.c (warn_for_range_copy): New function. (cp_convert_range_for): Call it. gcc/ChangeLog: PR c++/94695 * doc/invoke.texi: Document -Wrange-loop-construct. gcc/testsuite/ChangeLog: PR c++/94695 * g++.dg/warn/Wrange-loop-construct.C: New test.
Diffstat (limited to 'gcc/cp/parser.c')
-rw-r--r--gcc/cp/parser.c68
1 files changed, 64 insertions, 4 deletions
diff --git a/gcc/cp/parser.c b/gcc/cp/parser.c
index 8905833..cb44227 100644
--- a/gcc/cp/parser.c
+++ b/gcc/cp/parser.c
@@ -12646,6 +12646,64 @@ do_range_for_auto_deduction (tree decl, tree range_expr)
}
}
+/* Warns when the loop variable should be changed to a reference type to
+ avoid unnecessary copying. I.e., from
+
+ for (const auto x : range)
+
+ where range returns a reference, to
+
+ for (const auto &x : range)
+
+ if this version doesn't make a copy. DECL is the RANGE_DECL; EXPR is the
+ *__for_begin expression.
+ This function is never called when processing_template_decl is on. */
+
+static void
+warn_for_range_copy (tree decl, tree expr)
+{
+ if (!warn_range_loop_construct
+ || decl == error_mark_node)
+ return;
+
+ location_t loc = DECL_SOURCE_LOCATION (decl);
+ tree type = TREE_TYPE (decl);
+
+ if (from_macro_expansion_at (loc))
+ return;
+
+ if (TYPE_REF_P (type))
+ {
+ /* TODO: Implement reference warnings. */
+ return;
+ }
+ else if (!CP_TYPE_CONST_P (type))
+ return;
+
+ /* Since small trivially copyable types are cheap to copy, we suppress the
+ warning for them. 64B is a common size of a cache line. */
+ if (TREE_CODE (TYPE_SIZE_UNIT (type)) != INTEGER_CST
+ || (tree_to_uhwi (TYPE_SIZE_UNIT (type)) <= 64
+ && trivially_copyable_p (type)))
+ return;
+
+ tree rtype = cp_build_reference_type (type, /*rval*/false);
+ /* If we could initialize the reference directly, it wouldn't involve any
+ copies. */
+ if (!ref_conv_binds_directly_p (rtype, expr))
+ return;
+
+ auto_diagnostic_group d;
+ if (warning_at (loc, OPT_Wrange_loop_construct,
+ "loop variable %qD creates a copy from type %qT",
+ decl, type))
+ {
+ gcc_rich_location richloc (loc);
+ richloc.add_fixit_insert_before ("&");
+ inform (&richloc, "use reference type to prevent copying");
+ }
+}
+
/* Converts a range-based for-statement into a normal
for-statement, as per the definition.
@@ -12656,7 +12714,7 @@ do_range_for_auto_deduction (tree decl, tree range_expr)
{
auto &&__range = RANGE_EXPR;
- for (auto __begin = BEGIN_EXPR, end = END_EXPR;
+ for (auto __begin = BEGIN_EXPR, __end = END_EXPR;
__begin != __end;
++__begin)
{
@@ -12756,14 +12814,16 @@ cp_convert_range_for (tree statement, tree range_decl, tree range_expr,
cp_maybe_mangle_decomp (range_decl, decomp_first_name, decomp_cnt);
/* The declaration is initialized with *__begin inside the loop body. */
- cp_finish_decl (range_decl,
- build_x_indirect_ref (input_location, begin, RO_UNARY_STAR,
- tf_warning_or_error),
+ tree deref_begin = build_x_indirect_ref (input_location, begin, RO_UNARY_STAR,
+ tf_warning_or_error);
+ cp_finish_decl (range_decl, deref_begin,
/*is_constant_init*/false, NULL_TREE,
LOOKUP_ONLYCONVERTING);
if (VAR_P (range_decl) && DECL_DECOMPOSITION_P (range_decl))
cp_finish_decomp (range_decl, decomp_first_name, decomp_cnt);
+ warn_for_range_copy (range_decl, deref_begin);
+
return statement;
}