[clang-tools-extra] [clang-tidy] Add modernize-cleanup-static-cast check (PR #118033)
Helmut Januschka via cfe-commits
cfe-commits at lists.llvm.org
Thu Nov 28 12:18:15 PST 2024
https://github.com/hjanuschka created https://github.com/llvm/llvm-project/pull/118033
> 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?
>From 062aebbdc5bb858cdc25c4995752b9c8a0eb34e8 Mon Sep 17 00:00:00 2001
From: Helmut Januschka <helmut at januschka.com>
Date: Thu, 28 Nov 2024 21:15:25 +0100
Subject: [PATCH 1/2] [clang-tidy] Add modernize-cleanup-static-cast check
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.
---
.../clang-tidy/modernize/CMakeLists.txt | 1 +
.../modernize/CleanupStaticCastCheck.cpp | 74 +++++++++++++++++++
.../modernize/CleanupStaticCastCheck.h | 38 ++++++++++
.../modernize/ModernizeTidyModule.cpp | 3 +
.../modernize-cleanup-static-cast.cpp | 42 +++++++++++
5 files changed, 158 insertions(+)
create mode 100644 clang-tools-extra/clang-tidy/modernize/CleanupStaticCastCheck.cpp
create mode 100644 clang-tools-extra/clang-tidy/modernize/CleanupStaticCastCheck.h
create mode 100644 clang-tools-extra/test/clang-tidy/checkers/modernize/modernize-cleanup-static-cast.cpp
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..545ce4c5a58357
--- /dev/null
+++ b/clang-tools-extra/clang-tidy/modernize/CleanupStaticCastCheck.cpp
@@ -0,0 +1,74 @@
+//===--- 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 {
+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();
+}
+} // namespace
+
+namespace clang::tidy::modernize {
+
+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;
+
+ // Get the source expression and its type
+ 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;
+
+ // Use our helper function to get the source text
+ std::string ReplacementText = getText(SubExpr, *Result.Context);
+
+ // Suggest removing the cast
+ 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
>From 5ee115e7cd83fe5a59c0284e91893b71df79be0c Mon Sep 17 00:00:00 2001
From: Helmut Januschka <helmut at januschka.com>
Date: Thu, 28 Nov 2024 21:17:10 +0100
Subject: [PATCH 2/2] feedback
---
.../clang-tidy/modernize/CleanupStaticCastCheck.cpp | 10 +++-------
1 file changed, 3 insertions(+), 7 deletions(-)
diff --git a/clang-tools-extra/clang-tidy/modernize/CleanupStaticCastCheck.cpp b/clang-tools-extra/clang-tidy/modernize/CleanupStaticCastCheck.cpp
index 545ce4c5a58357..1fdb0cff149883 100644
--- a/clang-tools-extra/clang-tidy/modernize/CleanupStaticCastCheck.cpp
+++ b/clang-tools-extra/clang-tidy/modernize/CleanupStaticCastCheck.cpp
@@ -16,15 +16,14 @@
using namespace clang::ast_matchers;
-namespace {
+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();
}
-} // namespace
-
-namespace clang::tidy::modernize {
void CleanupStaticCastCheck::registerMatchers(MatchFinder *Finder) {
// Match static_cast expressions not in templates
@@ -42,7 +41,6 @@ void CleanupStaticCastCheck::check(const MatchFinder::MatchResult &Result) {
if (!Cast)
return;
- // Get the source expression and its type
const Expr *SubExpr = Cast->getSubExpr()->IgnoreParenImpCasts();
QualType SourceType = SubExpr->getType();
QualType TargetType = Cast->getType();
@@ -61,10 +59,8 @@ void CleanupStaticCastCheck::check(const MatchFinder::MatchResult &Result) {
"redundant static_cast to the same type %0") // Removed single quotes
<< TargetType;
- // Use our helper function to get the source text
std::string ReplacementText = getText(SubExpr, *Result.Context);
- // Suggest removing the cast
Diag << FixItHint::CreateReplacement(
Cast->getSourceRange(),
ReplacementText);
More information about the cfe-commits
mailing list