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

[Namgoo Lee via Phabricator via cfe-commits]

nlee updated this revision to Diff 434842.
nlee added a comment.

After some investigation, I think it is not always explicit that removing `const` makes any improvements when we are "constructing." For example, in this case <https://godbolt.org/z/5McKK538M>, the two codes which differ in the return type of function `f` (`S f()` vs. `const S f()`) produce the same assembly output. I assume this is due to various copy elisions stepping in.

However, when we are "assigning," it makes a difference. In this case <https://godbolt.org/z/h979nx7ov>, the existence of `const` in the return type of `f` decides whether the assignment invokes a move or not.

So I'm suggesting a different policy. Warn if:

- It is an assignment expression AND
- RHS is a function call expression AND
- The function returns by-const-value of a class or struct type AND
- The class or struct is move assignable

The following is a part of diagnosed warnings when applied to llvm:

  /Users/namgoolee/Documents/llvm-project/llvm/include/llvm/Option/Option.h:103:9: warning: const qualifying the return value prevents move semantics [-Wpessimizing-return-by-const]
    const Option getGroup() const {
          ^
  /Users/namgoolee/Documents/llvm-project/llvm/lib/Option/ArgList.cpp:38:10: note: copy assignment is invoked here even if move assignment is supported for type 'llvm::opt::Option'
         O = O.getGroup()) {
           ^
  --
  /Users/namgoolee/Documents/llvm-project/clang/include/clang/Lex/MacroInfo.h:421:9: warning: const qualifying the return value prevents move semantics [-Wpessimizing-return-by-const]
    const DefInfo findDirectiveAtLoc(SourceLocation L,
          ^
  /Users/namgoolee/Documents/llvm-project/clang/include/clang/Lex/Preprocessor.h:1167:10: note: copy assignment is invoked here even if move assignment is supported for type 'clang::MacroDirective::DefInfo'
        DI = MD->findDirectiveAtLoc(Loc, getSourceManager());
           ^


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

https://reviews.llvm.org/D125402

Files:
  clang/include/clang/Basic/DiagnosticGroups.td
  clang/include/clang/Basic/DiagnosticSemaKinds.td
  clang/include/clang/Sema/Sema.h
  clang/lib/Sema/SemaChecking.cpp
  clang/lib/Sema/SemaOverload.cpp
  clang/test/Misc/warning-wall.c
  clang/test/SemaCXX/warn-pessimizing-return-by-const.cpp

-------------- next part --------------
A non-text attachment was scrubbed...
Name: D125402.434842.patch
Type: text/x-patch
Size: 6292 bytes
Desc: not available
URL: <http://lists.llvm.org/pipermail/cfe-commits/attachments/20220607/b0414bc6/attachment.bin>