diff options
author | Marek Polacek <polacek@redhat.com> | 2020-09-24 14:30:50 -0400 |
---|---|---|
committer | Marek Polacek <polacek@redhat.com> | 2020-09-29 19:03:04 -0400 |
commit | 969baf03acd8124345617cea125b148568c7370a (patch) | |
tree | 0188d91dea5c97f873a1f94023f86f91449ec3a9 /gcc/cp/parser.c | |
parent | 01852cc865c9c53fa3ba6627c1b7abd2446f48c1 (diff) | |
download | gcc-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.c | 68 |
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; } |