aaron.ballman added reviewers: aaron.ballman, clang-language-wg. aaron.ballman added a comment.
Thank you for working on this new diagnostic! We don't typically add new diagnostics that are off by default unless they're for pedantic diagnostics or are reasonably expected to be enabled by default in the future. This is because we've had evidence that suggests such diagnostics aren't enabled often enough to warrant the maintenance cost for them. I'm not convinced this diagnostic can be enabled by default. Yes, it prevents move semantics, but that's not typically an issue with the correctness of the code. IIRC, we decided to put this diagnostic into clang-tidy because we thought it would be too chatty in practice, especially on older code bases. Perhaps there are ways we can improve the diagnostic behavior to allow it to be enabled by default though. One possibility would be to look at the return type to see if there's a user-provided move constructor (either `= default` or explicitly written) and only diagnose if one is found. But I think we'd want some evidence that this actually reduces the false positive rate in practice, which means trying to compile some large C++ projects (of various ages/code quality) to see. Would you be willing to run your patch against some large C++ projects to see what kind of true and false positives it generates? From there, we can decide whether we need additional heuristics or not. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D125402/new/ https://reviews.llvm.org/D125402 _______________________________________________ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
