================
Comment at: test/SemaCXX/warn-range-loop-analysis.cpp:41
@@ +40,3 @@
+
+ for (const int x : int_ref_container) {}
+ // expected-warning@-1 {{loop variable 'x' of type 'const int' creates a
copy from type 'int'}}
----------------
David Blaikie wrote:
> Richard Trieu wrote:
> > David Blaikie wrote:
> > > I think this is going to have a pretty high false positive rate... do you
> > > have some numbers?
> > What is your idea of a false positive?
> Users are taught to pass ints to functions by value, rather than reference,
> to avoid the indirection overhead. The same logic applies (to a degree) to
> for loops. It's not unreasonable to write "for (const int i :
> my_vec_of_ints)".
>
> I don't think this case is likely to be a good thing to warn on and we may
> need a heuristic of "if a type is 'large' enough (or has a non-trivial copy),
> warn when a copy is made". StringRef would be a similar situation - I can
> imagine lots of people (I bet we've got a few in Clang/LLVM already) looping
> over StringRef's by value.
>
> Do you have some stats on running this warning in its current form over
> LLVM/Clang? (and Google, for that matter, or any other substantial codebase).
>
> Looking at other test cases, this wouldn't warn if the "const" was removed?
> Perhaps we just need a fix to disregard cv qualifiers, then?
LLVM/Clang has a total of 5 warnings emitted for this warning. (Where changing
the variable to reference type will prevent a copy.) Two are pairs from maps,
one is a CXXBaseSpecifier, one is QualType, and one is a ParmVarDecl pointer.
From your description, the QualType and Decl pointer are both small and
trivially copyable so they should be exempt from this warning, so a 40% drop in
cases with your suggested filter.
Applying the same filter to results from Google, the drop is closer to 70%.
The filter is to ignore trivially copyable types which are 2x pointer size or
smaller.
The const is a good indication that the variable is read-only, so we can
suggest using a const-reference as the cheaper option. Without the const, we
don't know if the use in the loop was intended only for the loop copy or to
change the object in the container as well.
http://reviews.llvm.org/D4169
_______________________________________________
cfe-commits mailing list
[email protected]
http://lists.cs.uiuc.edu/mailman/listinfo/cfe-commits
