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
_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
