[PATCH] D144347: [clang-tidy] Add readability-forward-usage check

Piotr Zegar via Phabricator via cfe-commits cfe-commits at lists.llvm.org
Sun Feb 19 11:01:37 PST 2023


PiotrZSL created this revision.
Herald added subscribers: carlosgalvezp, xazax.hun.
Herald added a reviewer: njames93.
Herald added a project: All.
PiotrZSL updated this revision to Diff 498666.
PiotrZSL added a comment.
Eugene.Zelenko added reviewers: aaron.ballman, carlosgalvezp.
PiotrZSL marked 4 inline comments as done.
PiotrZSL updated this revision to Diff 498687.
PiotrZSL published this revision for review.
Herald added a project: clang-tools-extra.
Herald added a subscriber: cfe-commits.

Fix list.rst


PiotrZSL added a comment.

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 added a comment.

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


PiotrZSL added a comment.

Ready for review



================
Comment at: clang-tools-extra/clang-tidy/readability/ForwardUsageCheck.cpp:46
+inline SourceRange getAnglesLoc(const Expr &MatchedCallee) {
+
+  if (const auto *DeclRefExprCallee =
----------------
Excessive newline.


================
Comment at: clang-tools-extra/clang-tidy/readability/ForwardUsageCheck.cpp:55
+                       UnresolvedLookupExprCallee->getRAngleLoc());
+  return SourceRange();
+}
----------------
`return {};` is shorter.


================
Comment at: clang-tools-extra/clang-tidy/readability/ForwardUsageCheck.cpp:123
+void ForwardUsageCheck::check(const MatchFinder::MatchResult &Result) {
+  const auto &MatchedCallee = *Result.Nodes.getNodeAs<Expr>("callee");
+  if (MatchedCallee.getBeginLoc().isMacroID() ||
----------------
Usually pointers are used. Same in other similar places..


================
Comment at: clang-tools-extra/docs/clang-tidy/checks/readability/forward-usage.rst:101
+	Disables suggestions for type mismatch situations. When this option is
+    enabled, the check treats situations with different types as if they were
+    cv-equal and won't suggest using ``static_cast``, but will suggest using
----------------
Excessive indentation. Same for other options below.


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.


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D144347

Files:
  clang-tools-extra/clang-tidy/readability/CMakeLists.txt
  clang-tools-extra/clang-tidy/readability/ForwardUsageCheck.cpp
  clang-tools-extra/clang-tidy/readability/ForwardUsageCheck.h
  clang-tools-extra/clang-tidy/readability/ReadabilityTidyModule.cpp
  clang-tools-extra/docs/ReleaseNotes.rst
  clang-tools-extra/docs/clang-tidy/checks/list.rst
  clang-tools-extra/docs/clang-tidy/checks/readability/forward-usage.rst
  clang-tools-extra/test/clang-tidy/checkers/readability/forward-usage.cpp

-------------- next part --------------
A non-text attachment was scrubbed...
Name: D144347.498687.patch
Type: text/x-patch
Size: 27364 bytes
Desc: not available
URL: <http://lists.llvm.org/pipermail/cfe-commits/attachments/20230219/669941a5/attachment-0001.bin>


More information about the cfe-commits mailing list