llvm dev - May 2014 - [LLVMdev] Guidance on using pointers vs. references for function arguments

On May 26, 2014, at 5:02 PM, Chandler Carruth <chandlerc at google.com>
wrote:
> 
> On Mon, May 26, 2014 at 4:43 PM, Andrew Trick <atrick at apple.com>
wrote:
> This has been discussed before but I can’t find a reference to it. I could
have sworn this was in the coding convention at some point. Here’s what I
remember: during early LLVM development there was an effort to establish the
convention that you described above—use pointer types only when nullptr is
valid. This led to a lot of redundant declarations and annoying taking of
addresses and dereferences. It turns out that the convention doesn’t really help
for most informal/internal APIs. It’s actually no harder to debug a SIGSEGV than
a nullptr check. I also adhered to this convention in a previous project and it
never paid off.
> 
> Once you begin working on a piece of code you get a feel for which types
should be passed as pointers and which should be passed as reference. Then you
try to pass types consistently regardless of whether a null input is valid. For
example, some types, like the current context, should never be copied or passed
by value and are obviously not null. That’s lower overhead in practice forcing
callers to convert to a reference whenever we want to skip a null check.
This last sentence should read: it’s lower mental overhead for the programmer to
use the same type consistently rather than worrying about another convention to
follow at every call.

I would personally be happy to follow the pointer may be nullptr convention if
it were used consistently. I was just trying to reiterate arguments against it
that I’d seen w.r.t LLVM codebase, and I don’t see much value in forcing the
convention everywhere.

-Andy
> FWIW, I disagree.
> 
> I much prefer to pass by reference unless there is the expectation of null
inputs. I have never really agreed with the complaints about taking the address
and have never found it to be a burden. I also find the simplicity of a
consistent rule far more appealing than "getting a feel for which
types" should be passed as pointers.
> 
> Ironically, using a reference can result in better optimizations by
deleting redundant null checks. While I certainly hope this isn't relevant
to the performance of LLVM, it's still not something to completely
disregard.
> 
> Anyways, I've never really seen this become a problem in practice.
I'm pretty happy for folks to use whatever local conventions they want.
I'm usually happy to switch from reference to pointer if I'm hacking
some part of the codebase I don't usually touch and one of the maintainers
really prefers one over the other. It's not a big deal either way.

-------------- next part --------------
An HTML attachment was scrubbed...
URL:
<http://lists.llvm.org/pipermail/llvm-dev/attachments/20140526/48622133/attachment.html>

On Mon, May 26, 2014 at 7:41 PM, Andrew Trick <atrick at apple.com>
wrote:
>
> On May 26, 2014, at 5:02 PM, Chandler Carruth <chandlerc at
google.com> wrote:
>
>
> On Mon, May 26, 2014 at 4:43 PM, Andrew Trick <atrick at apple.com>
wrote:
>>
>> This has been discussed before but I can’t find a reference to it. I
could
>> have sworn this was in the coding convention at some point. Here’s what
I
>> remember: during early LLVM development there was an effort to
establish the
>> convention that you described above—use pointer types only when nullptr
is
>> valid. This led to a lot of redundant declarations and annoying taking
of
>> addresses and dereferences. It turns out that the convention doesn’t
really
>> help for most informal/internal APIs. It’s actually no harder to debug
a
>> SIGSEGV than a nullptr check. I also adhered to this convention in a
>> previous project and it never paid off.
>>
>> Once you begin working on a piece of code you get a feel for which
types
>> should be passed as pointers and which should be passed as reference.
Then
>> you try to pass types consistently regardless of whether a null input
is
>> valid. For example, some types, like the current context, should never
be
>> copied or passed by value and are obviously not null. That’s lower
overhead
>> in practice forcing callers to convert to a reference whenever we want
to
>> skip a null check.
>
>
> This last sentence should read: it’s lower mental overhead for the
> programmer to use the same type consistently rather than worrying about
> another convention to follow at every call.
>
> I would personally be happy to follow the pointer may be nullptr convention
> if it were used consistently. I was just trying to reiterate arguments
> against it that I’d seen w.r.t LLVM codebase, and I don’t see much value in
> forcing the convention everywhere.
Yeah, there's certainly some local consistency that's not worth
overriding just yet - but we do the same thing with function names,
for example (we've still got lots of functions named UpperFirst, try
to keep a single class's member functions consistent amongst
themselves, but when creating new classes we prefer the new convention
of lowerFirst).

But when given the choice (because there's no local consistency to
worry about) I'll pass by reference where possible & suggest others do
the same - but I'm not fussed enough to push this to be in the LLVM
Style guide. Won't complain if it ends up there either, though :)

- David
>
> -Andy
>
> FWIW, I disagree.
>
> I much prefer to pass by reference unless there is the expectation of null
> inputs. I have never really agreed with the complaints about taking the
> address and have never found it to be a burden. I also find the simplicity
> of a consistent rule far more appealing than "getting a feel for which
> types" should be passed as pointers.
>
> Ironically, using a reference can result in better optimizations by
deleting
> redundant null checks. While I certainly hope this isn't relevant to
the
> performance of LLVM, it's still not something to completely disregard.
>
> Anyways, I've never really seen this become a problem in practice.
I'm
> pretty happy for folks to use whatever local conventions they want. I'm
> usually happy to switch from reference to pointer if I'm hacking some
part
> of the codebase I don't usually touch and one of the maintainers really
> prefers one over the other. It's not a big deal either way.
>
>
>
>
> _______________________________________________
> LLVM Developers mailing list
> LLVMdev at cs.uiuc.edu         http://llvm.cs.uiuc.edu
> http://lists.cs.uiuc.edu/mailman/listinfo/llvmdev
>

On Mon, May 26, 2014 at 7:41 PM, Andrew Trick <atrick at apple.com> wrote:
> On May 26, 2014, at 5:02 PM, Chandler Carruth <chandlerc at
google.com>
> wrote:
>
>
> On Mon, May 26, 2014 at 4:43 PM, Andrew Trick <atrick at apple.com>
wrote:
>
>> This has been discussed before but I can’t find a reference to it. I
>> could have sworn this was in the coding convention at some point.
Here’s
>> what I remember: during early LLVM development there was an effort to
>> establish the convention that you described above—use pointer types
only
>> when nullptr is valid. This led to a lot of redundant declarations and
>> annoying taking of addresses and dereferences. It turns out that the
>> convention doesn’t really help for most informal/internal APIs. It’s
>> actually no harder to debug a SIGSEGV than a nullptr check. I also
adhered
>> to this convention in a previous project and it never paid off.
>>
>> Once you begin working on a piece of code you get a feel for which
types
>> should be passed as pointers and which should be passed as reference.
Then
>> you try to pass types consistently regardless of whether a null input
is
>> valid. For example, some types, like the current context, should never
be
>> copied or passed by value and are obviously not null. That’s lower
overhead
>> in practice forcing callers to convert to a reference whenever we want
to
>> skip a null check.
>>
>
> This last sentence should read: it’s lower mental overhead for the
> programmer to use the same type consistently rather than worrying about
> another convention to follow at every call.
>
> I would personally be happy to follow the pointer may be nullptr
> convention if it were used consistently. I was just trying to reiterate
> arguments against it that I’d seen w.r.t LLVM codebase, and I don’t see
> much value in forcing the convention everywhere.
>
Certainly.

My suggestion: unless we have evidence this is confusing people trying to
contribute to LLVM, then it isn't broken as is.

If folks are confused by this and it is becoming a neusance, then I would
suggest we draw a line in the sand much like we did with function names.
New code gets the new rule, existing code (and code with existing
conventions surrounding it) isn't impacted. My suggested rule would be
"use
a pointer if it might be null or re-pointed at some other entity, otherwise
use a reference". But I don't think adding that kind of rule is
necessary
unless folks are frequently tripped up by this in reviews, etc.

-Chandler
-------------- next part --------------
An HTML attachment was scrubbed...
URL:
<http://lists.llvm.org/pipermail/llvm-dev/attachments/20140526/f9da5537/attachment.html>

On May 26, 2014, at 8:21 PM, Chandler Carruth <chandlerc at google.com>
wrote:
> 
> On Mon, May 26, 2014 at 7:41 PM, Andrew Trick <atrick at apple.com>
wrote:
> On May 26, 2014, at 5:02 PM, Chandler Carruth <chandlerc at
google.com> wrote:
> 
>> 
>> On Mon, May 26, 2014 at 4:43 PM, Andrew Trick <atrick at
apple.com> wrote:
>> This has been discussed before but I can’t find a reference to it. I
could have sworn this was in the coding convention at some point. Here’s what I
remember: during early LLVM development there was an effort to establish the
convention that you described above—use pointer types only when nullptr is
valid. This led to a lot of redundant declarations and annoying taking of
addresses and dereferences. It turns out that the convention doesn’t really help
for most informal/internal APIs. It’s actually no harder to debug a SIGSEGV than
a nullptr check. I also adhered to this convention in a previous project and it
never paid off.
>> 
>> Once you begin working on a piece of code you get a feel for which
types should be passed as pointers and which should be passed as reference. Then
you try to pass types consistently regardless of whether a null input is valid.
For example, some types, like the current context, should never be copied or
passed by value and are obviously not null. That’s lower overhead in practice
forcing callers to convert to a reference whenever we want to skip a null check.
> 
> This last sentence should read: it’s lower mental overhead for the
programmer to use the same type consistently rather than worrying about another
convention to follow at every call.
> 
> I would personally be happy to follow the pointer may be nullptr convention
if it were used consistently. I was just trying to reiterate arguments against
it that I’d seen w.r.t LLVM codebase, and I don’t see much value in forcing the
convention everywhere.
> 
> Certainly.
> 
> My suggestion: unless we have evidence this is confusing people trying to
contribute to LLVM, then it isn't broken as is.
Chandler, Andy's recollection is correct.

I was personally infatuated by this idea and pushed it through a ton of the LLVM
IR APIs (this was many years ago, (in the pre-1.0 days, and then again early in
Clang's development because I'm a slow learner).  It led to all sorts of
weird cases where some arguments to IR objects would be taken by reference and
some by pointer.  It made it impossible to remember whether a given function
took a pointer or a reference, and made for absolute madness trying to program
against the APIs.  It got to the point where literally you had to do a build and
just add the missing &/*'s to get the build to go through.

-Chris


-------------- next part --------------
An HTML attachment was scrubbed...
URL:
<http://lists.llvm.org/pipermail/llvm-dev/attachments/20140526/ac78e2da/attachment.html>