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 _______________________________________________ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
