I would say it comes down to the module. I'm very comfortable in my
modules using auto, perhaps because our lifetime management is more
clear. To me, the unsafe examples are pretty strawman-y, since it's
very clear that there are at-risk lifetimes involved. (Returning a
bare pointer and relying on it staying valid is bad; const auto& takes
a reference, not a strong reference. If you want a new strong
reference, you obviously can't just use a reference)

`auto` was formalized in c++11, and it is now 2018. C++11 has been
required to compile Firefox since Firefox 25 in mid 2013. The rules
for auto are pretty easy to understand, too. (strips the c/v/r
decorations from the returning type, so that `const int& bar(); auto
foo = bar();` gives `foo` type `int`) In my mind, pushing back on auto
here is effectively a statement that we never expect our reviewing
ability to expand to include any new constructs, because we might
potentially maybe use those new constructs wrong. Yes, we should
always carefully consider new things. No, we shouldn't use every
new-fangled bit of the language. But honestly if you're writing c++
without auto today, it's not modern c++.

If you think auto isn't right for your module, don't accept it. That
much is agreeable, I think. To my knowledge, we've been using it for
years and so far I've only run into one at-all-related issue with it,
which ended up being a miscompilation on gcc<7 that affects all
references (including non-auto) to member scalars.
(https://bugzilla.mozilla.org/show_bug.cgi?id=1329044)

On Thu, Jan 4, 2018 at 11:31 AM, Randell Jesup <rje...@jesup.org> wrote:
> [SNIP]
>>> If foo->bar() can mutate the lifetime of foo, of course you must take a 
>>> strong
>>> reference to foo. Nothing about auto or range-for changes this.
>>
>>What auto does is make it really hard to tell whether a strong reference is
>>being taken.
>>
>>> If you don't understand your lifetimes, you get bugs.
>>
>>Fully agreed.  The discussion is about whether auto helps or hinders that
>>understanding.  The answer is that it depends on the surrounding code, from
>>where I sit...
>>
>>-Boris
>
> So, where do we go from here?
>
> It's clear that auto is not as safe as some/many believe it to be; it
> can as Boris (and smaug and Xidorn) say hide lifetime issues and lead to
> non-obvious UAFs and the like.  Some uses are certainly safe, but it
> isn't always obvious looking at a patch (per above), requiring more code
> investigation by the reviewer.  If the resultant type isn't obvious,
> then using it isn't helping comprehension.  When reading the code in the
> future, if the reader has to do non-trivial investigation just to know
> what's going on, it's hurting our work.
>
> If the win is to avoid typing.... I'd say it's not worth the risk.  Or
> allow it, but only with commentary as to why it's safe, or with rules
> about what sort of types it's allowed with and anything other than those
> requires justification in comments/etc.
>
> --
> Randell Jesup, Mozilla Corp
> remove "news" for personal email
> _______________________________________________
> dev-platform mailing list
> dev-platform@lists.mozilla.org
> https://lists.mozilla.org/listinfo/dev-platform
_______________________________________________
dev-platform mailing list
dev-platform@lists.mozilla.org
https://lists.mozilla.org/listinfo/dev-platform

Reply via email to