Logo Questions Linux Laravel Mysql Ubuntu Git Menu
 

What is clang's 'range-loop-analysis' diagnostic about?

Background:

Consider the following example:

#include <iostream>
#include <vector>

int main() {
    std::vector<bool> vectorBool{false, true};
    for(const auto &element : vectorBool) std::cout << std::boolalpha << element << ' ';
    return 0;
}

It emits the warning:

test.cpp:6:21: warning: loop variable 'element' is always a copy because the range of type 'std::vector<bool>' does not return a reference [-Wrange-loop-analysis]
    for(const auto &element : vectorBool) std::cout << std::boolalpha << element << ' ';
                    ^
test.cpp:6:9: note: use non-reference type 'std::_Bit_reference'
    for(const auto &element : vectorBool) std::cout << std::boolalpha << element << ' ';
        ^~~~~~~~~~~~~~~~~~~~~
1 warning generated.

When compiled using clang with the range-loop-analysis diagnostic enabled:

$ clang++ -Wrange-loop-analysis -o test test.cpp

Questions:

According to https://reviews.llvm.org/D4169, the warning emits when:

for (const Foo &x : Foos), where the range Foos only return a copy. Suggest using the non-reference type so the copy is obvious

I completely understand that std::vector<bool>'s iterators return a copy of a proxy type (instead of a reference), but I disagree with the statement "so the copy is obvious":

  1. Where exactly is that "hidden" copy operation taking place? As far as I can see, we are just binding a reference to a temporary object, and this should prolong the lifetime of the temporary to match that of the reference.
  2. Even in case we have written for(const auto element : vectorBool) (so that the warning disappears), We should have no copy/move operations under C++17's guaranteed copy elision rules (and even in pre-C++17 when using any decent compiler), so is this warning about making an elided copy operation obvious?!
like image 388
Mike Avatar asked Apr 27 '18 15:04

Mike


1 Answers

In C++17 a range based for loop is defined as

{
    auto && __range = range_expression ; 
    auto __begin = begin_expr ;
    auto __end = end_expr ;
    for ( ; __begin != __end; ++__begin) { 
        range_declaration = *__begin; 
        loop_statement 
    } 
}

And

range_declaration = *__begin;

Is the point where the range variable is initialized. Typically *__begin returns a reference so in

for (const auto& e : range_that_returns_references)

e can be eliminated and we can just work with the element from the range. In

for (const auto& e : range_that_returns_proxies_or_copies)

e can't be eleminated. *__begin will create a proxy or copy and then we bind that temporary to e. That means in every iteration you have an object that is being created and destroyed, which could be costly, and is not obvious as you are using a reference. The warning wants you to use a non reference type to make it obvious that you are not actually working with the element from the range but instead a copy/proxy of it.

like image 69
NathanOliver Avatar answered Nov 09 '22 08:11

NathanOliver