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.
This commit is contained in:
parent
01852cc865
commit
969baf03ac
|
@ -800,6 +800,10 @@ Wpacked-not-aligned
|
|||
C ObjC C++ ObjC++ Var(warn_packed_not_aligned) Warning LangEnabledBy(C ObjC C++ ObjC++,Wall)
|
||||
Warn when fields in a struct with the packed attribute are misaligned.
|
||||
|
||||
Wrange-loop-construct
|
||||
C++ ObjC++ Var(warn_range_loop_construct) Warning LangEnabledBy(C++ ObjC++,Wall)
|
||||
Warn when a range-based for-loop is creating unnecessary copies.
|
||||
|
||||
Wredundant-tags
|
||||
C++ ObjC++ Var(warn_redundant_tags) Warning
|
||||
Warn when a class or enumerated type is referenced using a redundant class-key.
|
||||
|
|
|
@ -8429,6 +8429,28 @@ conv_binds_ref_to_prvalue (conversion *c)
|
|||
return false;
|
||||
}
|
||||
|
||||
/* True iff converting EXPR to a reference type TYPE does not involve
|
||||
creating a temporary. */
|
||||
|
||||
bool
|
||||
ref_conv_binds_directly_p (tree type, tree expr)
|
||||
{
|
||||
gcc_assert (TYPE_REF_P (type));
|
||||
|
||||
/* Get the high-water mark for the CONVERSION_OBSTACK. */
|
||||
void *p = conversion_obstack_alloc (0);
|
||||
|
||||
conversion *conv = implicit_conversion (type, TREE_TYPE (expr), expr,
|
||||
/*c_cast_p=*/false,
|
||||
LOOKUP_IMPLICIT, tf_none);
|
||||
bool ret = conv && !conv->bad_p && !conv_binds_ref_to_prvalue (conv);
|
||||
|
||||
/* Free all the conversions we allocated. */
|
||||
obstack_free (&conversion_obstack, p);
|
||||
|
||||
return ret;
|
||||
}
|
||||
|
||||
/* Call the trivial destructor for INSTANCE, which can be either an lvalue of
|
||||
class type or a pointer to class type. If NO_PTR_DEREF is true and
|
||||
INSTANCE has pointer type, clobber the pointer rather than what it points
|
||||
|
|
|
@ -6225,6 +6225,7 @@ extern bool sufficient_parms_p (const_tree);
|
|||
extern tree type_decays_to (tree);
|
||||
extern tree extract_call_expr (tree);
|
||||
extern tree build_trivial_dtor_call (tree, bool = false);
|
||||
extern bool ref_conv_binds_directly_p (tree, tree);
|
||||
extern tree build_user_type_conversion (tree, tree, int,
|
||||
tsubst_flags_t);
|
||||
extern tree build_new_function_call (tree, vec<tree, va_gc> **,
|
||||
|
|
|
@ -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;
|
||||
}
|
||||
|
||||
|
|
|
@ -245,7 +245,7 @@ in the following sections.
|
|||
-Wmultiple-inheritance -Wnamespaces -Wnarrowing @gol
|
||||
-Wnoexcept -Wnoexcept-type -Wnon-virtual-dtor @gol
|
||||
-Wpessimizing-move -Wno-placement-new -Wplacement-new=@var{n} @gol
|
||||
-Wredundant-move -Wredundant-tags @gol
|
||||
-Wrange-loop-construct -Wredundant-move -Wredundant-tags @gol
|
||||
-Wreorder -Wregister @gol
|
||||
-Wstrict-null-sentinel -Wno-subobject-linkage -Wtemplates @gol
|
||||
-Wno-non-template-friend -Wold-style-cast @gol
|
||||
|
@ -3605,6 +3605,24 @@ treats the return value as if it were designated by an rvalue.
|
|||
|
||||
This warning is enabled by @option{-Wextra}.
|
||||
|
||||
@item -Wrange-loop-construct @r{(C++ and Objective-C++ only)}
|
||||
@opindex Wrange-loop-construct
|
||||
@opindex Wno-range-loop-construct
|
||||
This warning warns when a C++ range-based for-loop is creating an unnecessary
|
||||
copy. This can happen when the range declaration is not a reference, but
|
||||
probably should be. For example:
|
||||
|
||||
@smallexample
|
||||
struct S @{ char arr[128]; @};
|
||||
void fn () @{
|
||||
S arr[5];
|
||||
for (const auto x : arr) @{ @dots{} @}
|
||||
@}
|
||||
@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}.
|
||||
|
||||
@item -Wredundant-tags @r{(C++ and Objective-C++ only)}
|
||||
@opindex Wredundant-tags
|
||||
@opindex Wno-redundant-tags
|
||||
|
@ -5274,6 +5292,7 @@ Options} and @ref{Objective-C and Objective-C++ Dialect Options}.
|
|||
-Wparentheses @gol
|
||||
-Wpessimizing-move @r{(only for C++)} @gol
|
||||
-Wpointer-sign @gol
|
||||
-Wrange-loop-construct @r{(only for C++)} @gol
|
||||
-Wreorder @gol
|
||||
-Wrestrict @gol
|
||||
-Wreturn-type @gol
|
||||
|
|
|
@ -0,0 +1,207 @@
|
|||
// PR c++/94695
|
||||
// { dg-do compile { target c++11 } }
|
||||
// { dg-options "-Wrange-loop-construct" }
|
||||
|
||||
#include <initializer_list>
|
||||
|
||||
struct Small {
|
||||
char arr[64];
|
||||
};
|
||||
|
||||
struct Big_aggr {
|
||||
char arr[65];
|
||||
};
|
||||
|
||||
struct Big_triv_copy {
|
||||
char arr[65];
|
||||
Big_triv_copy() { }
|
||||
};
|
||||
|
||||
struct Big {
|
||||
char arr[65];
|
||||
Big () = default;
|
||||
Big(const Big&);
|
||||
};
|
||||
|
||||
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();
|
||||
};
|
||||
|
||||
#define TEST \
|
||||
void fn_macro() \
|
||||
{ \
|
||||
Cont<Bar &> cont_bar_ref; \
|
||||
for (const Bar x : cont_bar_ref) { (void) x; } \
|
||||
}
|
||||
|
||||
TEST
|
||||
|
||||
Cont<Bar &>& foo ();
|
||||
Cont<Bar &> foo2 ();
|
||||
|
||||
void
|
||||
fn1 ()
|
||||
{
|
||||
for (const auto x : foo () ) { (void) x; } // { dg-warning "creates a copy" }
|
||||
for (const auto x : foo2 () ) { (void) x; } // { dg-warning "creates a copy" }
|
||||
|
||||
Small s{};
|
||||
Small sa[5] = { };
|
||||
for (const auto x : sa) { (void) x; }
|
||||
for (const auto x : { s, s, s }) { (void) x; }
|
||||
|
||||
Big_aggr b{};
|
||||
Big_aggr ba[5] = { };
|
||||
for (const auto x : ba) { (void) x; } // { dg-warning "creates a copy" }
|
||||
for (const auto x : { b, b, b }) { (void) x; } // { dg-warning "creates a copy" }
|
||||
|
||||
Big_triv_copy bt{};
|
||||
Big_triv_copy bta[5];
|
||||
for (const auto x : bta) { (void) x; } // { dg-warning "creates a copy" }
|
||||
for (const auto x : { bt, bt, bt }) { (void) x; } // { dg-warning "creates a copy" }
|
||||
|
||||
Big b2;
|
||||
Big ba2[5];
|
||||
for (const auto x : ba2) { (void) x; } // { dg-warning "creates a copy" }
|
||||
for (const auto x : { b2, b2, b2 }) { (void) x; } // { dg-warning "creates a copy" }
|
||||
}
|
||||
|
||||
void
|
||||
fn2 ()
|
||||
{
|
||||
Cont<int> cont_int;
|
||||
for (const auto x : cont_int) { (void) x; }
|
||||
for (const int x : cont_int) { (void) x; }
|
||||
for (int x : cont_int) { (void) x; }
|
||||
for (const auto &x : cont_int) { (void) x; }
|
||||
for (double x : cont_int) { (void) x; }
|
||||
for (const double x : cont_int) { (void) x; }
|
||||
for (const Bar x : cont_int) { (void) x; }
|
||||
for (Bar x : cont_int) { (void) x; }
|
||||
}
|
||||
|
||||
void
|
||||
fn3 ()
|
||||
{
|
||||
Cont<int &> cont_int_ref;
|
||||
for (const int x : cont_int_ref) { (void) x; }
|
||||
for (int x : cont_int_ref) { (void) x; }
|
||||
for (const double x : cont_int_ref) { (void) x; }
|
||||
for (double x : cont_int_ref) { (void) x; }
|
||||
for (const Bar x : cont_int_ref) { (void) x; }
|
||||
for (Bar x : cont_int_ref) { (void) x; }
|
||||
}
|
||||
|
||||
void
|
||||
fn4 ()
|
||||
{
|
||||
Cont<Bar> cont_bar;
|
||||
for (const Bar x : cont_bar) { (void) x; }
|
||||
for (Bar x : cont_bar) { (void) x; }
|
||||
for (const int x : cont_bar) { (void) x; }
|
||||
for (int x : cont_bar) { (void) x; }
|
||||
}
|
||||
|
||||
void
|
||||
fn5 ()
|
||||
{
|
||||
Cont<Bar&> cont_bar_ref;
|
||||
for (const Bar x : cont_bar_ref) { (void) x; } // { dg-warning "creates a copy" }
|
||||
for (Bar x : cont_bar_ref) { (void) x; }
|
||||
for (const int x : cont_bar_ref) { (void) x; }
|
||||
for (int x : cont_bar_ref) { (void) x; }
|
||||
}
|
||||
|
||||
void
|
||||
fn6 ()
|
||||
{
|
||||
Cont<Foo> cont_foo;
|
||||
for (const Bar x : cont_foo) { (void) x; }
|
||||
for (Bar x : cont_foo) { (void) x; }
|
||||
}
|
||||
|
||||
void
|
||||
fn7 ()
|
||||
{
|
||||
Cont<Foo &> cont_foo_ref;
|
||||
for (const Bar x : cont_foo_ref) { (void) x; }
|
||||
for (Bar x : cont_foo_ref) { (void) x; }
|
||||
}
|
||||
|
||||
void
|
||||
fn8 ()
|
||||
{
|
||||
double arr[2];
|
||||
for (const double x : arr) { (void) x; }
|
||||
for (double x : arr) { (void) x; }
|
||||
for (const int x : arr) { (void) x; }
|
||||
for (int x : arr) { (void) x; }
|
||||
for (const Bar x : arr) { (void) x; }
|
||||
for (Bar x : arr) { (void) x; }
|
||||
}
|
||||
|
||||
void
|
||||
fn9 ()
|
||||
{
|
||||
Foo foo[2];
|
||||
for (const Foo x : foo) { (void) x; }
|
||||
for (Foo x : foo) { (void) x; }
|
||||
for (const Bar x : foo) { (void) x; }
|
||||
for (Bar x : foo) { (void) x; }
|
||||
}
|
||||
|
||||
void
|
||||
fn10 ()
|
||||
{
|
||||
Bar bar[2] = { 1, 2 };
|
||||
for (const Bar x : bar) { (void) x; } // { dg-warning "creates a copy" }
|
||||
for (Bar x : bar) { (void) x; }
|
||||
for (const int x : bar) { (void) x; }
|
||||
for (int x : bar) { (void) x; }
|
||||
}
|
||||
|
||||
template<typename T>
|
||||
void
|
||||
fn11 ()
|
||||
{
|
||||
Cont<Bar> cont_bar;
|
||||
for (const Bar x : cont_bar) { (void) x; }
|
||||
|
||||
Cont<Bar&> cont_bar_ref;
|
||||
for (const Bar x : cont_bar_ref) { (void) x; } // { dg-warning "creates a copy" }
|
||||
|
||||
Cont<T> cont_dep;
|
||||
for (const T x : cont_dep) { (void) x; }
|
||||
}
|
||||
|
||||
template<typename T>
|
||||
void
|
||||
fn12 ()
|
||||
{
|
||||
for (const auto x : { T{} }) { (void) x; } // { dg-warning "creates a copy" }
|
||||
}
|
||||
|
||||
void
|
||||
invoke ()
|
||||
{
|
||||
fn11<int> ();
|
||||
fn12<Big> ();
|
||||
}
|
Loading…
Reference in New Issue