Should we enable -Wrange-loop-analysis? (Was: [llvm] r261524 - Fix some abuse of auto...)

This is a pretty nice warning. Should we enable it for LLVM's build when
the host compiler supports it?

Benjamin Kramer via llvm-commits <llvm-commits@lists.llvm.org> writes:

What does this warning look like (and what cases does it catch)? Also does it have false positive?

Mehdi Amini <mehdi.amini@apple.com> writes:

What does this warning look like (and what cases does it catch)? Also
does it have false positive?

I ran it on LLVM and clang and it only caught real (albeit minor)
problems.

Here, someone wrote & instead of * by mistake:

    SelectionDAGBuilder.cpp:2382:22: error: loop variable 'U' is always a copy because the range of type 'llvm::iterator_range<llvm::Value::user_iterator_impl<const llvm::User> >' does not return a reference [-Werror,-Wrange-loop-analysis]
        for (const auto &U : User->users()) {
                         ^
    SelectionDAGBuilder.cpp:2382:10: note: use non-reference type 'const llvm::User *'
        for (const auto &U : User->users()) {
             ^~~~~~~~~~~~~~~

Here, we're copying this struct for no good reason (someone missed a *):

    LoopLoadElimination.cpp:439:47: error: loop variable 'Cand' of type 'const (anonymous namespace)::StoreToLoadForwardingCandidate' creates a copy from type 'const (anonymous namespace)::StoreToLoadForwardingCandidate' [-Werror,-Wrange-loop-analysis]
        for (const StoreToLoadForwardingCandidate Cand : StoreToLoadDependences) {
                                                  ^
    LoopLoadElimination.cpp:439:10: note: use reference type 'const (anonymous namespace)::StoreToLoadForwardingCandidate &' to prevent copying
        for (const StoreToLoadForwardingCandidate Cand : StoreToLoadDependences) {
             ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~

As it fires only on "const ref" and not on value type, it shouldn't have much false positive.
I'd be in favor of enabling it in Clang/LLVM codebase by default.