[PATCH] D73007: [Sema] Avoid Wrange-loop-analysis false positives

Aaron Puchert via Phabricator via cfe-commits cfe-commits at lists.llvm.org
Tue Jan 21 12:23:38 PST 2020


aaronpuchert added a comment.

I also still think that `warn_for_range_copy` //should be// emitted even in templates.

These aren't false positives in my opinion, especially since we're now pretty conservative about that warning.



================
Comment at: clang/lib/Sema/SemaStmt.cpp:2703-2704
 // suggest the const reference type that would do so.
 // For instance, given "for (const &Foo : Range)", suggest
 // "for (const Foo : Range)" to denote a copy is made for the loop.  If
 // possible, also suggest "for (const &Bar : Range)" if this type prevents
----------------
Mordante wrote:
> aaronpuchert wrote:
> > That's the part the bothers me.
> I think we shouldn't remove the warning, it is useful. We can consider to move this out of -Wall, but I first like to hear the opinion of the other reviewers in this matter. If we want this change I'll be happy to create a new patch.
I don't see a problem with having this as an optional warning that can be turned on for those who want it.

Note that this isn't my personal opinion, I'm actually fine with it. But I've met considerable resistance to turning the warning on before, and it's somewhat hard to argue against that. Binding references to temporaries is perfectly normal C++, and we don't discourage it anywhere else.


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D73007/new/

https://reviews.llvm.org/D73007





More information about the cfe-commits mailing list