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

+llvmdev

On Mar 7, 2014, at 5:15 AM, Tobias Grosser <tobias at grosser.es> wrote:
> On 03/07/2014 01:40 PM, Aaron Ballman wrote:
>> On Thu, Mar 6, 2014 at 7:12 PM, Aaron Ballman <aaron at
aaronballman.com> wrote:
>>> On Thu, Mar 6, 2014 at 7:10 PM, Tobias Grosser <tobias at
grosser.es> wrote:
>>>> 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. :-)
>> 
>> 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?

On Fri, Mar 7, 2014 at 8:58 AM, Duncan P. N. Exon Smith
<dexonsmith at apple.com> wrote:
> +llvmdev
>
> On Mar 7, 2014, at 5:15 AM, Tobias Grosser <tobias at grosser.es>
wrote:
>
>> On 03/07/2014 01:40 PM, Aaron Ballman wrote:
>>> On Thu, Mar 6, 2014 at 7:12 PM, Aaron Ballman <aaron at
aaronballman.com> wrote:
>>>> On Thu, Mar 6, 2014 at 7:10 PM, Tobias Grosser <tobias at
grosser.es> wrote:
>>>>> 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. :-)
>>>
>>> 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.

On Mar 7, 2014, at 9:15 AM, David Blaikie <dblaikie at gmail.com> wrote:
> On Fri, Mar 7, 2014 at 8:58 AM, Duncan P. N. Exon Smith
> <dexonsmith at apple.com> wrote:
>> 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.

On Fri, Mar 7, 2014 at 12:15 PM, David Blaikie <dblaikie at gmail.com>
wrote:
> On Fri, Mar 7, 2014 at 8:58 AM, Duncan P. N. Exon Smith
> <dexonsmith at apple.com> wrote:
>> +llvmdev
>>
>> On Mar 7, 2014, at 5:15 AM, Tobias Grosser <tobias at grosser.es>
wrote:
>>
>>> On 03/07/2014 01:40 PM, Aaron Ballman wrote:
>>>> On Thu, Mar 6, 2014 at 7:12 PM, Aaron Ballman <aaron at
aaronballman.com> wrote:
>>>>> On Thu, Mar 6, 2014 at 7:10 PM, Tobias Grosser <tobias
at grosser.es> wrote:
>>>>>> 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. :-)
>>>>
>>>> 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.
Thanks for the clarity on this -- I'll start switching over to using *
instead of just auto as I range-ify things.

~Aaron