[clang-tidy] Provide fix-its for downcasts in google-readability-casting #165411
Conversation
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters. Learn more about bidirectional Unicode characters
Member
| @llvm/pr-subscribers-clang-tools-extra @llvm/pr-subscribers-clang-tidy Author: Christian Kandeler (ckandeler) ChangesFull diff: https://github.com/llvm/llvm-project/pull/165411.diff 2 Files Affected:
diff --git a/clang-tools-extra/clang-tidy/google/AvoidCStyleCastsCheck.cpp b/clang-tools-extra/clang-tidy/google/AvoidCStyleCastsCheck.cpp index 174ecb0ed7b77..80c53de0cf237 100644 --- a/clang-tools-extra/clang-tidy/google/AvoidCStyleCastsCheck.cpp +++ b/clang-tools-extra/clang-tidy/google/AvoidCStyleCastsCheck.cpp @@ -269,6 +269,12 @@ void AvoidCStyleCastsCheck::check(const MatchFinder::MatchResult &Result) { return; } break; + case CK_BaseToDerived: + if (!needsConstCast(SourceType, DestType)) { + ReplaceWithNamedCast("static_cast"); + return; + } + break; default: break; } diff --git a/clang-tools-extra/test/clang-tidy/checkers/google/readability-casting.cpp b/clang-tools-extra/test/clang-tidy/checkers/google/readability-casting.cpp index 7ccdf705e8399..f9feb8854249b 100644 --- a/clang-tools-extra/test/clang-tidy/checkers/google/readability-casting.cpp +++ b/clang-tools-extra/test/clang-tidy/checkers/google/readability-casting.cpp @@ -102,9 +102,11 @@ void f(int a, double b, const char *cpc, const void *cpv, X *pX) { // CHECK-FIXES: b1 = static_cast<int>(b); Y *pB = (Y*)pX; - // CHECK-MESSAGES: :[[@LINE-1]]:11: warning: {{.*}}; use static_cast/const_cast/reinterpret_cast [ + // CHECK-MESSAGES: :[[@LINE-1]]:11: warning: {{.*}}; use static_cast {{.*}} + // CHECK-FIXES: Y *pB = static_cast<Y*>(pX); Y &rB = (Y&)*pX; - // CHECK-MESSAGES: :[[@LINE-1]]:11: warning: {{.*}}; use static_cast/const_cast/reinterpret_cast [ + // CHECK-MESSAGES: :[[@LINE-1]]:11: warning: {{.*}}; use static_cast {{.*}} + // CHECK-FIXES: Y &rB = static_cast<Y&>(*pX); const char *pc3 = (const char*)cpv; // CHECK-MESSAGES: :[[@LINE-1]]:21: warning: {{.*}}; use static_cast [ |
HerrCai0907 reviewed Oct 29, 2025
Comment on lines +272 to +277
| case CK_BaseToDerived: | ||
| if (!needsConstCast(SourceType, DestType)) { | ||
| ReplaceWithNamedCast("static_cast"); | ||
| return; | ||
| } | ||
| break; |
Contributor
There was a problem hiding this comment.
Is it a best practice to use static_cast for base to derived always?
I think under RTTI, we should use dynamic_cast and under non-RTTI, static_cast could be used also.
Member Author
There was a problem hiding this comment.
The fact that the code uses an unchecked cast implies a priori knowledge about the safety to do so, so static_cast seems to be the correct equivalent. Using dynamic_cast would likely incur unwanted overhead.
(I'm not too familiar with the clang-tidy code base: Is it possible/recommended to suggest competing fixes? If so, what happens to the respective code if --fix is used?)
Contributor
There was a problem hiding this comment.
AFAIK under the hood C-style cast tries first to static_cast then reinterpret_cast so we transform code equally.
(I'm not too familiar with the clang-tidy code base: Is it possible/recommended to suggest competing fixes? If so, what happens to the respective code if --fix is used?)
I we don't have competing fixes by design, but there is only some conflict resolution in case two checks try to change one piece of code: https://github.com/llvm/llvm-project/blob/main/clang-tools-extra/clang-tidy/ClangTidy.cpp#L149-L175
It's not recommended competing fixes - it's just better to write in warning message possible options and let the dev write by hand
Contributor
| Please update Release Notes. |
ckandeler force-pushed the QTCREATORBUG-18270-clang-tidy-fixits branch from 2240190 to 60c1beb Compare Member Author
| ping |
| | ||
| - Improved :doc:`google-readability-casting | ||
| <clang-tidy/checks/google/readability-casting>` check by adding a fix-it | ||
| for downcasts. |
Contributor
There was a problem hiding this comment.
notes for for downcasts?
Member Author
There was a problem hiding this comment.
Sorry, I don't understand what you mean. Can you elaborate?
Contributor
There was a problem hiding this comment.
I meant using fix-it notes in this statement.
ckandeler force-pushed the QTCREATORBUG-18270-clang-tidy-fixits branch from 60c1beb to 2920d95 Compare 
Collaborator
| LLVM Buildbot has detected a new failure on builder Full details are available at: https://lab.llvm.org/buildbot/#/builders/169/builds/16973 Here is the relevant piece of the build log for the reference |
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
6 participants
Add this suggestion to a batch that can be applied as a single commit. This suggestion is invalid because no changes were made to the code. Suggestions cannot be applied while the pull request is closed. Suggestions cannot be applied while viewing a subset of changes. Only one suggestion per line can be applied in a batch. Add this suggestion to a batch that can be applied as a single commit. Applying suggestions on deleted lines is not supported. You must change the existing code in this line in order to create a valid suggestion. Outdated suggestions cannot be applied. This suggestion has been applied or marked resolved. Suggestions cannot be applied from pending reviews. Suggestions cannot be applied on multi-line comments. Suggestions cannot be applied while the pull request is queued to merge. Suggestion cannot be applied right now. Please check back later.
No description provided.