generally prefer references in range-based for loops

[Walt Karas]

This line of code:
https://github.com/apache/trafficserver/blob/master/iocore/net/SSLSNIConfig.cc#L59

should be:
for (const auto &item : Y_sni.items) {

This causes item to be of type const Item & instead of Item. Thus
avoiding making a copy of each element in the vector. Look at this
example code:

https://godbolt.org/z/7j0LFT

Note in the assembler code how foo() calls the copy constructor for
Item in the for loop, and foo2() does not.

[Walt Karas]

https://github.com/apache/trafficserver/blob/master/proxy/logging/LogObject.cc#L1008

We also have cases of the opposite problem, using a reference for a
container of pointers / iterators / string_views or other small
objects. But this is less serious and probably just an aesthetic
issue (gets optimized out).

Good catch. In Modern Effective C++ Scott Myers talks about a few scenarios when auto doesn't do what the programmer intended.
Earlier I was inclined to suggest that we should avoid auto in ATS but I think it's better to make mistakes and learn.

Post by Walt Karas
https://github.com/apache/trafficserver/blob/master/iocore/net/SSLSNIConfig.cc#L59
for (const auto &item : Y_sni.items) {
This causes item to be of type const Item & instead of Item. Thus
avoiding making a copy of each element in the vector. Look at this
https://godbolt.org/z/7j0LFT
Note in the assembler code how foo() calls the copy constructor for
Item in the for loop, and foo2() does not.

--
pushkar

[Alan Carroll]

I tend to use `for ( auto && x : container)`. That seems to work well and
gives the compiler more flexibility. I recommend it unless there's a
specific reason to not do so. Testing with a std::vector<int> and a
function that takes (int) it does the correct thing - it passes the int
values directly to the function.

Post by Walt Karas
https://github.com/apache/trafficserver/blob/master/proxy/logging/LogObject.cc#L1008
We also have cases of the opposite problem, using a reference for a
container of pointers / iterators / string_views or other small
objects. But this is less serious and probably just an aesthetic
issue (gets optimized out).

Good catch. In Modern Effective C++ Scott Myers talks about a few

scenarios when auto doesn't do what the programmer intended.

Earlier I was inclined to suggest that we should avoid auto in ATS but I

think it's better to make mistakes and learn.
https://github.com/apache/trafficserver/blob/master/iocore/net/SSLSNIConfig.cc#L59

Post by Walt Karas
for (const auto &item : Y_sni.items) {
This causes item to be of type const Item & instead of Item. Thus
avoiding making a copy of each element in the vector. Look at this
https://godbolt.org/z/7j0LFT
Note in the assembler code how foo() calls the copy constructor for
Item in the for loop, and foo2() does not.

--
pushkar

[Walt Karas]

I'm not convinced: https://godbolt.org/z/M8pZl0

On Mon, Oct 29, 2018 at 1:48 PM Alan Carroll

Post by Alan Carroll
I tend to use `for ( auto && x : container)`. That seems to work well and
gives the compiler more flexibility. I recommend it unless there's a
specific reason to not do so. Testing with a std::vector<int> and a
function that takes (int) it does the correct thing - it passes the int
values directly to the function.

Post by Walt Karas
https://github.com/apache/trafficserver/blob/master/proxy/logging/LogObject.cc#L1008
We also have cases of the opposite problem, using a reference for a
container of pointers / iterators / string_views or other small
objects. But this is less serious and probably just an aesthetic
issue (gets optimized out).

Good catch. In Modern Effective C++ Scott Myers talks about a few

scenarios when auto doesn't do what the programmer intended.

Earlier I was inclined to suggest that we should avoid auto in ATS but I

think it's better to make mistakes and learn.
https://github.com/apache/trafficserver/blob/master/iocore/net/SSLSNIConfig.cc#L59

Post by Walt Karas
for (const auto &item : Y_sni.items) {
This causes item to be of type const Item & instead of Item. Thus
avoiding making a copy of each element in the vector. Look at this
https://godbolt.org/z/7j0LFT
Note in the assembler code how foo() calls the copy constructor for
Item in the for loop, and foo2() does not.

--
pushkar

--
*Beware the fisherman who's casting out his line in to a dried up riverbed.*
*Oh don't try to tell him 'cause he won't believe. Throw some bread to the
ducks instead.*
*It's easier that way. *- Genesis : Duke : VI 25-28

[Pushkar Pradhan]

It seems like auto isn't that "auto". You still need to be familiar with
all the gotchas.


On Mon, Oct 29, 2018 at 11:48 AM Alan Carroll

Post by Alan Carroll
I tend to use `for ( auto && x : container)`. That seems to work well and
gives the compiler more flexibility. I recommend it unless there's a
specific reason to not do so. Testing with a std::vector<int> and a
function that takes (int) it does the correct thing - it passes the int
values directly to the function.
https://github.com/apache/trafficserver/blob/master/proxy/logging/LogObject.cc#L1008

Post by Walt Karas
We also have cases of the opposite problem, using a reference for a
container of pointers / iterators / string_views or other small
objects. But this is less serious and probably just an aesthetic
issue (gets optimized out).

Good catch. In Modern Effective C++ Scott Myers talks about a few

scenarios when auto doesn't do what the programmer intended.

Earlier I was inclined to suggest that we should avoid auto in ATS but

I

Post by Walt Karas
think it's better to make mistakes and learn.

https://github.com/apache/trafficserver/blob/master/iocore/net/SSLSNIConfig.cc#L59

Post by Walt Karas

Post by Walt Karas
for (const auto &item : Y_sni.items) {
This causes item to be of type const Item & instead of Item. Thus
avoiding making a copy of each element in the vector. Look at this
https://godbolt.org/z/7j0LFT
Note in the assembler code how foo() calls the copy constructor for
Item in the for loop, and foo2() does not.

--
pushkar

--
*Beware the fisherman who's casting out his line in to a dried up riverbed.*
*Oh don't try to tell him 'cause he won't believe. Throw some bread to the
ducks instead.*
*It's easier that way. *- Genesis : Duke : VI 25-28

[Walt Karas]

Yes and no. I think Alan is correct in the sense that if you use for
(auto &&r : , and it compiles OK, the object code is going to be
correct and optimal. I just have my aesthetic opinion that it can
make the code slightly harder to maintain.
On Mon, Oct 29, 2018 at 6:47 PM Pushkar Pradhan

Post by Pushkar Pradhan
It seems like auto isn't that "auto". You still need to be familiar with
all the gotchas.
On Mon, Oct 29, 2018 at 11:48 AM Alan Carroll

Post by Alan Carroll
I tend to use `for ( auto && x : container)`. That seems to work well and
gives the compiler more flexibility. I recommend it unless there's a
specific reason to not do so. Testing with a std::vector<int> and a
function that takes (int) it does the correct thing - it passes the int
values directly to the function.
https://github.com/apache/trafficserver/blob/master/proxy/logging/LogObject.cc#L1008

Post by Walt Karas
We also have cases of the opposite problem, using a reference for a
container of pointers / iterators / string_views or other small
objects. But this is less serious and probably just an aesthetic
issue (gets optimized out).

Good catch. In Modern Effective C++ Scott Myers talks about a few

scenarios when auto doesn't do what the programmer intended.

Earlier I was inclined to suggest that we should avoid auto in ATS but

I

Post by Walt Karas
think it's better to make mistakes and learn.

https://github.com/apache/trafficserver/blob/master/iocore/net/SSLSNIConfig.cc#L59

Post by Walt Karas

Post by Walt Karas
for (const auto &item : Y_sni.items) {
This causes item to be of type const Item & instead of Item. Thus
avoiding making a copy of each element in the vector. Look at this
https://godbolt.org/z/7j0LFT
Note in the assembler code how foo() calls the copy constructor for
Item in the for loop, and foo2() does not.

--
pushkar

--
*Beware the fisherman who's casting out his line in to a dried up riverbed.*
*Oh don't try to tell him 'cause he won't believe. Throw some bread to the
ducks instead.*
*It's easier that way. *- Genesis : Duke : VI 25-28

--
pushkar