[PATCH] D43322: Diagnose cases of "return x" that should be "return std::move(x)" for efficiency

[Nico Weber via Phabricator via cfe-commits]

thakis added a comment.

This is a cool warning, thanks for adding it. We ran into one thing while enabling this in Chromium that I'd like to mention here. We have code that basically does:

  struct Foo {
    using passwords_iterator = std::map<base::string16,
                                        std::set<std::string>,
                                        ReverseStringLess>::const_iterator;
    std::map<base::string16, std::set<std::string>, ReverseStringLess> passwords_;
    passwords_iterator get(const base::string16& in) {
      auto it = passwords_.lower_bound(in);
      return it;
    }
  };

Here, the warning gets emitted because `auto it` becomes a non-const iterator, and passwords_iterator is a const_iterator. Maybe the warning could suggest something like "cv qualifiers don't match, make them match" on a note in addition or instead of std::move() for this case?

And then someone else pointed out that http://www.open-std.org/jtc1/sc22/wg21/docs/cwg_defects.html#1579 might mean that the code-as is should be fine too in C++14 – I don't know if that's true, but maybe you could comment on that too :-)

(references:
https://cs.chromium.org/chromium/src/components/password_manager/core/browser/password_reuse_detector.h?type=cs&q=passwords_iterator&sq=package:chromium&l=64
https://chromium-review.googlesource.com/c/chromium/src/+/1025435/6/components/password_manager/core/browser/password_reuse_detector.cc
https://chromium-review.googlesource.com/c/chromium/src/+/1025435/8/components/password_manager/core/browser/password_reuse_detector.cc
)


Repository:
  rC Clang

https://reviews.llvm.org/D43322