-
Notifications
You must be signed in to change notification settings - Fork 15.6k
[clang-tidy] Add fix-it for C-style cast of nullptr #173159
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
base: main
Are you sure you want to change the base?
Conversation
|
Thank you for submitting a Pull Request (PR) to the LLVM Project! This PR will be automatically labeled and the relevant teams will be notified. If you wish to, you can add reviewers by using the "Reviewers" section on this page. If this is not working for you, it is probably because you do not have write permissions for the repository. In which case you can instead tag reviewers by name in a comment by using If you have received no comments on your PR for a week, you can request a review by "ping"ing the PR by adding a comment “Ping”. The common courtesy "ping" rate is once a week. Please remember that you are asking for valuable time from other developers. If you have further questions, they may be answered by the LLVM GitHub User Guide. You can also ask questions in a comment on this PR, on the LLVM Discord or on the forums. |
|
@llvm/pr-subscribers-clang-tools-extra @llvm/pr-subscribers-clang-tidy Author: Mradul Sonare (SonareMradul) ChangesAdds a fix-it for C-style casts of nullptr in google-readability-casting, Fixes #<173147> Full diff: https://github.com/llvm/llvm-project/pull/173159.diff 1 Files Affected:
diff --git a/clang-tools-extra/clang-tidy/modernize/AvoidCStyleCastCheck.cpp b/clang-tools-extra/clang-tidy/modernize/AvoidCStyleCastCheck.cpp
index 76f2030158c81..2e7fc0f364b48 100644
--- a/clang-tools-extra/clang-tidy/modernize/AvoidCStyleCastCheck.cpp
+++ b/clang-tools-extra/clang-tidy/modernize/AvoidCStyleCastCheck.cpp
@@ -218,6 +218,11 @@ void AvoidCStyleCastCheck::check(const MatchFinder::MatchResult &Result) {
ReplaceWithNamedCast("static_cast");
return;
case CK_NoOp:
+ if (isa<CXXNullPtrLiteralExpr>(CastExpr->getSubExprAsWritten()->IgnoreImpCasts())) {
+ ReplaceWithNamedCast("static_cast");
+ return;
+}
+
if (FnToFnCast) {
ReplaceWithNamedCast("static_cast");
return;
|
You can test this locally with the following command:git-clang-format --diff origin/main HEAD --extensions cpp -- clang-tools-extra/clang-tidy/modernize/AvoidCStyleCastCheck.cpp clang-tools-extra/test/clang-tidy/checkers/modernize/avoid-c-style-cast.cpp --diff_from_common_commit
View the diff from clang-format here.diff --git a/clang-tools-extra/clang-tidy/modernize/AvoidCStyleCastCheck.cpp b/clang-tools-extra/clang-tidy/modernize/AvoidCStyleCastCheck.cpp
index 2e7fc0f36..6c49869cf 100644
--- a/clang-tools-extra/clang-tidy/modernize/AvoidCStyleCastCheck.cpp
+++ b/clang-tools-extra/clang-tidy/modernize/AvoidCStyleCastCheck.cpp
@@ -218,10 +218,11 @@ void AvoidCStyleCastCheck::check(const MatchFinder::MatchResult &Result) {
ReplaceWithNamedCast("static_cast");
return;
case CK_NoOp:
- if (isa<CXXNullPtrLiteralExpr>(CastExpr->getSubExprAsWritten()->IgnoreImpCasts())) {
- ReplaceWithNamedCast("static_cast");
- return;
-}
+ if (isa<CXXNullPtrLiteralExpr>(
+ CastExpr->getSubExprAsWritten()->IgnoreImpCasts())) {
+ ReplaceWithNamedCast("static_cast");
+ return;
+ }
if (FnToFnCast) {
ReplaceWithNamedCast("static_cast");
|
|
The change looks good, but please:
|
| on Windows when the check was enabled with a 32-bit :program:`clang-tidy` | ||
| binary. | ||
|
|
||
| - Fixed google-readability-casting to provide a fix-it for C-style casts of |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Please follow the existing style of documenting changes:
- - Fixed google-readability-casting to provide a fix-it for C-style casts of
- nullptr.
+ - Improved :doc:`modernize-avoid-c-style-cast
+ <clang-tidy/checks/modernize/avoid-c-style-cast>` by providing correct fixes
+ for C-style casts of nullptr| ReplaceWithNamedCast("static_cast"); | ||
| return; | ||
| case CK_NoOp: | ||
| if (isa<CXXNullPtrLiteralExpr>(CastExpr->getSubExprAsWritten()->IgnoreImpCasts())) { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Seems like a formatting issue? You can run git clang-format HEAD~1 to fix it :)
🐧 Linux x64 Test Results
Failed Tests(click on a test name to see its output) Clang ToolsClang Tools.clang-tidy/infrastructure/alphabetical-order.testIf these failures are unrelated to your changes (for example tests are broken or flaky at HEAD), please open an issue at https://github.com/llvm/llvm-project/issues and add the |
🪟 Windows x64 Test Results
Failed Tests(click on a test name to see its output) Clang ToolsClang Tools.clang-tidy/infrastructure/alphabetical-order.testIf these failures are unrelated to your changes (for example tests are broken or flaky at HEAD), please open an issue at https://github.com/llvm/llvm-project/issues and add the |
|
@zeyi2 Could you please advise what is required from my side to get the Linux and Windows checks to paas? |
| on Windows when the check was enabled with a 32-bit :program:`clang-tidy` | ||
| binary. | ||
|
|
||
| - Improved :doc:`modernize-avoid-c-style-cast |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
In Clang-Tidy documentations, we want entries in Changes in existing checks to keep alphabetical order, so this new entries should be placed between modernize-avoid-c-arrays and modernize-use-constraints :)
You can try building If you notice any failures on GitHub, the specific output will be available in the comments to help you troubleshoot. |
| } | ||
| // CHECK-MESSAGES: warning: C-style casts are discouraged | ||
| // CHECK-FIXES: f(static_cast<int*>(nullptr)); | ||
|
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Excessive newline.
|
|
||
| throw S2(5.0f); | ||
| } | ||
| void f(int *); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Will be good idea to separate with newline.
Adds a fix-it for C-style casts of nullptr in google-readability-casting,
suggesting static_cast<T*>(nullptr).
Fixes #173147