[clang-tidy][NFC] add '.clang-tidy' config for clang-tidy project #147793

vbvictor · 2025-07-09T17:48:11Z

Added .clang-tidy config as discussed in RFC.
Added bugprone, readability, modernize, performance checks that didn't create many warnings.
Fixed minor warnings to make /clang-tidy directory complaint with clang-tidy-20.

Disabled checks will be enabled in future PRs after fixing their warnings.

llvmbot · 2025-07-09T17:48:45Z

@llvm/pr-subscribers-clang-tidy

Author: Baranov Victor (vbvictor)

Changes

Added .clang-tidy config as discussed in RFC.
Added bugprone checks that didn't create many warnings.
Fixed minor warnings to make /clang-tidy directory complaint with clang-tidy-20.

Disabled checks will be enabled in future PRs after fixing their warnings.

Full diff: https://github.com/llvm/llvm-project/pull/147793.diff

3 Files Affected:

(added) clang-tools-extra/clang-tidy/.clang-tidy (+11)
(modified) clang-tools-extra/clang-tidy/bugprone/CrtpConstructorAccessibilityCheck.cpp (+2-5)
(modified) clang-tools-extra/clang-tidy/bugprone/SizeofExpressionCheck.cpp (+5-5)

diff --git a/clang-tools-extra/clang-tidy/.clang-tidy b/clang-tools-extra/clang-tidy/.clang-tidy
new file mode 100644
index 0000000000000..45d1929b6b742
--- /dev/null
+++ b/clang-tools-extra/clang-tidy/.clang-tidy
@@ -0,0 +1,11 @@
+InheritParentConfig: true
+Checks: [
+  bugprone-*
+  -bugprone-assignment-in-if-condition,
+  -bugprone-branch-clone,
+  -bugprone-easily-swappable-parameters,
+  -bugprone-narrowing-conversions,
+  -bugprone-suspicious-stringview-data-usage,
+  -bugprone-unchecked-optional-access,
+  -bugprone-unused-return-value,
+]
diff --git a/clang-tools-extra/clang-tidy/bugprone/CrtpConstructorAccessibilityCheck.cpp b/clang-tools-extra/clang-tidy/bugprone/CrtpConstructorAccessibilityCheck.cpp
index 28e8fe002d575..6565fa3f7c85b 100644
--- a/clang-tools-extra/clang-tidy/bugprone/CrtpConstructorAccessibilityCheck.cpp
+++ b/clang-tools-extra/clang-tidy/bugprone/CrtpConstructorAccessibilityCheck.cpp
@@ -129,13 +129,10 @@ void CrtpConstructorAccessibilityCheck::check(
         << HintFriend;
   }
 
-  auto WithFriendHintIfNeeded =
-      [&](const DiagnosticBuilder &Diag,
-          bool NeedsFriend) -> const DiagnosticBuilder & {
+  auto WithFriendHintIfNeeded = [&](const DiagnosticBuilder &Diag,
+                                    bool NeedsFriend) {
     if (NeedsFriend)
       Diag << HintFriend;
-
-    return Diag;
   };
 
   if (!CRTPDeclaration->hasUserDeclaredConstructor()) {
diff --git a/clang-tools-extra/clang-tidy/bugprone/SizeofExpressionCheck.cpp b/clang-tools-extra/clang-tidy/bugprone/SizeofExpressionCheck.cpp
index 88d2f2c388d07..88e048e65d4e8 100644
--- a/clang-tools-extra/clang-tidy/bugprone/SizeofExpressionCheck.cpp
+++ b/clang-tools-extra/clang-tidy/bugprone/SizeofExpressionCheck.cpp
@@ -370,16 +370,16 @@ void SizeofExpressionCheck::check(const MatchFinder::MatchResult &Result) {
         << E->getSourceRange();
   } else if (Result.Nodes.getNodeAs<Stmt>("loop-expr")) {
     auto *SizeofArgTy = Result.Nodes.getNodeAs<Type>("sizeof-arg-type");
-    if (const auto member = dyn_cast<MemberPointerType>(SizeofArgTy))
-      SizeofArgTy = member->getPointeeType().getTypePtr();
+    if (const auto *Member = dyn_cast<MemberPointerType>(SizeofArgTy))
+      SizeofArgTy = Member->getPointeeType().getTypePtr();
 
     const auto *SzOfExpr = Result.Nodes.getNodeAs<Expr>("sizeof-expr");
 
-    if (const auto type = dyn_cast<ArrayType>(SizeofArgTy)) {
+    if (const auto *Type = dyn_cast<ArrayType>(SizeofArgTy)) {
       // check if the array element size is larger than one. If true,
       // the size of the array is higher than the number of elements
-      CharUnits sSize = Ctx.getTypeSizeInChars(type->getElementType());
-      if (!sSize.isOne()) {
+      CharUnits SSize = Ctx.getTypeSizeInChars(Type->getElementType());
+      if (!SSize.isOne()) {
         diag(SzOfExpr->getBeginLoc(),
              "suspicious usage of 'sizeof' in the loop")
             << SzOfExpr->getSourceRange();

llvmbot · 2025-07-09T17:48:45Z

@llvm/pr-subscribers-clang-tools-extra

Author: Baranov Victor (vbvictor)

Changes

Added .clang-tidy config as discussed in RFC.
Added bugprone checks that didn't create many warnings.
Fixed minor warnings to make /clang-tidy directory complaint with clang-tidy-20.

Disabled checks will be enabled in future PRs after fixing their warnings.

Full diff: https://github.com/llvm/llvm-project/pull/147793.diff

3 Files Affected:

(added) clang-tools-extra/clang-tidy/.clang-tidy (+11)
(modified) clang-tools-extra/clang-tidy/bugprone/CrtpConstructorAccessibilityCheck.cpp (+2-5)
(modified) clang-tools-extra/clang-tidy/bugprone/SizeofExpressionCheck.cpp (+5-5)

diff --git a/clang-tools-extra/clang-tidy/.clang-tidy b/clang-tools-extra/clang-tidy/.clang-tidy
new file mode 100644
index 0000000000000..45d1929b6b742
--- /dev/null
+++ b/clang-tools-extra/clang-tidy/.clang-tidy
@@ -0,0 +1,11 @@
+InheritParentConfig: true
+Checks: [
+  bugprone-*
+  -bugprone-assignment-in-if-condition,
+  -bugprone-branch-clone,
+  -bugprone-easily-swappable-parameters,
+  -bugprone-narrowing-conversions,
+  -bugprone-suspicious-stringview-data-usage,
+  -bugprone-unchecked-optional-access,
+  -bugprone-unused-return-value,
+]
diff --git a/clang-tools-extra/clang-tidy/bugprone/CrtpConstructorAccessibilityCheck.cpp b/clang-tools-extra/clang-tidy/bugprone/CrtpConstructorAccessibilityCheck.cpp
index 28e8fe002d575..6565fa3f7c85b 100644
--- a/clang-tools-extra/clang-tidy/bugprone/CrtpConstructorAccessibilityCheck.cpp
+++ b/clang-tools-extra/clang-tidy/bugprone/CrtpConstructorAccessibilityCheck.cpp
@@ -129,13 +129,10 @@ void CrtpConstructorAccessibilityCheck::check(
         << HintFriend;
   }
 
-  auto WithFriendHintIfNeeded =
-      [&](const DiagnosticBuilder &Diag,
-          bool NeedsFriend) -> const DiagnosticBuilder & {
+  auto WithFriendHintIfNeeded = [&](const DiagnosticBuilder &Diag,
+                                    bool NeedsFriend) {
     if (NeedsFriend)
       Diag << HintFriend;
-
-    return Diag;
   };
 
   if (!CRTPDeclaration->hasUserDeclaredConstructor()) {
diff --git a/clang-tools-extra/clang-tidy/bugprone/SizeofExpressionCheck.cpp b/clang-tools-extra/clang-tidy/bugprone/SizeofExpressionCheck.cpp
index 88d2f2c388d07..88e048e65d4e8 100644
--- a/clang-tools-extra/clang-tidy/bugprone/SizeofExpressionCheck.cpp
+++ b/clang-tools-extra/clang-tidy/bugprone/SizeofExpressionCheck.cpp
@@ -370,16 +370,16 @@ void SizeofExpressionCheck::check(const MatchFinder::MatchResult &Result) {
         << E->getSourceRange();
   } else if (Result.Nodes.getNodeAs<Stmt>("loop-expr")) {
     auto *SizeofArgTy = Result.Nodes.getNodeAs<Type>("sizeof-arg-type");
-    if (const auto member = dyn_cast<MemberPointerType>(SizeofArgTy))
-      SizeofArgTy = member->getPointeeType().getTypePtr();
+    if (const auto *Member = dyn_cast<MemberPointerType>(SizeofArgTy))
+      SizeofArgTy = Member->getPointeeType().getTypePtr();
 
     const auto *SzOfExpr = Result.Nodes.getNodeAs<Expr>("sizeof-expr");
 
-    if (const auto type = dyn_cast<ArrayType>(SizeofArgTy)) {
+    if (const auto *Type = dyn_cast<ArrayType>(SizeofArgTy)) {
       // check if the array element size is larger than one. If true,
       // the size of the array is higher than the number of elements
-      CharUnits sSize = Ctx.getTypeSizeInChars(type->getElementType());
-      if (!sSize.isOne()) {
+      CharUnits SSize = Ctx.getTypeSizeInChars(Type->getElementType());
+      if (!SSize.isOne()) {
         diag(SzOfExpr->getBeginLoc(),
              "suspicious usage of 'sizeof' in the loop")
             << SzOfExpr->getSourceRange();

vbvictor · 2025-07-09T17:50:00Z

Added all people that participated in RFC and regular clang-tidy reviewers to see this change.

EugeneZelenko · 2025-07-09T17:52:20Z

How about adding modernize relevant to currently used C++ standard? Also some of readability checks are useful. Please also consider top-level .clang-tidy file re-formatting.

vbvictor · 2025-07-09T17:55:17Z

How about adding modernize relevant to currently used C++ standard? Also some of readability checks are useful.

Yes, I will add more and more checks to config. Just started with bugprone as it is the most straightforward improvement. Or do you think we can make it in one go?

Please also consider top-level .clang-tidy file re-formatting.

Noted, will check that.

EugeneZelenko · 2025-07-09T18:00:06Z

How about adding modernize relevant to currently used C++ standard? Also some of readability checks are useful.

Yes, I will add more and more checks to config. Just started with bugprone as it is the most straightforward improvement. Or do you think we can make it in one go?

Yes, one go is straightforward. However, if preliminary code fixes will be necessary, they should go into separate pull request.

vbvictor · 2025-07-09T18:43:50Z

Results of running with config (disabled checks that doesn't align with our style)

InheritParentConfig: true
Checks: [
  bugprone-*,
  modernize-*,
  -modernize-avoid-c-arrays,
  -modernize-use-trailing-return-type,
  readability-*,
  -readability-braces-around-statements,
  -readability-identifier-length,
  -readability-magic-numbers,
  -readability-implicit-bool-conversion,
  performance-*
]

report.html.txt (delete .txt to view)
report.txt
I will add checks with a couple of warnings and disable others for future PR's.

…ategories in .clang-tidy

EugeneZelenko

But please wait for other reviewers.

EugeneZelenko · 2025-07-09T21:26:13Z

Will be good idea to similar Clang-Tidy configuration over other parts of LLVM code base. May be some check warnings will be easy to fix and enforce. I did such cleanups in past (by quite remote :-().

PiotrZSL

LGTM

clang-tools-extra/clang-tidy/misc/RedundantExpressionCheck.cpp

carlosgalvezp

LGTM!

vbvictor · 2025-07-10T16:23:12Z

Based on carlosgalvezp's comment in #147902 (review) changed Checks: [] to Checks: > to comply with older version of clang-tidy.

LegalizeAdulthood · 2025-07-10T16:40:48Z

LGTM, I see there was a surprising amount of cruft we've accumulated :)

5chmidti

Thanks for driving this. Let's see what checks we can enable in the follow-ups.

carlosgalvezp · 2025-07-10T20:47:34Z

Thank you for driving this, LGTM!

One last thing I'm missing: can you update the clang-tidy contributors guide, to explain that we expect people to run clang-tidy as well (and what command to run)? Since it's not currently enforced in CI.

vbvictor · 2025-07-11T05:25:40Z

One last thing I'm missing: can you update the clang-tidy contributors guide, to explain that we expect people to run clang-tidy as well (and what command to run)? Since it's not currently enforced in CI.

I will add it in a separate PR. Don't feel right to make kind-of unrelated changes after most of the people gave their consent.

carlosgalvezp · 2025-07-11T07:51:05Z

I will add it in a separate PR. Don't feel right to make kind-of unrelated changes after most of the people gave their consent.

Ok!

…ontributing.rst (#148547) Follow up to #147793. _Originally suggested by carlosgalvezp in #147793 (comment)

…hanges in Contributing.rst (#148547) Follow up to llvm/llvm-project#147793. _Originally suggested by carlosgalvezp in llvm/llvm-project#147793 (comment)

**KEY POINTS** - MVP workflow to automatically lint C++ files, located **only** in `clang-tools-extra/clang-tidy`. It's chosen this way as `clang-tools-extra/clang-tidy` is almost 100% `clang-tidy` complaint, thus we would automatically enforce a [high quality standard for clang-tidy source code](https://discourse.llvm.org/t/rfc-create-hardened-clang-tidy-config-for-clang-tidy-directory/87247). (#147793) - Implementation is very similar to code-format job, but without the ability to run `code-lint-helper.py` locally. **FOUND ISSUES** + open questions - Speed: it takes ~ 1m40sec to download and unpack `clang-tidy` plus additional ~4 mins to configure and CodeGen targets. I see that `premerge.yaml` runs on special `llvm-premerge-linux-runners` runners which can use `sccache` for speed. Can we use the same runners for this job? Exact timings can be found [here](https://github.com/llvm/llvm-project/actions/runs/17135749067/job/48611150680?pr=154223). **TO DO** - Place common components from `code-lint-helper.py` and `code-format-helper.py` into a separate file and reuse it in both CI's. - Compute CodeGen targets based on `projects_to_build`, for now `clang-tablegen-targets` is hardcoded for `clang-tools-extra/`. - Automatically generate and upload `.yaml` for `clang-apply-replacements` - Create an RFC with a plan how to enable `clang-tidy` in other projects so that Maintainers of LLVM components could choose if they want `clang-tidy` or not. - Add more linters like `pylint`, `ruff` in the future.

**KEY POINTS** - MVP workflow to automatically lint C++ files, located **only** in `clang-tools-extra/clang-tidy`. It's chosen this way as `clang-tools-extra/clang-tidy` is almost 100% `clang-tidy` complaint, thus we would automatically enforce a [high quality standard for clang-tidy source code](https://discourse.llvm.org/t/rfc-create-hardened-clang-tidy-config-for-clang-tidy-directory/87247). (llvm/llvm-project#147793) - Implementation is very similar to code-format job, but without the ability to run `code-lint-helper.py` locally. **FOUND ISSUES** + open questions - Speed: it takes ~ 1m40sec to download and unpack `clang-tidy` plus additional ~4 mins to configure and CodeGen targets. I see that `premerge.yaml` runs on special `llvm-premerge-linux-runners` runners which can use `sccache` for speed. Can we use the same runners for this job? Exact timings can be found [here](https://github.com/llvm/llvm-project/actions/runs/17135749067/job/48611150680?pr=154223). **TO DO** - Place common components from `code-lint-helper.py` and `code-format-helper.py` into a separate file and reuse it in both CI's. - Compute CodeGen targets based on `projects_to_build`, for now `clang-tablegen-targets` is hardcoded for `clang-tools-extra/`. - Automatically generate and upload `.yaml` for `clang-apply-replacements` - Create an RFC with a plan how to enable `clang-tidy` in other projects so that Maintainers of LLVM components could choose if they want `clang-tidy` or not. - Add more linters like `pylint`, `ruff` in the future.

vbvictor requested review from 5chmidti, EugeneZelenko, HerrCai0907, LegalizeAdulthood, PiotrZSL, carlosgalvezp and nicovank July 9, 2025 17:48

llvmbot added clang-tools-extra clang-tidy labels Jul 9, 2025

vbvictor changed the title ~~[clang-tidy] add .clang-tidy config for clang-tidy project~~ [clang-tidy][NFC] add '.clang-tidy' config for clang-tidy project Jul 9, 2025

vbvictor force-pushed the add-clang-tidy-custom-config branch from 86802f2 to a0fd1b0 Compare July 9, 2025 18:01

vbvictor added 2 commits July 9, 2025 23:32

[clang-tidy][NFC] add '.clang-tidy' config for clang-tidy project

c4466d6

[clang-tidy][NFC] enabled 'modernize', 'readability', 'performance' c…

4b061f9

…ategories in .clang-tidy

vbvictor force-pushed the add-clang-tidy-custom-config branch from 7dbf98f to 4b061f9 Compare July 9, 2025 20:41

EugeneZelenko approved these changes Jul 9, 2025

View reviewed changes

PiotrZSL approved these changes Jul 10, 2025

View reviewed changes

nicovank approved these changes Jul 10, 2025

View reviewed changes

clang-tools-extra/clang-tidy/misc/RedundantExpressionCheck.cpp Outdated Show resolved Hide resolved

remove redundant nullptr in RedundantExpressionCheck

c5d4575

carlosgalvezp approved these changes Jul 10, 2025

View reviewed changes

change yaml list to string for older clang-tidy

9283b03

LegalizeAdulthood approved these changes Jul 10, 2025

View reviewed changes

5chmidti approved these changes Jul 10, 2025

View reviewed changes

carlosgalvezp approved these changes Jul 11, 2025

View reviewed changes

vbvictor merged commit d7a1754 into llvm:main Jul 11, 2025
9 checks passed

vbvictor deleted the add-clang-tidy-custom-config branch July 11, 2025 14:18

vbvictor mentioned this pull request Jul 13, 2025

[clang-tidy][NFC] Add mention of running 'clang-tidy' on changes in Contributing.rst #148547

Merged

vbvictor added a commit that referenced this pull request Jul 18, 2025

[clang-tidy][NFC] Add mention of running 'clang-tidy' on changes in C…

4e6157f

…ontributing.rst (#148547) Follow up to #147793. _Originally suggested by carlosgalvezp in #147793 (comment)

vbvictor mentioned this pull request Jul 18, 2025

[clang-tidy] add new check: modernize-use-scoped-lock #126434

Merged

vbvictor mentioned this pull request Aug 21, 2025

[GitHub][CI] Add clang-tidy premerge workflow #154829

Merged

[clang-tidy][NFC] add '.clang-tidy' config for clang-tidy project #147793

[clang-tidy][NFC] add '.clang-tidy' config for clang-tidy project #147793

Uh oh!

Conversation

vbvictor commented Jul 9, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

llvmbot commented Jul 9, 2025

Uh oh!

llvmbot commented Jul 9, 2025

Uh oh!

vbvictor commented Jul 9, 2025

Uh oh!

EugeneZelenko commented Jul 9, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

vbvictor commented Jul 9, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

EugeneZelenko commented Jul 9, 2025

Uh oh!

vbvictor commented Jul 9, 2025

Uh oh!

EugeneZelenko left a comment

Choose a reason for hiding this comment

Uh oh!

EugeneZelenko commented Jul 9, 2025

Uh oh!

PiotrZSL left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

carlosgalvezp left a comment

Choose a reason for hiding this comment

Uh oh!

vbvictor commented Jul 10, 2025

Uh oh!

LegalizeAdulthood commented Jul 10, 2025

Uh oh!

5chmidti left a comment

Choose a reason for hiding this comment

Uh oh!

carlosgalvezp commented Jul 10, 2025

Uh oh!

vbvictor commented Jul 11, 2025

Uh oh!

carlosgalvezp commented Jul 11, 2025

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

8 participants

vbvictor commented Jul 9, 2025 •

edited

Loading

EugeneZelenko commented Jul 9, 2025 •

edited

Loading

vbvictor commented Jul 9, 2025 •

edited

Loading