On 2014-10-02, 1:14 AM, Xidorn Quan wrote:
On Wed, Oct 1, 2014 at 6:27 PM, Ehsan Akhgari <ehsan.akhg...@gmail.com
<mailto: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?
Your request is very reasonable, but as you note, it's not possible in
C++. C++'s answer to this is "ranged-based for loops"
<http://en.wikipedia.org/wiki/C%2B%2B11#Range-based_for_loop> which in
my experience are *super* nice. They let you iterate over the array
like this:
for (const auto& element : array) {
// Use |element|
}
_______________________________________________
dev-platform mailing list
dev-platform@lists.mozilla.org
https://lists.mozilla.org/listinfo/dev-platform
