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

[Aaron Puchert via Phabricator via cfe-commits]

aaronpuchert added a comment.

In D73007#1829709 <https://reviews.llvm.org/D73007#1829709>, @xbolva00 wrote:

> Yes, but minimal fix is better for release branch, so @hans should merge it.


I think `-Wall` shouldn't warn about reference types in a range-based for loop (unless it's the wrong type and inadvertently creates a copy).

> Handling your case probably needs more work and patches..

See my inline comments. The bug in itself is not terribly troublesome, this issue has existed in many previous releases. The problem is that the warning is now in `-Wall`, and I think we should be more conservative in `-Wall`.



================
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
----------------
That's the part the bothers me.


================
Comment at: clang/lib/Sema/SemaStmt.cpp:2768-2778
     // The range always returns a copy, so a temporary is always created.
     // Suggest removing the reference from the loop variable.
     // If the type is a rvalue reference do not warn since that changes the
     // semantic of the code.
     SemaRef.Diag(VD->getLocation(), diag::warn_for_range_variable_always_copy)
         << VD << RangeInitType;
     QualType NonReferenceType = VariableType.getNonReferenceType();
----------------
We could either remove this part or group `warn_for_range_variable_always_copy` under a separate flag that's not in `-Wall`.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D73007