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

_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

Reply via email to