[RFC] Relax the rules on const auto&? (was Re: r203179 - [C++11] Replacing iterators redecls_begin() and redecls_end() ...)

+llvmdev

I wonder if you could use 'auto const &' in some of these cases?

http://llvm.org/docs/CodingStandards.html#beware-unnecessary-copies-with-auto

I'll look into it once I've gotten the build bots green again. Thank
you for the reminder. :slight_smile:

I looked into this a bit more, and I'm not certain there's any real
benefit. Everything returned from these containers is already a
pointer, so this would change them from const Decl * to const Decl *&,
which doesn't strike me as any better (or worse). So I am thinking I
will continue without the const auto & in these cases (and as I move
other pointer-based ranges forward), but thank you for the reminder!

I follow your reasoning. It seems the coding standards are formulated a little to strictly. Duncan, would it make sense to put an exception
for cases like this into the standard. It seems to be a very common case in fact.

Unless the pointers themselves are changing, the only benefit here is
consistency. But ranges of iterators should use const&, since many
iterators are expensive to copy.

Accidental copies can be a huge headache, since they lead to subtle
bugs that are hard to track down (performance implications aside).

I still favour applying the rule generally, since consistency matters.
But we could text like:

    Pointer ranges are a common exception.

Any other opinions?

+llvmdev

I wonder if you could use 'auto const &' in some of these cases?

http://llvm.org/docs/CodingStandards.html#beware-unnecessary-copies-with-auto

I'll look into it once I've gotten the build bots green again. Thank
you for the reminder. :slight_smile:

I looked into this a bit more, and I'm not certain there's any real
benefit. Everything returned from these containers is already a
pointer, so this would change them from const Decl * to const Decl *&,
which doesn't strike me as any better (or worse). So I am thinking I
will continue without the const auto & in these cases (and as I move
other pointer-based ranges forward), but thank you for the reminder!

I follow your reasoning. It seems the coding standards are formulated a little to strictly. Duncan, would it make sense to put an exception
for cases like this into the standard. It seems to be a very common case in fact.

Unless the pointers themselves are changing, the only benefit here is
consistency. But ranges of iterators should use const&, since many
iterators are expensive to copy.

To clarify, this isn't about copying iterators (iterators are
generally cheap to copy) but copy the elements being iterated over.

Accidental copies can be a huge headache, since they lead to subtle
bugs that are hard to track down (performance implications aside).

I still favour applying the rule generally, since consistency matters.
But we could text like:

    Pointer ranges are a common exception.

Any other opinions?

If you write the exception as "auto *x" then it's unambiguous and easy
to still look at "auto x" as being questionable/to-be-avoided.

Unless the pointers themselves are changing, the only benefit here is
consistency. But ranges of iterators should use const&, since many
iterators are expensive to copy.

To clarify, this isn't about copying iterators

You’re right, that was a tangent.

(iterators are
generally cheap to copy) but copy the elements being iterated over.

The point (of my tangent) is that some iterators (scc_iterator,
po_iterator, etc.) have non-trivial state, so copies can be expensive.

Accidental copies can be a huge headache, since they lead to subtle
bugs that are hard to track down (performance implications aside).

I still favour applying the rule generally, since consistency matters.
But we could text like:

   Pointer ranges are a common exception.

Any other opinions?

If you write the exception as "auto *x" then it's unambiguous and easy
to still look at "auto x" as being questionable/to-be-avoided.

This is obviously better. Changed in r203254.

Thanks for the clarity on this -- I'll start switching over to using *
instead of just auto as I range-ify things.

~Aaron