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

[Arthur O'Dwyer via Phabricator via cfe-commits]

Quuxplusone added a comment.

@rsmith and/or @rtrieu, please take another look?  All my TODOs are done now: there are fixits, and the wording of the diagnostic changes if it's a "throw" instead of a "return", and the wording has been updated per Richard Smith's suggestions.

I have one very minor nit that I don't know how to fix:

  warn-return-std-move.cpp:220:12: warning: local variable 'd' will be copied
        despite being returned by name [-Wreturn-std-move]
      return (d);  // e17
             ^
  warn-return-std-move.cpp:220:12: note: call 'std::move' explicitly to avoid copying
      return (d);  // e17
             ^~~
             std::move(d)

The warning places a caret directly under the `(`, when I wish it would place `^~~` under the entire expression, the way the fixit does.

I also spent a little time looking into whether I could/should diagnose

  auto [x, y] = std::make_pair(Derived(), Derived());
  return x;  // 'x' will be copied despite being returned by name

but I have decided that this is probably too far afield to be rolled into this patch, even if I could figure out how to detect it, which to a first approximation I cannot. So I am deliberately punting on that one.


Repository:
  rC Clang

https://reviews.llvm.org/D43322