Commit Graph

11 Commits

Author SHA1 Message Date
Aaron Puchert 33bb32bbc6 [Sema] Reword -Wrange-loop-analysis warning messages
Summary:
The messages for two of the warnings are misleading:
* warn_for_range_const_reference_copy suggests that the initialization
  of the loop variable results in a copy. But that's not always true,
  we just know that some conversion happens, potentially invoking a
  constructor or conversion operator. The constructor might copy, as in
  the example that lead to this message [1], but it might also not.
  However, the constructed object is bound to a reference, which is
  potentially misleading, so we rewrite the message to emphasize that.
  We also make sure that we print the reference type into the warning
  message to clarify that this warning only appears when operator*
  returns a reference.
* warn_for_range_variable_always_copy suggests that a reference type
  loop variable initialized from a temporary "is always a copy". But
  we don't know this, the range might just return temporary objects
  which aren't copies of anything. (Assuming RVO a copy constructor
  might never have been called.)

The message for warn_for_range_copy is a bit repetitive: the type of a
VarDecl and its initialization Expr are the same up to cv-qualifiers,
because Sema will insert implicit casts or constructor calls to make
them match.

[1] https://bugs.llvm.org/show_bug.cgi?id=32823

Reviewers: aaron.ballman, Mordante, rtrieu

Reviewed By: aaron.ballman

Differential Revision: https://reviews.llvm.org/D75613
2020-03-06 14:57:01 +01:00
Mark de Wever c03349e40f [Sema] Remove a -Wrange warning from -Wall
During the review of D73007 Aaron Puchert mentioned
`warn_for_range_variable_always_copy` shouldn't be part of -Wall since
some coding styles require `for(const auto &bar : bars)`. This warning
would cause false positives for these users. Based on Aaron's proposal
refactored the warnings:

* -Wrange-loop-construct warns about possibly unintended constructor
  calls. This is part of -Wall. It contains
  * warn_for_range_copy: loop variable A of type B creates a copy from
    type C
  * warn_for_range_const_reference_copy: loop variable A is initialized
    with a value of a different type resulting in a copy
* -Wrange-loop-bind-reference warns about misleading use of reference
  types. This is not part of -Wall. It contains
  * warn_for_range_variable_always_copy: loop variable A is always a copy
    because the range of type B does not return a reference

Differential Revision: https://reviews.llvm.org/D73434
2020-02-01 18:44:27 +01:00
Mark de Wever 41fcd17250 [Sema] Avoid Wrange-loop-analysis false positives
When Wrange-loop-analysis issues a diagnostic on a dependent type in a
template the diagnostic may not be valid for all instantiations. Therefore
the diagnostic is suppressed during the instantiation. Non dependent types
still issue a diagnostic.

The same can happen when using macros. Therefore the diagnostic is
disabled for macros.

Fixes https://bugs.llvm.org/show_bug.cgi?id=44556

Differential Revision: https://reviews.llvm.org/D73007
2020-01-21 21:14:10 +01:00
Mark de Wever 9c74fb402e [Sema] Improve -Wrange-loop-analysis warnings.
No longer generate a diagnostic when a small trivially copyable type is
used without a reference. Before the test looked for a POD type and had no
size restriction. Since the range-based for loop is only available in
C++11 and POD types are trivially copyable in C++11 it's not required to
test for a POD type.

Since copying a large object will be expensive its size has been
restricted. 64 bytes is a common size of a cache line and if the object is
aligned the copy will be cheap. No performance impact testing has been
done.

Differential Revision: https://reviews.llvm.org/D72212
2020-01-11 15:34:02 +01:00
Mark de Wever eec0240f97 Adds -Wrange-loop-analysis to -Wall
This makes the range loop warnings part of -Wall.

Fixes PR32823: Warn about accidental coping of data in range based for

Differential Revision: https://reviews.llvm.org/D68912

Recomitted after fixing the warnings it created.
2020-01-06 17:37:47 +01:00
Mark de Wever 8ca79dac55 Revert "Adds -Wrange-loop-analysis to -Wall"
The sanitizer-x86_64-linux buildbot failed to build lld with -Werror.

This reverts commit d8117542ac.
2020-01-01 22:19:18 +01:00
Mark de Wever d8117542ac Adds -Wrange-loop-analysis to -Wall
This makes the range loop warnings part of -Wall.

Fixes PR32823: Warn about accidental coping of data in range based for

Differential Revision: https://reviews.llvm.org/D68912
2020-01-01 20:02:42 +01:00
Mark de Wever e5ab1e49f9 Improve Wrange-loop-analyses for rvalue reference
The Wrange-loop-analyses warns if a copy is made. Suppress this warning when
a temporary is bound to a rvalue reference.

While fixing this issue also found a copy-paste error in test6, which is also
fixed.

Differential Revision: https://reviews.llvm.org/D71806
2020-01-01 20:02:18 +01:00
Mark de Wever f022a5a792 Adds fixit hints to the -Wrange-loop-analysis
Differential Revision: https://reviews.llvm.org/D68913
2020-01-01 20:02:00 +01:00
Nico Weber 8be5d034e4 fix typo "varaible"
llvm-svn: 279639
2016-08-24 16:37:21 +00:00
Richard Trieu 3e1d483e0e Add new warning -Wrange-loop-analysis to warn on copies during loops.
-Wrange-loop-analysis is a subgroup of -Wloop-analysis and will warn when
a range-based for-loop makes copies of the elements in the range.  If possible,
suggest the proper type to prevent copies, or the non-reference to help
distinguish copy versus non-copy forms.  Existing warnings in -Wloop-analysis
are moved to -Wfor-loop-analysis, also a subgroup of -Wloop-analysis.

Differential Revision: http://reviews.llvm.org/D4169

llvm-svn: 234804
2015-04-13 22:08:55 +00:00