aaronpuchert added a comment.
I've been thinking about the warning messages, which seem to a bit inaccurate.
Sorry for piggy-backing this onto the change, I hope you don't mind.
================
Comment at: clang/lib/Sema/SemaStmt.cpp:2758-2759
// the correct time to bind a const reference.
SemaRef.Diag(VD->getLocation(), diag::warn_for_range_const_reference_copy)
<< VD << VariableType << E->getType();
QualType NonReferenceType = VariableType.getNonReferenceType();
----------------
The message here says: "loop variable A has type B but is initialized with type
C resulting in a copy".
That's not necessarily true, we just know (I think) that a constructor is
called. The constructor might copy, but it might also not. However, the
constructed object is bound to a reference, which is potentially misleading.
One writes `const X& x : range` and `x` doesn't bind to the return value of
`*begin(range)`, but to a temporary constructed from that.
Perhaps a more accurate message would be: "loop variable A of type B binds to a
temporary constructed from type C" with the appropriate types.
================
Comment at: clang/lib/Sema/SemaStmt.cpp:2772-2773
// semantic of the code.
SemaRef.Diag(VD->getLocation(), diag::warn_for_range_variable_always_copy)
<< VD << RangeInitType;
QualType NonReferenceType = VariableType.getNonReferenceType();
----------------
The message here is: "loop variable A is always a **copy** because the range of
type B does not return a reference". This is somewhat misleading, some ranges
create objects on-the-fly, in which case these aren't copies. (Assuming RVO the
copy constructor might have never been called.)
I would suggest to rephrase this as something like "loop variable A binds to a
temporary value produced by a range of type B".
================
Comment at: clang/lib/Sema/SemaStmt.cpp:2823-2824
// if doing so will prevent a copy.
SemaRef.Diag(VD->getLocation(), diag::warn_for_range_copy)
<< VD << VariableType << InitExpr->getType();
SemaRef.Diag(VD->getBeginLoc(), diag::note_use_reference_type)
----------------
Given that `VariableType == VD->getType()` and `InitExpr->getType() ==
VD->getInit()->getType()`, can these two types ever be different other than
cv-qualifiers? We only call this function if `VariableType` is not a reference
type.
Which begs the question: which value provides C in "loop variable A of type B
creates a copy from type C"? I would suggest to omit the second type and write:
"loop variable A creates a copy of type B", perhaps with stripping const from B.
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
