On 11/28/17 10:28 PM, Jeff Gilbert wrote:
const auto& foo = getFoo();
foo->bar(); // foo is always safe here

Sadly not true.  Some counterexamples:

  Foo* getFoo() {
    return mFoo; // Hey, this is the common case!
  }

  const RefPtr<Foo>& getFoo() {
    return mFoo;  // Can be nulled out by the call to bar()
  }

It's safe if you have:

  RefPtr<Foo> getFoo() {
    return mFoo;
  }

but of course now every access has refcounting, which is not exactly 0-cost.

Use of auto instead of auto& should be less common.

At a quick glance, we have ~16000 mentions of "auto" in our .h/.cpp files (some in comments, I expect). ~2000 are "auto&", ~800 are "auto &", ~700 are "auto*", ~200 are "auto *".

There's a lot of

  for (auto iter = mObserverTopicTable.Iter(); !iter.Done(); iter.Next()) {

going on (~1000).

There's also things like:

  for (auto entry : aLookAndFeelIntCache) {

and whatnot.

Almost every other auto is `const auto&`, which has quite good and
predictable behavior: It names a temporary and extends its lifetime to
the current scope.

That's fine, but the temporary may not keep the thing you're dealing with alive; see above.

   for (auto foo : myFoos) {
     foo->bar();
   }
This is always safe if decltype(foo) is a strong ref.

Which it may or may not be; that depends on what the iterator defined on myFoos do.

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
_______________________________________________
dev-platform mailing list
dev-platform@lists.mozilla.org
https://lists.mozilla.org/listinfo/dev-platform

Reply via email to