On 07/12/2018 05:17 PM, Richard Sandiford wrote:
> Pedro Alves <pal...@redhat.com> writes:
>> (an
>>> alternative to pointers is to return a struct with the wide int result
>>> and the overflow flag),
>>
>> +1. I've been pushing GDB in that direction whenever possible.
>
> I agree that can sometimes be better. I guess it depends on the
> context. If a function returns a bool and some other data that has no
> obvious "failure" value, it can be easier to write chained conditions if
> the function returns the bool and provides the other data via an output
> parameter.
I agree it depends on context, though your example sounds like a
case for std::optional<T>. (We have gdb::optional<T> in gdb, since
std::optional is C++17 and gdb requires C++11. LLVM has a similar
type, I believe.)
>
>>> but ...
>>>
>>>> +int *elt = &array[i]; // OK
>>>> +int &elt = array[i]; // Please avoid
>>>
>>> ... this seems unnecessary. If the function is so long that the fact
>>> elt is a reference can easily get lost, the problem is the length of
>>> the function, not the use of a reference.
>>>
>>
>> +1000. This seems really unnecessary. References have the advantage
>> of implicit never-null semantics, for instance.
>
> The nonnullness is there either way in the above example though.
>
> I don't feel as strongly about the non-const-reference variables, but for:
>
> int &elt = array[i];
> ...
> elt = 1;
>
> it can be easy to misread that "elt = 1" is changing more than just
> a local variable.
I think that might be just the case for people who haven't used
references before, and once you get some exposure, the effect
goes away. See examples below.
>
> I take Jonathan's point that it shouldn't be much of a problem if
> functions are a reasonable size, but we've not tended to be very
> good at keeping function sizes down. I guess part of the problem
> is that even functions that start off small tend to grow over time.
>
> We have been better at enforcing more specific rules like the ones
> in the patch. And that's easier to do because a patch either adds
> references or it doesn't. It's harder to force (or remember to force)
> someone to split up a function if they're adding 5 lines to one that is
> already 20 lines long. Then for the next person it's 25 lines long, etc.
>
> Also, the "int *elt" approach copes with cases in which "elt" might
> have to be redirected later, so we're going to need to use it in some
> cases.
Sure, if you need to reseat, certainly use a pointer. That actually
highlights an advantage of references -- the fact that you are sure
nothing could reseat it. With a local pointer, you need to be mindful
of that. "Does this loop change the pointer?" Etc. If the semantics
of the function call for having a variable that is not meant to be
reseated, why not allow expressing it with a reference.
"Write what you mean." Of course, just like all code, there's going
to be a judgment call.
A place where references frequently appear is in C++11 range-for loops.
Like, random example from gdb's codebase:
for (const symbol_search &p : symbols)
GCC doesn't yet require C++11, but it should be a matter
of time, one would hope.
Other places references appear often is in coming up with
an alias to avoid repeating long expressions, like
for example (again from gdb):
auto &vec = objfile->per_bfd->demangled_hash_languages;
auto it = std::lower_bound (vec.begin (), vec.end (),
MSYMBOL_LANGUAGE (sym));
if (it == vec.end () || *it != MSYMBOL_LANGUAGE (sym))
vec.insert (it, MSYMBOL_LANGUAGE (sym));
I don't think the fact that "vec" is a reference here confuses
anyone.
That was literally a random sample (grepped for "&[a-z]* = ") so
I ended up picking one with C++11 "auto", but here's another
random example, spelling out the type name, similarly using
a reference as a shorthand alias:
osdata_item &item = osdata->items.back ();
item.columns.emplace_back (std::move (data->property_name),
std::string (body_text));
> It's then a question of whether "int &elt" is useful enough that
> it's worth accepting it too, and having a mixture of styles in the codebase.
I don't see it as a mixture of styles, as I don't see
pointers and references the exact same thing, but rather
see references as another tool in the box.
Thanks,
Pedro Alves
