[PATCH] D125402: [clang][diag] warn if function returns class type by-const-value

[Aaron Ballman via Phabricator via cfe-commits]

aaron.ballman added inline comments.


================
Comment at: clang/include/clang/Basic/DiagnosticSemaKinds.td:6632
+  "const qualifying the return value prevents move semantics">,
+  InGroup<PessimizingReturnByConst>, DefaultIgnore;
+def note_pessimizing_return_by_const : Note<
----------------
Given that this is a `DefaultIgnore`, I'm still not quite sure this belongs in Clang rather than continuing to be covered by clang-tidy.

If you leave it on by default, how many Clang tests break as a result? Do the new warnings all look like true positives? (Basically, I'm trying to see how far away we are from being able to enable this diagnostic by default.)


================
Comment at: clang/test/SemaCXX/warn-pessimizing-return-by-const.cpp:50
+
+const S3 f3_const() { // do not warn if move assignment operator is deleted
+  return S3{};
----------------
We should also have a test for an explicitly deleted move assignment, and another one where a member of the class is not movable (and thus the move assignment should be deleted that way as well).


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

https://reviews.llvm.org/D125402