Page MenuHomePhabricator

[clang-tidy] Add readability-forward-usage check
Needs ReviewPublic

Authored by PiotrZSL on Sun, Feb 19, 5:22 AM.

Details

Summary

Suggests removing or replacing std::forward with std::move or
static_cast in cases where the template argument type is invariant and
the call always produces the same result.

Diff Detail

Event Timeline

PiotrZSL created this revision.Sun, Feb 19, 5:22 AM
Herald added a project: Restricted Project. · View Herald Transcript
PiotrZSL updated this revision to Diff 498666.Sun, Feb 19, 5:25 AM

Fix list.rst

Eugene.Zelenko added inline comments.
clang-tools-extra/clang-tidy/readability/ForwardUsageCheck.cpp
47

Excessive newline.

56

return {}; is shorter.

124

Usually pointers are used. Same in other similar places..

clang-tools-extra/docs/clang-tidy/checks/readability/forward-usage.rst
102

Excessive indentation. Same for other options below.

PiotrZSL marked 4 inline comments as done.EditedSun, Feb 19, 10:10 AM

Does anyone know why on WINDOWS generated AST is different:

LINUX (when function is never called):
|-FunctionTemplateDecl 0x5577338b2580 <line:211:1, line:216:1> line:212:6 testPartialTemplate
| |-TemplateTypeParmDecl 0x5577338b2288 <line:211:10, col:19> col:19 referenced typename depth 0 index 0 T
| `-FunctionDecl 0x5577338b24d8 <line:212:1, line:216:1> line:212:6 testPartialTemplate 'void (WrapperEx<T>)'
|   |-ParmVarDecl 0x5577338b23e0 <col:26, col:39> col:39 referenced value 'WrapperEx<T>':'WrapperEx<T>'
|   `-CompoundStmt 0x5577338b28a0 <col:46, line:216:1>
|     `-CallExpr 0x5577338b2878 <line:213:5, col:41> '<dependent type>'
|       |-UnresolvedLookupExpr 0x5577338b2668 <col:5> '<overloaded function type>' lvalue (ADL) = 'test' 0x557733898c10
|       `-CallExpr 0x5577338b2850 <col:10, col:40> '<dependent type>'
|         |-UnresolvedLookupExpr 0x5577338b27b0 <col:10, col:33> '<dependent type>' lvalue (no ADL) = 'forward' 0x5577338961d8 0x557733896818
|         `-DeclRefExpr 0x5577338b2830 <col:35> 'WrapperEx<T>':'WrapperEx<T>' lvalue ParmVar 0x5577338b23e0 'value' 'WrapperEx<T>':'WrapperEx<T>'

LINUX (when function is called)
|-FunctionTemplateDecl 0x556b33e8a5b0 <line:211:1, line:216:1> line:212:6 testPartialTemplate
| |-TemplateTypeParmDecl 0x556b33e8a2b8 <line:211:10, col:19> col:19 referenced typename depth 0 index 0 T
| |-FunctionDecl 0x556b33e8a508 <line:212:1, line:216:1> line:212:6 testPartialTemplate 'void (WrapperEx<T>)'
| | |-ParmVarDecl 0x556b33e8a410 <col:26, col:39> col:39 referenced value 'WrapperEx<T>':'WrapperEx<T>'
| | `-CompoundStmt 0x556b33e8a8d0 <col:46, line:216:1>
| |   `-CallExpr 0x556b33e8a8a8 <line:213:5, col:41> '<dependent type>'
| |     |-UnresolvedLookupExpr 0x556b33e8a698 <col:5> '<overloaded function type>' lvalue (ADL) = 'test' 0x556b33e70c40
| |     `-CallExpr 0x556b33e8a880 <col:10, col:40> '<dependent type>'
| |       |-UnresolvedLookupExpr 0x556b33e8a7e0 <col:10, col:33> '<dependent type>' lvalue (no ADL) = 'forward' 0x556b33e6e208 0x556b33e6e848
| |       `-DeclRefExpr 0x556b33e8a860 <col:35> 'WrapperEx<T>':'WrapperEx<T>' lvalue ParmVar 0x556b33e8a410 'value' 'WrapperEx<T>':'WrapperEx<T>'
| `-FunctionDecl 0x556b33e8e888 <line:212:1, line:216:1> line:212:6 used testPartialTemplate 'void (WrapperEx<TestType>)'

WINDOWS (when function is never called):
|-FunctionTemplateDecl 0x1ac13659de0 <line:211:1, line:212:44> col:6 testPartialTemplate
| |-TemplateTypeParmDecl 0x1ac13652238 <line:211:10, col:19> col:19 referenced typename depth 0 index 0 T
| `-FunctionDecl 0x1ac13659d38 <line:212:1, col:44> col:6 testPartialTemplate 'void (WrapperEx<T>)'
|   |-ParmVarDecl 0x1ac13652390 <col:26, col:39> col:39 value 'WrapperEx<T>':'WrapperEx<T>'
|   `-<<<NULL>>>

WINDOWS (when function is called):
|-FunctionTemplateDecl 0x1fe6f1b99a0 <line:211:1, line:216:1> line:212:6 testPartialTemplate
| |-TemplateTypeParmDecl 0x1fe6f1b9568 <line:211:10, col:19> col:19 referenced typename depth 0 index 0 T
| |-FunctionDecl 0x1fe6f1b98f8 <line:212:1, line:216:1> line:212:6 testPartialTemplate 'void (WrapperEx<T>)'
| | |-ParmVarDecl 0x1fe6f1b96c0 <col:26, col:39> col:39 referenced value 'WrapperEx<T>':'WrapperEx<T>'
| | `-CompoundStmt 0x1fe6f1c5070 <col:46, line:216:1>
| |   `-CallExpr 0x1fe6f1c5048 <line:213:5, col:41> '<dependent type>'
| |     |-UnresolvedLookupExpr 0x1fe6f1c4e40 <col:5> '<overloaded function type>' lvalue (ADL) = 'test' 0x1fe6f18df70
| |     `-CallExpr 0x1fe6f1c5020 <col:10, col:40> '<dependent type>'
| |       |-UnresolvedLookupExpr 0x1fe6f1c4f80 <col:10, col:33> '<dependent type>' lvalue (no ADL) = 'forward' 0x1fe6f18c3e8 0x1fe6f18ca28
| |       `-DeclRefExpr 0x1fe6f1c5000 <col:35> 'WrapperEx<T>':'WrapperEx<T>' lvalue ParmVar 0x1fe6f1b96c0 'value' 'WrapperEx<T>':'WrapperEx<T>'
| `-FunctionDecl 0x1fe6f1bebd8 <line:212:1, line:216:1> line:212:6 used testPartialTemplate 'void (WrapperEx<TestType>)'
|   |-TemplateArgument type 'TestType'

EDIT: Ok, found that -fno-delayed-template-parsing is an workaround for this (but still strange).

PiotrZSL updated this revision to Diff 498687.Sun, Feb 19, 10:18 AM

Fixed review comments and added -fno-delayed-template-parsing

PiotrZSL published this revision for review.Sun, Feb 19, 11:01 AM

Ready for review

Herald added a project: Restricted Project. · View Herald TranscriptSun, Feb 19, 11:01 AM
Herald added a subscriber: cfe-commits. · View Herald Transcript
PiotrZSL updated this revision to Diff 503092.Tue, Mar 7, 10:38 AM

Ping, Rebase

PiotrZSL updated this revision to Diff 503297.Wed, Mar 8, 3:22 AM

Rebase due to broken baseline

PiotrZSL updated this revision to Diff 503357.Wed, Mar 8, 7:04 AM

Rebase due to broken baseline