sammccall accepted this revision. sammccall added a comment. Still LG, thanks!
================ Comment at: clang-tools-extra/clangd/QueryDriverDatabase.cpp:344 + } + return Cmd; } ---------------- kadircet wrote: > kadircet wrote: > > ofc we don't need it. but the thing is we are returning an > > `Optional<tooling::Command>` hence the return statement needs to invoke a > > constructor for `Optional` and without `std::move` it would invoke a copy > > constructor rather than a move-based one. > > > > And even though rest of the calculations are cached per driver&language > > name, this copy is going to happen for every translation unit. So in theory > > it would be nice to prevent that. > oh actually never mind, `Cmd` itself is also an `Optional<tooling::Command>`, > i thought it was a naked `tooling::Command` It's moot here, but this isn't the case anymore, `return x` can call a converting move constructor: http://www.open-std.org/jtc1/sc22/wg21/docs/cwg_defects.html#1579 This is part of C++14, and so we should be able to rely on it in LLVM. It wasn't true in the original C++11 spec, but since it was a defect report, reasonably-modern compilers allow the optimization even in C++11 mode. Buggy compilers certainly exist, but I don't think we need to work around them for small performance issues. (If the code fails to compile on old-gcc without std::move, that's another story...) Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D92012/new/ https://reviews.llvm.org/D92012 _______________________________________________ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
