[PATCH] D70390: [clang-tidy] new performance-no-automatic-move check.

Clement Courbet via Phabricator via cfe-commits cfe-commits at lists.llvm.org
Mon Nov 18 23:47:30 PST 2019


courbet abandoned this revision.
courbet marked an inline comment as done.
courbet added inline comments.


================
Comment at: clang-tools-extra/docs/clang-tidy/checks/performance-no-automatic-move.rst:47
+    std::vector<int>&& obj = ...;
+    return std::move(obj);  // calls StatusOr::StatusOr(std::vector<int>&&)
+  }
----------------
lebedev.ri wrote:
> JonasToth wrote:
> > lebedev.ri wrote:
> > > courbet wrote:
> > > > JonasToth wrote:
> > > > > While checking this example it seems clang already has a warning for that case?
> > > > > 
> > > > > https://godbolt.org/z/q5zzh-
> > > > > 
> > > > > What parts of this check will be more then the warnings already do?
> > > > I was not aware of that, thanks for pointing that out. I don't think the check does more than the warning in that case. TBH I have not seen instances of this while running the check on our codebase (I'm only looking at a sample of the mistakes though, there are too many hits to look at all of them). All mistakes I have seen are of the `const` kind. 
> > > > 
> > > > The value we get from having this in the form of a check is more control over which types are allowed through the clang-tidy options.
> > > The `const std::vector<int> f` isn't diagnosed by the existing diag: https://godbolt.org/z/ZTQ3H6
> > so should we split this up in a diagnostic and a clang-tidy check? maybe it makes more sense to improve the warning further?
> I'd +1 just enhancing the clang diagnostic.
> The downside is that it does not allow blacklist, sure.
OK I'll work on improving the warning.



Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D70390/new/

https://reviews.llvm.org/D70390





More information about the cfe-commits mailing list