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

On May 15, 2014, at 3:22 AM, Artyom Skrobov <Artyom.Skrobov at arm.com>
wrote:
> Hello,
> 
> Carrying on this conversation from llvm-commits:
> 
>>>>> Would it be reasonable if we name both SwapByteOrder() --
it's
> difficult
>>>>> to describe their purpose in any other way -- and make the
in-place
>>>>> function take a pointer, instead of a reference?
>>>> 
>>>> Pointer is the wrong API: it implies having to check for null.
>>> 
>>> I see that in general, the choice between pointer parameters and
> reference
>>> parameters can mean one of many things: input vs output, change of
> ownership
>>> vs no change, validity of NULL, and perhaps more.
>>> 
>>> For example, Google C++ Style Guide mandates: "All parameters
passed by
>>> reference must be labeled const. [...] it is a very strong
convention in
>>> Google code that input arguments are values or const references
while
> output
>>> arguments are pointers."
> (http://google-styleguide.googlecode.com/svn/trunk/
>>> cppguide.xml#Reference_Arguments )
>> 
>> I haven't seen much of that around here.
>> 
>>> I see that LLVM Coding Standards document doesn't touch this
subject at
> all.
>>> Should we use this opportunity to add to it that in LLVM, the
choice
> between
>>> pointer parameters and reference parameters is defined by whether
NULL is
> a
>>> valid input?
>> 
>> Not sure that's necessary, but feel free to send a proposal to
llvmdev.
> 
> What does the community think about such an addition?
> 
> ==================================================================> ---
docs/CodingStandards.rst	(revision 208684)
> +++ docs/CodingStandards.rst	(working copy)
> @@ -837,6 +837,21 @@
>     It's okay to put extra implementation methods in a public class
itself.
> Just
>     make them private (or protected) and all is well.
> 
> +Use References for Non-null Function Arguments
> +^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
> +
> +Using a pointer for a function argument implies that the function must
> treat
> +NULL input in a sensible way. Where such check is unpractical (e.g. when
> every
> +call site is known to pass a non-null input), or when you want to express
> it in
> +the function signature that nullptr is not a valid input, use a reference
> +argument instead.
> +
> +This is an application of "Use references when you can, and pointers
when
> you
> +have to." maxim from the C++FAQ. Avoid using the pointers vs.
references
> +distinction to convey other meanings, e.g. to mark the distinction between
> +input and output arguments, as may be advised by other style guides.
> +
> +
> .. _early exits:
> 
> Use Early Exits and ``continue`` to Simplify Code
> ==================================================================
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.

-Andy

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.
>
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/eb53ff73/attachment.html>

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>