[clang-tools-extra] [clang-tidy] Add modernize-cleanup-static-cast check (PR #118033)
via cfe-commits
cfe-commits at lists.llvm.org
Thu Nov 28 12:18:48 PST 2024
llvmbot wrote:
<!--LLVM PR SUMMARY COMMENT-->
@llvm/pr-subscribers-clang-tools-extra
Author: Helmut Januschka (hjanuschka)
<details>
<summary>Changes</summary>
> Add a new check that detects and removes redundant static_cast operations where the source and target types are identical. This helps clean up code after type system changes, improving readability and reducing opportunities for errors during future refactoring.
before polishing it and adding documentation, can someone tell me if the check makes sense to be picked in modernize?
---
Full diff: https://github.com/llvm/llvm-project/pull/118033.diff
5 Files Affected:
- (modified) clang-tools-extra/clang-tidy/modernize/CMakeLists.txt (+1)
- (added) clang-tools-extra/clang-tidy/modernize/CleanupStaticCastCheck.cpp (+70)
- (added) clang-tools-extra/clang-tidy/modernize/CleanupStaticCastCheck.h (+38)
- (modified) clang-tools-extra/clang-tidy/modernize/ModernizeTidyModule.cpp (+3)
- (added) clang-tools-extra/test/clang-tidy/checkers/modernize/modernize-cleanup-static-cast.cpp (+42)
``````````diff
diff --git a/clang-tools-extra/clang-tidy/modernize/CMakeLists.txt b/clang-tools-extra/clang-tidy/modernize/CMakeLists.txt
index c919d49b42873a..9ce32473c201bc 100644
--- a/clang-tools-extra/clang-tidy/modernize/CMakeLists.txt
+++ b/clang-tools-extra/clang-tidy/modernize/CMakeLists.txt
@@ -49,6 +49,7 @@ add_clang_library(clangTidyModernizeModule STATIC
UseTransparentFunctorsCheck.cpp
UseUncaughtExceptionsCheck.cpp
UseUsingCheck.cpp
+ CleanupStaticCastCheck.cpp
LINK_LIBS
clangTidy
diff --git a/clang-tools-extra/clang-tidy/modernize/CleanupStaticCastCheck.cpp b/clang-tools-extra/clang-tidy/modernize/CleanupStaticCastCheck.cpp
new file mode 100644
index 00000000000000..1fdb0cff149883
--- /dev/null
+++ b/clang-tools-extra/clang-tidy/modernize/CleanupStaticCastCheck.cpp
@@ -0,0 +1,70 @@
+//===--- CleanupStaticCastCheck.cpp - clang-tidy-----------------*- C++ -*-===//
+//
+// Part of the LLVM Project, under the Apache License v2.0 with LLVM Exceptions.
+// See https://llvm.org/LICENSE.txt for license information.
+// SPDX-License-Identifier: Apache-2.0 WITH LLVM-exception
+//
+//===----------------------------------------------------------------------===//
+
+#include "CleanupStaticCastCheck.h"
+#include "clang/AST/ASTContext.h"
+#include "clang/AST/DeclTemplate.h"
+#include "clang/ASTMatchers/ASTMatchFinder.h"
+#include "clang/ASTMatchers/ASTMatchers.h"
+#include "clang/Basic/SourceManager.h"
+#include "clang/Lex/Lexer.h"
+
+using namespace clang::ast_matchers;
+
+namespace clang::tidy::modernize {
+
+
+std::string getText(const clang::Expr *E, const clang::ASTContext &Context) {
+ auto &SM = Context.getSourceManager();
+ auto Range = clang::CharSourceRange::getTokenRange(E->getSourceRange());
+ return clang::Lexer::getSourceText(Range, SM, Context.getLangOpts()).str();
+}
+
+void CleanupStaticCastCheck::registerMatchers(MatchFinder *Finder) {
+ // Match static_cast expressions not in templates
+ Finder->addMatcher(
+ cxxStaticCastExpr(
+ unless(hasAncestor(functionTemplateDecl())),
+ unless(hasAncestor(classTemplateDecl())),
+ unless(isInTemplateInstantiation()))
+ .bind("cast"),
+ this);
+}
+
+void CleanupStaticCastCheck::check(const MatchFinder::MatchResult &Result) {
+ const auto *Cast = Result.Nodes.getNodeAs<CXXStaticCastExpr>("cast");
+ if (!Cast)
+ return;
+
+ const Expr *SubExpr = Cast->getSubExpr()->IgnoreParenImpCasts();
+ QualType SourceType = SubExpr->getType();
+ QualType TargetType = Cast->getType();
+
+ // Skip if either type is dependent
+ if (SourceType->isDependentType() || TargetType->isDependentType())
+ return;
+
+ // Compare canonical types and qualifiers
+ SourceType = SourceType.getCanonicalType();
+ TargetType = TargetType.getCanonicalType();
+
+ if (SourceType == TargetType) {
+ auto Diag =
+ diag(Cast->getBeginLoc(),
+ "redundant static_cast to the same type %0") // Removed single quotes
+ << TargetType;
+
+ std::string ReplacementText = getText(SubExpr, *Result.Context);
+
+ Diag << FixItHint::CreateReplacement(
+ Cast->getSourceRange(),
+ ReplacementText);
+ }
+}
+
+} // namespace clang::tidy::modernize
\ No newline at end of file
diff --git a/clang-tools-extra/clang-tidy/modernize/CleanupStaticCastCheck.h b/clang-tools-extra/clang-tidy/modernize/CleanupStaticCastCheck.h
new file mode 100644
index 00000000000000..a96e7cac3f1f72
--- /dev/null
+++ b/clang-tools-extra/clang-tidy/modernize/CleanupStaticCastCheck.h
@@ -0,0 +1,38 @@
+// CleanupStaticCastCheck.h
+#ifndef LLVM_CLANG_TOOLS_EXTRA_CLANG_TIDY_MODERNIZE_CLEANUPSTATICCASTCHECK_H
+#define LLVM_CLANG_TOOLS_EXTRA_CLANG_TIDY_MODERNIZE_CLEANUPSTATICCASTCHECK_H
+
+#include "../ClangTidyCheck.h"
+
+namespace clang::tidy::modernize {
+
+/// Finds and removes static_cast where target type exactly matches source type.
+///
+/// This check helps clean up redundant static_cast operations that remain after
+/// type system changes, improving code readability and maintainability.
+///
+/// For the given code:
+/// \code
+/// size_t s = 42;
+/// foo(static_cast<size_t>(s));
+/// \endcode
+///
+/// The check will suggest removing the redundant cast:
+/// \code
+/// size_t s = 42;
+/// foo(s);
+/// \endcode
+///
+/// Note: This check intentionally ignores redundant casts in template instantiations
+/// as they might be needed for other template parameter types.
+class CleanupStaticCastCheck : public ClangTidyCheck {
+public:
+ CleanupStaticCastCheck(StringRef Name, ClangTidyContext *Context)
+ : ClangTidyCheck(Name, Context) {}
+ void registerMatchers(ast_matchers::MatchFinder *Finder) override;
+ void check(const ast_matchers::MatchFinder::MatchResult &Result) override;
+};
+
+} // namespace clang::tidy::modernize
+
+#endif // LLVM_CLANG_TOOLS_EXTRA_CLANG_TIDY_MODERNIZE_CLEANUPSTATICCASTCHECK_H
\ No newline at end of file
diff --git a/clang-tools-extra/clang-tidy/modernize/ModernizeTidyModule.cpp b/clang-tools-extra/clang-tidy/modernize/ModernizeTidyModule.cpp
index 18607593320635..364f1d6b2051f9 100644
--- a/clang-tools-extra/clang-tidy/modernize/ModernizeTidyModule.cpp
+++ b/clang-tools-extra/clang-tidy/modernize/ModernizeTidyModule.cpp
@@ -42,6 +42,7 @@
#include "UseNullptrCheck.h"
#include "UseOverrideCheck.h"
#include "UseRangesCheck.h"
+#include "CleanupStaticCastCheck.h"
#include "UseStartsEndsWithCheck.h"
#include "UseStdFormatCheck.h"
#include "UseStdNumbersCheck.h"
@@ -122,6 +123,8 @@ class ModernizeModule : public ClangTidyModule {
CheckFactories.registerCheck<UseUncaughtExceptionsCheck>(
"modernize-use-uncaught-exceptions");
CheckFactories.registerCheck<UseUsingCheck>("modernize-use-using");
+ CheckFactories.registerCheck<CleanupStaticCastCheck>(
+ "modernize-cleanup-static-cast");
}
};
diff --git a/clang-tools-extra/test/clang-tidy/checkers/modernize/modernize-cleanup-static-cast.cpp b/clang-tools-extra/test/clang-tidy/checkers/modernize/modernize-cleanup-static-cast.cpp
new file mode 100644
index 00000000000000..b25dfc20d9774c
--- /dev/null
+++ b/clang-tools-extra/test/clang-tidy/checkers/modernize/modernize-cleanup-static-cast.cpp
@@ -0,0 +1,42 @@
+// RUN: %check_clang_tidy %s modernize-cleanup-static-cast %t
+
+void foo(unsigned long x) {}
+void bar(int x) {}
+
+void test() {
+ unsigned long s = 42;
+ foo(static_cast<unsigned long>(s)); // Should warn
+ // CHECK-MESSAGES: [[@LINE-1]]:7: warning: redundant static_cast to the same type 'unsigned long' [modernize-cleanup-static-cast]
+ // CHECK-FIXES: foo(s);
+
+ // Different types - no warning
+ int i = 42;
+ foo(static_cast<unsigned long>(i));
+
+ // Test with typedef - should warn
+ typedef unsigned long my_ul_t;
+ my_ul_t ms = 42;
+ foo(static_cast<unsigned long>(ms));
+ // CHECK-MESSAGES: [[@LINE-1]]:7: warning: redundant static_cast to the same type 'unsigned long' [modernize-cleanup-static-cast]
+ // CHECK-FIXES: foo(ms);
+}
+
+// Template - no warnings
+template<typename T>
+void template_function(T value) {
+ foo(static_cast<unsigned long>(value));
+}
+
+void test_templates() {
+ template_function<unsigned long>(42);
+ template_function<int>(42);
+}
+
+// Test multiple casts
+void test_multiple() {
+ unsigned long s = 42;
+ foo(static_cast<unsigned long>(static_cast<unsigned long>(s)));
+ // CHECK-MESSAGES: [[@LINE-1]]:7: warning: redundant static_cast to the same type 'unsigned long' [modernize-cleanup-static-cast]
+ // CHECK-MESSAGES: [[@LINE-2]]:34: warning: redundant static_cast to the same type 'unsigned long' [modernize-cleanup-static-cast]
+ // CHECK-FIXES: foo(static_cast<unsigned long>(s));
+}
\ No newline at end of file
``````````
</details>
https://github.com/llvm/llvm-project/pull/118033
More information about the cfe-commits
mailing list