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 <hel...@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 <hel...@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); _______________________________________________ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
