[
https://issues.apache.org/jira/browse/ARROW-14960?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=17452036#comment-17452036
]
Weston Pace commented on ARROW-14960:
-------------------------------------
It's even more confusing:
{noformat}
Parameters are either input to the function, output from the
function, or both. Input parameters should usually be values
or <code>const</code> references,
while required (non-nullable) output and input/output parameters should
usually be references. Generally, use <code>absl::optional</code> to represent
optional inputs, and non-<code>const</code> pointers to represent
optional outputs.
{noformat}
So output parameters should be pointers if nullptr is a possible output and
mutable references if the output is guaranteed non-null. I also agree with
Antoine that the pointer requirement makes it clearer at the call site.
The justification (there is a good summary here:
https://github.com/dmlc/treelite/issues/205) appears to be that pointer output
parameters require the caller to ensure the output is not set to null so a
non-null assertion is required at the call-site. Given that pretty much all of
our calls are either accompanied by a Status or use nullptr to indicate failure
(in which case a pointer is needed anyways) I don't think this rationale makes
much sense in the context of Arrow.
So I'm +1 for leaving Arrow as-is and just noting it as a deviation in the C++
style guide (we already have a few)
> [C++] Google style guide allows mutable references now, what do?
> ----------------------------------------------------------------
>
> Key: ARROW-14960
> URL: https://issues.apache.org/jira/browse/ARROW-14960
> Project: Apache Arrow
> Issue Type: Bug
> Components: C++
> Reporter: Ben Kietzman
> Priority: Minor
>
> As of
> https://github.com/google/styleguide/commit/7a7a2f510efe7d7fc5ea8fbed549ddb31fac8f3e
> the Google Style Guide no longer forbids use of mutable references for
> output arguments, and actually encourages using them when the output argument
> is not optional.
> This puts arrow c++ style out of sync since we've continued to police toward
> usage of pointers for output arguments. We could:
> - keep the ban and note this as a deviation from google style in
> [development.rst|https://github.com/bkietz/arrow/blob/392af8aa999f940ab8fd61684820b2c6d89f7871/docs/source/developers/cpp/development.rst#L74-L75]
> - open JIRA(s) for deprecating/replacing pointer-output APIs where applicable
--
This message was sent by Atlassian Jira
(v8.20.1#820001)
