On Wed, Oct 1, 2014 at 6:27 PM, Ehsan Akhgari <ehsan.akhg...@gmail.com>
wrote:
> On 2014-10-01, 7:42 PM, L. David Baron wrote:
>
>> On Wednesday 2014-10-01 16:24 -0700, Eric Rescorla wrote:
>>
>>> Obviously, if you have some argument that auto is bad programming
>>> practice
>>> or a hazard and should thus be forbidden, that's something you could make
>>> and
>>> see if people generally agree...
>>>
>>
>> I think there are cases where |auto| makes code substantially less
>> readable, and cases where it's fine. I don't think I have enough
>> experience reading code using |auto| to say exactly what's in which
>> set, but I have mandated less use of |auto| in code reviews.
>>
>> Some of it is simple readability; there are cases where seeing the
>> type of a variable allows its name to be simpler while preserving
>> the same level of readability, and if we're going to hide the type,
>> I'd want it to have a more complex name to make the code obvious.
>>
>> But I'm also worried about use of auto leading people to stop using
>> const or & where they should be (particularly where |auto| instead
>> of |const auto&| leads to unnecessary expensive copies). (And I
>> think knowing whether I want |auto| or |const auto&| requires
>> knowing the type, which makes the feature less valuable to me.)
>>
>> I'm fine with just enforcing reasonable judgment in code reviews,
>> although I suspect some people would be bothered by having code
>> reviewers enforce style rules that aren't written down.
>>
>
> FWIW, I have had some actual practical experience in using |auto| in the
> LLVM code base (and with reading code that uses it). Here is my take on
> things:
>
> * Beware of using auto in cases where types can be constructed implicitly
> from other types.
>
> This is the most important rule, I think. Here's an example of how this
> can introduce real bugs. Let's imagine you have code like this:
>
> nsIFoo* FooGetter();
> nsCOMPtr<nsIFoo> foo = FooGetter();
>
> DO NOT replace the type here with auto:
>
> auto foo = FooGetter(); // NOT COOL!
>
> After this change, foo will be a raw nsIFoo*, which is a weak reference,
> so this changes the meaning of the program.
>
> Arguably, this is not an issue with auto, but with implicit conversions.
> It is one of the reasons why implicit conversions are evil. :-)
>
> * |auto| itself is rarely what you want (unless you know the underlying
> type is not copyable), you probably want |auto&| for references if you
> intend to modify the original value, or |const auto&| if you don't. A
> sufficiently smart compiler should be able to figure out that it doesn't
> actually need to create a reference under the hood if the type it's seeing
> is a built-in one. (clang seems to be able to do this based on a quick
> test.)
>
> * If you find yourself typing the type once in the statement already, use
> |auto|. Example:
>
> auto ptr = static_cast<Class*>(foo);
>
> * Do not use auto for functions that can return different types that have
> the same interface, as it obscures away the operation performed on the
> type. Example:
>
> class Container {
> // ...
> // Assume that the below iterator types map closely to the STL concepts
> iterator foo(int x);
> reverse_iterator foo(string y);
> };
>
> Container c;
> auto it = c.foo("str"); // What kind of iterator do I have?!
>
> * Do use auto for situations where the type doesn't really matter. For
> example, if you have code like below:
>
> foo(bar());
>
> And want to put the call to |bar| into its own statements, and do not care
> what the return type is, use auto:
>
> const auto& result = bar();
> foo(result);
>
> (Of course, depending on the nature of |foo| and |bar|, not all code that
> looks like this should be refactored this way.)
>
> * Do not use auto outside of a local scope. This helps restrict the
> implications of auto to a hopefully smaller amount of code.
>
> * Do not overuse auto. Consider auto as a handy tool that prevents you
> from typing needlessly. But if you have code which uses auto all over the
> place, you're *probably* doing it wrong.
>
> * Remember that at the end of the day, code is read way more often than it
> is written. If in doubt, opt for keeping the code more readable by typing
> in what you mean, IOW, don't use auto when in doubt.
>
I want to add one case, which is when receiving some plain integers from
some kinds of functions, and doing computation based on them. For example:
> nsTArray<nsString> array;
> auto length = array.Length();
In this case, the perfectly exact type of |length| should be
"nsTArray<nsString>::size_type". Before we have |auto|, the common way is
to write int32_t, uint32_t, size_t, or something similar. There are two
main problems of using those types:
1. the variable type may be different with the return type of the function,
which causes (possibly unwanted) implicit type conversion, like converting
between signed and unsigned, or 32bit and 64bit;
2. the return type may change over time, (e.g. uint32_t -> size_t), which
again causes the first problem for previously perfect code.
Hence, I suggest for cases like this, |auto| should be a recommendation.
IIRC, it is one of the cases that |auto| was introduced for.
For cases like:
> for (int32_t i = 0; i < array.Length(); i++)
I even want to write it as
> for (decltype(array.Length()) i = 0; i < array.Length(); i++)
but I feel it too redundant. I hope C++ could support
> for (auto iend = array.Length(), i = 0; i < iend; i++)
but, unfortunately, it does not.
Do you think this case reasonable?
_______________________________________________
dev-platform mailing list
dev-platform@lists.mozilla.org
https://lists.mozilla.org/listinfo/dev-platform
