https://github.com/DheerajAgarwal1234 updated https://github.com/llvm/llvm-project/pull/166822
>From 39db2936dd36c8631c190610314079d46bcb10c7 Mon Sep 17 00:00:00 2001 From: Dheeraj <[email protected]> Date: Thu, 6 Nov 2025 23:55:16 +0530 Subject: [PATCH 1/6] refactoring idea --- clang-tools-extra/CMakeLists.txt | 1 + .../clang-convert-ternary-if/CMakeLists.txt | 20 ++++ .../ConvertTernaryIf.cpp | 111 ++++++++++++++++++ .../ConvertTernaryIf.h | 39 ++++++ .../clang-convert-ternary-if/ToolMain.cpp | 80 +++++++++++++ 5 files changed, 251 insertions(+) create mode 100644 clang-tools-extra/clang-convert-ternary-if/CMakeLists.txt create mode 100644 clang-tools-extra/clang-convert-ternary-if/ConvertTernaryIf.cpp create mode 100644 clang-tools-extra/clang-convert-ternary-if/ConvertTernaryIf.h create mode 100644 clang-tools-extra/clang-convert-ternary-if/ToolMain.cpp diff --git a/clang-tools-extra/CMakeLists.txt b/clang-tools-extra/CMakeLists.txt index 87050db4e0e75..c0e6888b6209a 100644 --- a/clang-tools-extra/CMakeLists.txt +++ b/clang-tools-extra/CMakeLists.txt @@ -21,6 +21,7 @@ add_subdirectory(clang-apply-replacements) add_subdirectory(clang-reorder-fields) add_subdirectory(modularize) add_subdirectory(clang-tidy) +add_subdirectory(clang-convert-ternary-if) add_subdirectory(clang-change-namespace) add_subdirectory(clang-doc) diff --git a/clang-tools-extra/clang-convert-ternary-if/CMakeLists.txt b/clang-tools-extra/clang-convert-ternary-if/CMakeLists.txt new file mode 100644 index 0000000000000..780fca4405e64 --- /dev/null +++ b/clang-tools-extra/clang-convert-ternary-if/CMakeLists.txt @@ -0,0 +1,20 @@ +#===- CMakeLists.txt - clang-convert-ternary-if tool ---------------------===# +# This defines the build configuration for the standalone refactoring tool. + +add_clang_executable(clang-convert-ternary-if + ConvertTernaryIf.cpp + ToolMain.cpp +) + +target_link_libraries(clang-convert-ternary-if + PRIVATE + clangTooling + clangBasic + clangAST + clangASTMatchers + clangRewrite + clangFrontend +) + +install(TARGETS clang-convert-ternary-if RUNTIME DESTINATION bin) + diff --git a/clang-tools-extra/clang-convert-ternary-if/ConvertTernaryIf.cpp b/clang-tools-extra/clang-convert-ternary-if/ConvertTernaryIf.cpp new file mode 100644 index 0000000000000..8843510d08b72 --- /dev/null +++ b/clang-tools-extra/clang-convert-ternary-if/ConvertTernaryIf.cpp @@ -0,0 +1,111 @@ +//===--- ConvertTernaryIf.cpp ---------------------------------------------===// +// +// Implements a tool that refactors between ternary (?:) expressions +// and equivalent if/else statements. +// +// Usage Example: +// clang-convert-ternary-if test.cpp -- +// +//===----------------------------------------------------------------------===// + +#include "ConvertTernaryIf.h" +#include "clang/ASTMatchers/ASTMatchFinder.h" +#include "clang/Rewrite/Core/Rewriter.h" +#include "clang/Lex/Lexer.h" +#include "llvm/Support/raw_ostream.h" + +using namespace clang; +using namespace clang::ast_matchers; + +namespace clang { +namespace convertternary { + + +// Callback: Called when a match is found +void ConvertTernaryIfCallback::run(const MatchFinder::MatchResult &Result) { + //Initialize the Rewriter safely (fixes segmentation fault) + if (!IsInitialized) { + if (Result.SourceManager && Result.Context) { + TheRewriter.setSourceMgr(*Result.SourceManager, + Result.Context->getLangOpts()); + IsInitialized = true; + llvm::errs() << "Rewriter initialized successfully.\n"; + } else { + llvm::errs() << "Error: Missing SourceManager or Context.\n"; + return; + } + } + + const auto *CondOp = Result.Nodes.getNodeAs<ConditionalOperator>("condOp"); + const auto *IfStmtNode = Result.Nodes.getNodeAs<IfStmt>("ifStmt"); + + const SourceManager &SM = *Result.SourceManager; + + // === Convert Ternary -> If === + if (CondOp) { + const Expr *Cond = CondOp->getCond(); + const Expr *TrueExpr = CondOp->getTrueExpr(); + const Expr *FalseExpr = CondOp->getFalseExpr(); + + auto getText = [&](const Expr *E) -> std::string { + return Lexer::getSourceText(CharSourceRange::getTokenRange(E->getSourceRange()), SM, + Result.Context->getLangOpts()) + .str(); + }; + + std::string CondText = getText(Cond); + std::string TrueText = getText(TrueExpr); + std::string FalseText = getText(FalseExpr); + + std::string IfReplacement = "if (" + CondText + ") {\n " + TrueText + + ";\n} else {\n " + FalseText + ";\n}"; + + TheRewriter.ReplaceText(CondOp->getSourceRange(), IfReplacement); + llvm::errs() << "Converted ternary to if/else.\n"; + } + + // === Convert If -> Ternary === + if (IfStmtNode) { + const Expr *Cond = IfStmtNode->getCond(); + const Stmt *Then = IfStmtNode->getThen(); + const Stmt *Else = IfStmtNode->getElse(); + + if (!Then || !Else) + return; + + auto getTextStmt = [&](const Stmt *S) -> std::string { + return Lexer::getSourceText(CharSourceRange::getTokenRange(S->getSourceRange()), SM, + Result.Context->getLangOpts()) + .str(); + }; + + std::string CondText = Lexer::getSourceText( + CharSourceRange::getTokenRange(Cond->getSourceRange()), SM, + Result.Context->getLangOpts()) + .str(); + + std::string ThenText = getTextStmt(Then); + std::string ElseText = getTextStmt(Else); + + std::string Ternary = + "(" + CondText + ") ? " + ThenText + " : " + ElseText + ";"; + + TheRewriter.ReplaceText(IfStmtNode->getSourceRange(), Ternary); + llvm::errs() << "Converted if/else to ternary.\n"; + } +} + +// === Register AST Matchers === +void setupMatchers(MatchFinder &Finder, ConvertTernaryIfCallback &Callback) { + Finder.addMatcher( + conditionalOperator(isExpansionInMainFile()).bind("condOp"), &Callback); + + Finder.addMatcher( + ifStmt(hasThen(stmt()), hasElse(stmt()), isExpansionInMainFile()) + .bind("ifStmt"), + &Callback); +} + +} // namespace convertternary +} // namespace clang + diff --git a/clang-tools-extra/clang-convert-ternary-if/ConvertTernaryIf.h b/clang-tools-extra/clang-convert-ternary-if/ConvertTernaryIf.h new file mode 100644 index 0000000000000..2d71cf5f8242d --- /dev/null +++ b/clang-tools-extra/clang-convert-ternary-if/ConvertTernaryIf.h @@ -0,0 +1,39 @@ +//===--- ConvertTernaryIf.h ------------------------------------*- C++ -*-===// +// +// This file declares the refactoring logic that converts +// ternary operators (?:) into if/else statements and vice versa. +// +//===----------------------------------------------------------------------===// + +#ifndef LLVM_CLANG_CONVERT_TERNARY_IF_H +#define LLVM_CLANG_CONVERT_TERNARY_IF_H + +#include "clang/AST/AST.h" +#include "clang/ASTMatchers/ASTMatchFinder.h" +#include "clang/Tooling/Refactoring.h" +#include "clang/Rewrite/Core/Rewriter.h" + +namespace clang { +namespace convertternary { + +class ConvertTernaryIfCallback + : public ast_matchers::MatchFinder::MatchCallback { +public: + ConvertTernaryIfCallback(Rewriter &R) + : TheRewriter(R), IsInitialized(false) {} + + void run(const ast_matchers::MatchFinder::MatchResult &Result) override; + +private: + Rewriter &TheRewriter; + bool IsInitialized; + }; + +void setupMatchers(ast_matchers::MatchFinder &Finder, + ConvertTernaryIfCallback &Callback); + +} // namespace convertternary +} // namespace clang + +#endif + diff --git a/clang-tools-extra/clang-convert-ternary-if/ToolMain.cpp b/clang-tools-extra/clang-convert-ternary-if/ToolMain.cpp new file mode 100644 index 0000000000000..bf9a793309944 --- /dev/null +++ b/clang-tools-extra/clang-convert-ternary-if/ToolMain.cpp @@ -0,0 +1,80 @@ +//===--- ToolMain.cpp - Entry point for clang-convert-ternary-if ----------===// +// +// This tool runs the refactoring logic defined in ConvertTernaryIf.cpp. +// +// Usage: +// clang-convert-ternary-if <source-file> -- +// +// It prints the rewritten (refactored) source code to stdout. +// +//===----------------------------------------------------------------------===// + +#include "ConvertTernaryIf.h" +#include "clang/Tooling/CommonOptionsParser.h" +#include "clang/Tooling/Tooling.h" +#include "clang/Frontend/TextDiagnosticPrinter.h" +#include "llvm/Support/CommandLine.h" +#include "llvm/Support/raw_ostream.h" + +using namespace clang; +using namespace clang::tooling; +using namespace clang::convertternary; +using namespace llvm; + +static llvm::cl::OptionCategory ToolCategory("convert-ternary-if options"); + +int main(int argc, const char **argv) { + // Parse command-line options + auto ExpectedParser = + CommonOptionsParser::create(argc, argv, ToolCategory, cl::ZeroOrMore); + if (!ExpectedParser) { + llvm::errs() << ExpectedParser.takeError(); + return 1; + } + + CommonOptionsParser &OptionsParser = ExpectedParser.get(); + ClangTool Tool(OptionsParser.getCompilations(), + OptionsParser.getSourcePathList()); + + // Set up the Rewriter and the Matcher + clang::Rewriter Rewrite; + ast_matchers::MatchFinder Finder; + ConvertTernaryIfCallback Callback(Rewrite); + setupMatchers(Finder, Callback); + + llvm::outs() << "=== Running clang-convert-ternary-if ===\n"; + int Result = Tool.run(newFrontendActionFactory(&Finder).get()); + + if (Result != 0) { + llvm::errs() << "Error: Tool execution failed.\n"; + return Result; + } + + // No changes? + if (Rewrite.buffer_begin() == Rewrite.buffer_end()) { + llvm::outs() << "No changes made.\n"; + return 0; + } + + llvm::outs() << "\n=== Rewritten Files ===\n"; + + // Print all rewritten files + for (auto It = Rewrite.buffer_begin(); It != Rewrite.buffer_end(); ++It) { + clang::FileID FID = It->first; + const llvm::RewriteBuffer &RewriteBuf = It->second; + const clang::SourceManager &SM = Rewrite.getSourceMgr(); + + // Get the filename safely + llvm::StringRef FileName = SM.getFilename(SM.getLocForStartOfFile(FID)); + if (FileName.empty()) + FileName = "<unknown file>"; + + llvm::outs() << "\n--- " << FileName << " ---\n"; + RewriteBuf.write(llvm::outs()); + llvm::outs() << "\n"; + } + + llvm::outs() << "\n=== Refactoring complete ===\n"; + return 0; +} + >From bc62ab53a297d6ce354c787d5ee801dbd1b1ff20 Mon Sep 17 00:00:00 2001 From: Dheeraj <[email protected]> Date: Fri, 7 Nov 2025 18:20:40 +0530 Subject: [PATCH 2/6] Add modernize-conditional-to-if clang-tidy check --- clang-tools-extra/CMakeLists.txt | 1 - .../clang-convert-ternary-if/CMakeLists.txt | 20 -- .../ConvertTernaryIf.cpp | 111 ------ .../ConvertTernaryIf.h | 39 --- .../clang-convert-ternary-if/ToolMain.cpp | 80 ----- .../clang-tidy/modernize/CMakeLists.txt | 1 + .../modernize/ConditionalToIfCheck.cpp | 328 ++++++++++++++++++ .../modernize/ConditionalToIfCheck.h | 39 +++ .../modernize/ModernizeTidyModule.cpp | 2 + 9 files changed, 370 insertions(+), 251 deletions(-) delete mode 100644 clang-tools-extra/clang-convert-ternary-if/CMakeLists.txt delete mode 100644 clang-tools-extra/clang-convert-ternary-if/ConvertTernaryIf.cpp delete mode 100644 clang-tools-extra/clang-convert-ternary-if/ConvertTernaryIf.h delete mode 100644 clang-tools-extra/clang-convert-ternary-if/ToolMain.cpp create mode 100644 clang-tools-extra/clang-tidy/modernize/ConditionalToIfCheck.cpp create mode 100644 clang-tools-extra/clang-tidy/modernize/ConditionalToIfCheck.h diff --git a/clang-tools-extra/CMakeLists.txt b/clang-tools-extra/CMakeLists.txt index c0e6888b6209a..87050db4e0e75 100644 --- a/clang-tools-extra/CMakeLists.txt +++ b/clang-tools-extra/CMakeLists.txt @@ -21,7 +21,6 @@ add_subdirectory(clang-apply-replacements) add_subdirectory(clang-reorder-fields) add_subdirectory(modularize) add_subdirectory(clang-tidy) -add_subdirectory(clang-convert-ternary-if) add_subdirectory(clang-change-namespace) add_subdirectory(clang-doc) diff --git a/clang-tools-extra/clang-convert-ternary-if/CMakeLists.txt b/clang-tools-extra/clang-convert-ternary-if/CMakeLists.txt deleted file mode 100644 index 780fca4405e64..0000000000000 --- a/clang-tools-extra/clang-convert-ternary-if/CMakeLists.txt +++ /dev/null @@ -1,20 +0,0 @@ -#===- CMakeLists.txt - clang-convert-ternary-if tool ---------------------===# -# This defines the build configuration for the standalone refactoring tool. - -add_clang_executable(clang-convert-ternary-if - ConvertTernaryIf.cpp - ToolMain.cpp -) - -target_link_libraries(clang-convert-ternary-if - PRIVATE - clangTooling - clangBasic - clangAST - clangASTMatchers - clangRewrite - clangFrontend -) - -install(TARGETS clang-convert-ternary-if RUNTIME DESTINATION bin) - diff --git a/clang-tools-extra/clang-convert-ternary-if/ConvertTernaryIf.cpp b/clang-tools-extra/clang-convert-ternary-if/ConvertTernaryIf.cpp deleted file mode 100644 index 8843510d08b72..0000000000000 --- a/clang-tools-extra/clang-convert-ternary-if/ConvertTernaryIf.cpp +++ /dev/null @@ -1,111 +0,0 @@ -//===--- ConvertTernaryIf.cpp ---------------------------------------------===// -// -// Implements a tool that refactors between ternary (?:) expressions -// and equivalent if/else statements. -// -// Usage Example: -// clang-convert-ternary-if test.cpp -- -// -//===----------------------------------------------------------------------===// - -#include "ConvertTernaryIf.h" -#include "clang/ASTMatchers/ASTMatchFinder.h" -#include "clang/Rewrite/Core/Rewriter.h" -#include "clang/Lex/Lexer.h" -#include "llvm/Support/raw_ostream.h" - -using namespace clang; -using namespace clang::ast_matchers; - -namespace clang { -namespace convertternary { - - -// Callback: Called when a match is found -void ConvertTernaryIfCallback::run(const MatchFinder::MatchResult &Result) { - //Initialize the Rewriter safely (fixes segmentation fault) - if (!IsInitialized) { - if (Result.SourceManager && Result.Context) { - TheRewriter.setSourceMgr(*Result.SourceManager, - Result.Context->getLangOpts()); - IsInitialized = true; - llvm::errs() << "Rewriter initialized successfully.\n"; - } else { - llvm::errs() << "Error: Missing SourceManager or Context.\n"; - return; - } - } - - const auto *CondOp = Result.Nodes.getNodeAs<ConditionalOperator>("condOp"); - const auto *IfStmtNode = Result.Nodes.getNodeAs<IfStmt>("ifStmt"); - - const SourceManager &SM = *Result.SourceManager; - - // === Convert Ternary -> If === - if (CondOp) { - const Expr *Cond = CondOp->getCond(); - const Expr *TrueExpr = CondOp->getTrueExpr(); - const Expr *FalseExpr = CondOp->getFalseExpr(); - - auto getText = [&](const Expr *E) -> std::string { - return Lexer::getSourceText(CharSourceRange::getTokenRange(E->getSourceRange()), SM, - Result.Context->getLangOpts()) - .str(); - }; - - std::string CondText = getText(Cond); - std::string TrueText = getText(TrueExpr); - std::string FalseText = getText(FalseExpr); - - std::string IfReplacement = "if (" + CondText + ") {\n " + TrueText + - ";\n} else {\n " + FalseText + ";\n}"; - - TheRewriter.ReplaceText(CondOp->getSourceRange(), IfReplacement); - llvm::errs() << "Converted ternary to if/else.\n"; - } - - // === Convert If -> Ternary === - if (IfStmtNode) { - const Expr *Cond = IfStmtNode->getCond(); - const Stmt *Then = IfStmtNode->getThen(); - const Stmt *Else = IfStmtNode->getElse(); - - if (!Then || !Else) - return; - - auto getTextStmt = [&](const Stmt *S) -> std::string { - return Lexer::getSourceText(CharSourceRange::getTokenRange(S->getSourceRange()), SM, - Result.Context->getLangOpts()) - .str(); - }; - - std::string CondText = Lexer::getSourceText( - CharSourceRange::getTokenRange(Cond->getSourceRange()), SM, - Result.Context->getLangOpts()) - .str(); - - std::string ThenText = getTextStmt(Then); - std::string ElseText = getTextStmt(Else); - - std::string Ternary = - "(" + CondText + ") ? " + ThenText + " : " + ElseText + ";"; - - TheRewriter.ReplaceText(IfStmtNode->getSourceRange(), Ternary); - llvm::errs() << "Converted if/else to ternary.\n"; - } -} - -// === Register AST Matchers === -void setupMatchers(MatchFinder &Finder, ConvertTernaryIfCallback &Callback) { - Finder.addMatcher( - conditionalOperator(isExpansionInMainFile()).bind("condOp"), &Callback); - - Finder.addMatcher( - ifStmt(hasThen(stmt()), hasElse(stmt()), isExpansionInMainFile()) - .bind("ifStmt"), - &Callback); -} - -} // namespace convertternary -} // namespace clang - diff --git a/clang-tools-extra/clang-convert-ternary-if/ConvertTernaryIf.h b/clang-tools-extra/clang-convert-ternary-if/ConvertTernaryIf.h deleted file mode 100644 index 2d71cf5f8242d..0000000000000 --- a/clang-tools-extra/clang-convert-ternary-if/ConvertTernaryIf.h +++ /dev/null @@ -1,39 +0,0 @@ -//===--- ConvertTernaryIf.h ------------------------------------*- C++ -*-===// -// -// This file declares the refactoring logic that converts -// ternary operators (?:) into if/else statements and vice versa. -// -//===----------------------------------------------------------------------===// - -#ifndef LLVM_CLANG_CONVERT_TERNARY_IF_H -#define LLVM_CLANG_CONVERT_TERNARY_IF_H - -#include "clang/AST/AST.h" -#include "clang/ASTMatchers/ASTMatchFinder.h" -#include "clang/Tooling/Refactoring.h" -#include "clang/Rewrite/Core/Rewriter.h" - -namespace clang { -namespace convertternary { - -class ConvertTernaryIfCallback - : public ast_matchers::MatchFinder::MatchCallback { -public: - ConvertTernaryIfCallback(Rewriter &R) - : TheRewriter(R), IsInitialized(false) {} - - void run(const ast_matchers::MatchFinder::MatchResult &Result) override; - -private: - Rewriter &TheRewriter; - bool IsInitialized; - }; - -void setupMatchers(ast_matchers::MatchFinder &Finder, - ConvertTernaryIfCallback &Callback); - -} // namespace convertternary -} // namespace clang - -#endif - diff --git a/clang-tools-extra/clang-convert-ternary-if/ToolMain.cpp b/clang-tools-extra/clang-convert-ternary-if/ToolMain.cpp deleted file mode 100644 index bf9a793309944..0000000000000 --- a/clang-tools-extra/clang-convert-ternary-if/ToolMain.cpp +++ /dev/null @@ -1,80 +0,0 @@ -//===--- ToolMain.cpp - Entry point for clang-convert-ternary-if ----------===// -// -// This tool runs the refactoring logic defined in ConvertTernaryIf.cpp. -// -// Usage: -// clang-convert-ternary-if <source-file> -- -// -// It prints the rewritten (refactored) source code to stdout. -// -//===----------------------------------------------------------------------===// - -#include "ConvertTernaryIf.h" -#include "clang/Tooling/CommonOptionsParser.h" -#include "clang/Tooling/Tooling.h" -#include "clang/Frontend/TextDiagnosticPrinter.h" -#include "llvm/Support/CommandLine.h" -#include "llvm/Support/raw_ostream.h" - -using namespace clang; -using namespace clang::tooling; -using namespace clang::convertternary; -using namespace llvm; - -static llvm::cl::OptionCategory ToolCategory("convert-ternary-if options"); - -int main(int argc, const char **argv) { - // Parse command-line options - auto ExpectedParser = - CommonOptionsParser::create(argc, argv, ToolCategory, cl::ZeroOrMore); - if (!ExpectedParser) { - llvm::errs() << ExpectedParser.takeError(); - return 1; - } - - CommonOptionsParser &OptionsParser = ExpectedParser.get(); - ClangTool Tool(OptionsParser.getCompilations(), - OptionsParser.getSourcePathList()); - - // Set up the Rewriter and the Matcher - clang::Rewriter Rewrite; - ast_matchers::MatchFinder Finder; - ConvertTernaryIfCallback Callback(Rewrite); - setupMatchers(Finder, Callback); - - llvm::outs() << "=== Running clang-convert-ternary-if ===\n"; - int Result = Tool.run(newFrontendActionFactory(&Finder).get()); - - if (Result != 0) { - llvm::errs() << "Error: Tool execution failed.\n"; - return Result; - } - - // No changes? - if (Rewrite.buffer_begin() == Rewrite.buffer_end()) { - llvm::outs() << "No changes made.\n"; - return 0; - } - - llvm::outs() << "\n=== Rewritten Files ===\n"; - - // Print all rewritten files - for (auto It = Rewrite.buffer_begin(); It != Rewrite.buffer_end(); ++It) { - clang::FileID FID = It->first; - const llvm::RewriteBuffer &RewriteBuf = It->second; - const clang::SourceManager &SM = Rewrite.getSourceMgr(); - - // Get the filename safely - llvm::StringRef FileName = SM.getFilename(SM.getLocForStartOfFile(FID)); - if (FileName.empty()) - FileName = "<unknown file>"; - - llvm::outs() << "\n--- " << FileName << " ---\n"; - RewriteBuf.write(llvm::outs()); - llvm::outs() << "\n"; - } - - llvm::outs() << "\n=== Refactoring complete ===\n"; - return 0; -} - diff --git a/clang-tools-extra/clang-tidy/modernize/CMakeLists.txt b/clang-tools-extra/clang-tidy/modernize/CMakeLists.txt index 882f2dc9fb4d8..608a81a7f200c 100644 --- a/clang-tools-extra/clang-tidy/modernize/CMakeLists.txt +++ b/clang-tools-extra/clang-tidy/modernize/CMakeLists.txt @@ -53,6 +53,7 @@ add_clang_library(clangTidyModernizeModule STATIC UseTransparentFunctorsCheck.cpp UseUncaughtExceptionsCheck.cpp UseUsingCheck.cpp + ConditionalToIfCheck.cpp LINK_LIBS clangTidy diff --git a/clang-tools-extra/clang-tidy/modernize/ConditionalToIfCheck.cpp b/clang-tools-extra/clang-tidy/modernize/ConditionalToIfCheck.cpp new file mode 100644 index 0000000000000..22e6c47058d88 --- /dev/null +++ b/clang-tools-extra/clang-tidy/modernize/ConditionalToIfCheck.cpp @@ -0,0 +1,328 @@ +#include "ConditionalToIfCheck.h" + +#include "clang/AST/ASTContext.h" +#include "clang/AST/Expr.h" +#include "clang/ASTMatchers/ASTMatchFinder.h" +#include "clang/Basic/SourceManager.h" +#include "clang/Lex/Lexer.h" +#include "llvm/ADT/StringSwitch.h" + +using namespace clang; +using namespace clang::ast_matchers; + +namespace clang::tidy::modernize { + +namespace { + +bool isInMacro(const SourceRange &R, const MatchFinder::MatchResult &Res) { + return R.getBegin().isMacroID() || R.getEnd().isMacroID(); +} + +CharSourceRange tokenRange(const SourceRange &R, + const MatchFinder::MatchResult &Res) { + return CharSourceRange::getTokenRange(R); +} + +std::string getTokenText(const SourceRange &R, + const MatchFinder::MatchResult &Res) { + return Lexer::getSourceText(tokenRange(R, Res), + *Res.SourceManager, Res.Context->getLangOpts()) + .str(); +} + +const Expr *strip(const Expr *E) { return E ? E->IgnoreParenImpCasts() : E; } + +// Find the statement that directly contains E (best-effort). +const Stmt *enclosingStmt(const Expr *E, const MatchFinder::MatchResult &Res) { + if (!E) return nullptr; + const Stmt *Cur = E; + while (true) { + auto Parents = Res.Context->getParents(*Cur); + if (Parents.empty()) return Cur; + if (const Stmt *S = Parents[0].get<Stmt>()) { + Cur = S; + // Stop when Cur itself is a statement that could be replaced wholesale. + if (isa<ExprWithCleanups>(Cur) || isa<ReturnStmt>(Cur) || + isa<CompoundStmt>(Cur) || isa<IfStmt>(Cur) || + isa<DeclStmt>(Cur) || isa<Expr>(Cur)) + continue; + return Cur; + } else { + return Cur; + } + } +} + +} // namespace + +ConditionalToIfCheck::ConditionalToIfCheck(StringRef Name, + ClangTidyContext *Ctx) + : ClangTidyCheck(Name, Ctx) { + StringRef V = Options.get("PreferredForm", "if"); + // Use StringSwitch for broad LLVM version compatibility. + PreferredForm = llvm::StringSwitch<Preferred>(V) + .CaseLower("conditional", Preferred::Conditional) + .Default(Preferred::If); +} + +void ConditionalToIfCheck::storeOptions(ClangTidyOptions::OptionMap &Opts) { + Options.store(Opts, "PreferredForm", + PreferredForm == Preferred::If ? "if" : "conditional"); +} + +void ConditionalToIfCheck::registerMatchers(MatchFinder *Finder) { + // Only match code written in the main file to avoid noisy headers. + auto InMain = isExpansionInMainFile(); + + if (PreferredForm == Preferred::If) { + // 1) x = cond ? a : b; + Finder->addMatcher( + binaryOperator(InMain, isAssignmentOperator(), + hasRHS(conditionalOperator().bind("condop")), + hasLHS(expr().bind("assignLHS"))) + .bind("assign"), + this); + + // 2) return cond ? a : b; + Finder->addMatcher( + returnStmt(InMain, + hasReturnValue(conditionalOperator().bind("retop"))) + .bind("ret"), + this); + } else { + // Preferred::Conditional + + // 3a) if (cond) x = A; else x = B; + // (Match a simple assignment in each branch; allow it to be wrapped in + // an ExprWithCleanups/ExprStmt or a single-statement compound.) + auto AssignThen = + binaryOperator(isAssignmentOperator()) + .bind("thenA"); + auto AssignElse = + binaryOperator(isAssignmentOperator()) + .bind("elseA"); + + Finder->addMatcher( + ifStmt(InMain, + hasThen(anyOf( + hasDescendant(AssignThen), + compoundStmt(statementCountIs(1), + hasAnySubstatement(hasDescendant(AssignThen))))), + hasElse(anyOf( + hasDescendant(AssignElse), + compoundStmt(statementCountIs(1), + hasAnySubstatement(hasDescendant(AssignElse)))))) + .bind("ifAssign"), + this); + + // 3b) if (cond) return A; else return B; + auto RetThen = returnStmt(hasReturnValue(expr().bind("thenR"))).bind("thenRet"); + auto RetElse = returnStmt(hasReturnValue(expr().bind("elseR"))).bind("elseRet"); + + Finder->addMatcher( + ifStmt(InMain, + hasThen(anyOf(RetThen, + compoundStmt(statementCountIs(1), + hasAnySubstatement(RetThen)))), + hasElse(anyOf(RetElse, + compoundStmt(statementCountIs(1), + hasAnySubstatement(RetElse))))) + .bind("ifReturn"), + this); + } +} + +bool ConditionalToIfCheck::locationsAreOK( + const SourceRange &R, const MatchFinder::MatchResult &Rst) { + if (R.isInvalid()) + return false; + if (isInMacro(R, Rst)) + return false; + if (!Rst.SourceManager->isWrittenInMainFile(R.getBegin())) + return false; + return true; +} + +std::string ConditionalToIfCheck::getText( + const SourceRange &R, const MatchFinder::MatchResult &Rst) { + return getTokenText(R, Rst); +} + +bool ConditionalToIfCheck::hasObviousSideEffects(const Expr *E, + ASTContext &Ctx) { + if (!E) + return true; + E = strip(E); + + // Very conservative: rely on Clang's side-effect query. + if (E->HasSideEffects(Ctx)) + return true; + + // Additional heuristics for common side-effect nodes. + if (isa<CallExpr>(E) || isa<CXXConstructExpr>(E) || isa<CXXOperatorCallExpr>(E)) + return true; + + if (const auto *U = dyn_cast<UnaryOperator>(E)) { + if (U->isIncrementDecrementOp() || U->getOpcode() == UO_AddrOf || + U->getOpcode() == UO_Deref) + return true; + } + if (const auto *B = dyn_cast<BinaryOperator>(E)) { + if (B->isAssignmentOp() || B->getOpcode() == BO_Comma) + return true; + } + return false; +} + +void ConditionalToIfCheck::check(const MatchFinder::MatchResult &Res) { + ASTContext &Ctx = *Res.Context; + + if (PreferredForm == Preferred::If) { + // Handle: return cond ? a : b; + if (const auto *Ret = Res.Nodes.getNodeAs<ReturnStmt>("ret")) { + const auto *CO = Res.Nodes.getNodeAs<ConditionalOperator>("retop"); + if (!CO) return; + + const Expr *Cond = strip(CO->getCond()); + const Expr *TrueE = strip(CO->getTrueExpr()); + const Expr *FalseE = strip(CO->getFalseExpr()); + + if (hasObviousSideEffects(Cond, Ctx) || + hasObviousSideEffects(TrueE, Ctx) || + hasObviousSideEffects(FalseE, Ctx)) + return; + + SourceRange SR = Ret->getSourceRange(); + if (!locationsAreOK(SR, Res)) + return; + + std::string CondS = getText(Cond->getSourceRange(), Res); + std::string TS = getText(TrueE->getSourceRange(), Res); + std::string FS = getText(FalseE->getSourceRange(), Res); + + std::string Replacement = "if (" + CondS + ") {\n return " + TS + + ";\n} else {\n return " + FS + ";\n}"; + diag(Ret->getBeginLoc(), + "replace simple conditional return with if/else") + << FixItHint::CreateReplacement(CharSourceRange::getCharRange(SR), + Replacement); + return; + } + + // Handle: x = cond ? a : b; + if (const auto *Assign = Res.Nodes.getNodeAs<BinaryOperator>("assign")) { + const auto *CO = Res.Nodes.getNodeAs<ConditionalOperator>("condop"); + const auto *LHS = Res.Nodes.getNodeAs<Expr>("assignLHS"); + if (!CO || !LHS) return; + + const Expr *Cond = strip(CO->getCond()); + const Expr *TrueE = strip(CO->getTrueExpr()); + const Expr *FalseE = strip(CO->getFalseExpr()); + + if (hasObviousSideEffects(Cond, Ctx) || + hasObviousSideEffects(TrueE, Ctx) || + hasObviousSideEffects(FalseE, Ctx) || + hasObviousSideEffects(LHS, Ctx)) + return; + + const Stmt *Carrier = enclosingStmt(Assign, Res); + if (!Carrier) + Carrier = Assign; + + SourceRange SR = Carrier->getSourceRange(); + if (!locationsAreOK(SR, Res)) + return; + + std::string LHSS = getText(LHS->getSourceRange(), Res); + std::string CondS = getText(Cond->getSourceRange(), Res); + std::string TS = getText(TrueE->getSourceRange(), Res); + std::string FS = getText(FalseE->getSourceRange(), Res); + + std::string Replacement = "if (" + CondS + ") {\n " + LHSS + " = " + TS + + ";\n} else {\n " + LHSS + " = " + FS + + ";\n}"; + diag(Carrier->getBeginLoc(), + "replace simple conditional assignment with if/else") + << FixItHint::CreateReplacement(CharSourceRange::getCharRange(SR), + Replacement); + return; + } + return; + } + + // PreferredForm == Conditional + + // if (cond) return A; else return B; + if (const auto *IfR = Res.Nodes.getNodeAs<IfStmt>("ifReturn")) { + const Expr *Cond = strip(IfR->getCond()); + const Expr *ThenR = strip(Res.Nodes.getNodeAs<Expr>("thenR")); + const Expr *ElseR = strip(Res.Nodes.getNodeAs<Expr>("elseR")); + if (!Cond || !ThenR || !ElseR) return; + + if (hasObviousSideEffects(Cond, Ctx) || + hasObviousSideEffects(ThenR, Ctx) || + hasObviousSideEffects(ElseR, Ctx)) + return; + + SourceRange SR = IfR->getSourceRange(); + if (!locationsAreOK(SR, Res)) + return; + + std::string CondS = getText(Cond->getSourceRange(), Res); + std::string TS = getText(ThenR->getSourceRange(), Res); + std::string FS = getText(ElseR->getSourceRange(), Res); + + std::string Replacement = + "return (" + CondS + ") ? " + TS + " : " + FS + ";"; + diag(IfR->getBeginLoc(), "replace simple if/else with conditional return") + << FixItHint::CreateReplacement(CharSourceRange::getCharRange(SR), + Replacement); + return; + } + + // if (cond) x = A; else x = B; + if (const auto *IfA = Res.Nodes.getNodeAs<IfStmt>("ifAssign")) { + const Expr *Cond = strip(IfA->getCond()); + const auto *ThenA = Res.Nodes.getNodeAs<BinaryOperator>("thenA"); + const auto *ElseA = Res.Nodes.getNodeAs<BinaryOperator>("elseA"); + if (!Cond || !ThenA || !ElseA) + return; + + const Expr *ThenL = strip(ThenA->getLHS()); + const Expr *ElseL = strip(ElseA->getLHS()); + const Expr *ThenR = strip(ThenA->getRHS()); + const Expr *ElseR = strip(ElseA->getRHS()); + if (!ThenL || !ElseL || !ThenR || !ElseR) + return; + + // LHS must be textually identical (safe & simple). + std::string LThen = getText(ThenL->getSourceRange(), Res); + std::string LElse = getText(ElseL->getSourceRange(), Res); + if (LThen != LElse) + return; + + if (hasObviousSideEffects(Cond, Ctx) || + hasObviousSideEffects(ThenR, Ctx) || + hasObviousSideEffects(ElseR, Ctx) || + hasObviousSideEffects(ThenL, Ctx)) + return; + + SourceRange SR = IfA->getSourceRange(); + if (!locationsAreOK(SR, Res)) + return; + + std::string CondS = getText(Cond->getSourceRange(), Res); + std::string TS = getText(ThenR->getSourceRange(), Res); + std::string FS = getText(ElseR->getSourceRange(), Res); + + std::string Replacement = + LThen + " = (" + CondS + ") ? " + TS + " : " + FS + ";"; + diag(IfA->getBeginLoc(), + "replace simple if/else assignment with conditional expression") + << FixItHint::CreateReplacement(CharSourceRange::getCharRange(SR), + Replacement); + return; + } +} + +} // namespace clang::tidy::modernize diff --git a/clang-tools-extra/clang-tidy/modernize/ConditionalToIfCheck.h b/clang-tools-extra/clang-tidy/modernize/ConditionalToIfCheck.h new file mode 100644 index 0000000000000..95d159b580d37 --- /dev/null +++ b/clang-tools-extra/clang-tidy/modernize/ConditionalToIfCheck.h @@ -0,0 +1,39 @@ +#ifndef LLVM_CLANG_TOOLS_EXTRA_CLANG_TIDY_MODERNIZE_CONDITIONALTOIFCHECK_H +#define LLVM_CLANG_TOOLS_EXTRA_CLANG_TIDY_MODERNIZE_CONDITIONALTOIFCHECK_H + +#include "../ClangTidyCheck.h" + +namespace clang::tidy::modernize { + +/// Convert between simple conditional (?:) expressions and equivalent if/else +/// statements in safe, syntactically simple cases. +/// +/// Direction is controlled by the option: +/// - modernize-conditional-to-if.PreferredForm: "if" | "conditional" +class ConditionalToIfCheck : public ClangTidyCheck { +public: + ConditionalToIfCheck(llvm::StringRef Name, ClangTidyContext *Context); + + void storeOptions(ClangTidyOptions::OptionMap &Opts) override; + void registerMatchers(ast_matchers::MatchFinder *Finder) override; + void check(const ast_matchers::MatchFinder::MatchResult &Result) override; + +private: + enum class Preferred { If, Conditional }; + Preferred PreferredForm; + + // Conservative side-effect check (needs ASTContext). + static bool hasObviousSideEffects(const Expr *E, ASTContext &Ctx); + + // Fetch exact source text for a range. + static std::string getText(const SourceRange &R, + const ast_matchers::MatchFinder::MatchResult &Rst); + + // Ensure locations are in main file and not in macros. + static bool locationsAreOK(const SourceRange &R, + const ast_matchers::MatchFinder::MatchResult &Rst); +}; + +} // namespace clang::tidy::modernize + +#endif diff --git a/clang-tools-extra/clang-tidy/modernize/ModernizeTidyModule.cpp b/clang-tools-extra/clang-tidy/modernize/ModernizeTidyModule.cpp index 360e2b8434d0c..527040a4fadd2 100644 --- a/clang-tools-extra/clang-tidy/modernize/ModernizeTidyModule.cpp +++ b/clang-tools-extra/clang-tidy/modernize/ModernizeTidyModule.cpp @@ -54,6 +54,7 @@ #include "UseTransparentFunctorsCheck.h" #include "UseUncaughtExceptionsCheck.h" #include "UseUsingCheck.h" +#include "ConditionalToIfCheck.h" using namespace clang::ast_matchers; @@ -134,6 +135,7 @@ class ModernizeModule : public ClangTidyModule { CheckFactories.registerCheck<UseUncaughtExceptionsCheck>( "modernize-use-uncaught-exceptions"); CheckFactories.registerCheck<UseUsingCheck>("modernize-use-using"); + CheckFactories.registerCheck<ConditionalToIfCheck>("modernize-conditional-to-if"); } }; >From 0cc65c5ef48d6c3ca36a8358e95f9a61c62c2c44 Mon Sep 17 00:00:00 2001 From: Dheeraj <[email protected]> Date: Fri, 7 Nov 2025 19:00:52 +0530 Subject: [PATCH 3/6] clang formatted --- .../modernize/ConditionalToIfCheck.cpp | 98 +++++++++---------- .../modernize/ModernizeTidyModule.cpp | 5 +- 2 files changed, 50 insertions(+), 53 deletions(-) diff --git a/clang-tools-extra/clang-tidy/modernize/ConditionalToIfCheck.cpp b/clang-tools-extra/clang-tidy/modernize/ConditionalToIfCheck.cpp index 22e6c47058d88..64de9b1e70af2 100644 --- a/clang-tools-extra/clang-tidy/modernize/ConditionalToIfCheck.cpp +++ b/clang-tools-extra/clang-tidy/modernize/ConditionalToIfCheck.cpp @@ -25,8 +25,8 @@ CharSourceRange tokenRange(const SourceRange &R, std::string getTokenText(const SourceRange &R, const MatchFinder::MatchResult &Res) { - return Lexer::getSourceText(tokenRange(R, Res), - *Res.SourceManager, Res.Context->getLangOpts()) + return Lexer::getSourceText(tokenRange(R, Res), *Res.SourceManager, + Res.Context->getLangOpts()) .str(); } @@ -34,17 +34,19 @@ const Expr *strip(const Expr *E) { return E ? E->IgnoreParenImpCasts() : E; } // Find the statement that directly contains E (best-effort). const Stmt *enclosingStmt(const Expr *E, const MatchFinder::MatchResult &Res) { - if (!E) return nullptr; + if (!E) + return nullptr; const Stmt *Cur = E; while (true) { auto Parents = Res.Context->getParents(*Cur); - if (Parents.empty()) return Cur; + if (Parents.empty()) + return Cur; if (const Stmt *S = Parents[0].get<Stmt>()) { Cur = S; // Stop when Cur itself is a statement that could be replaced wholesale. if (isa<ExprWithCleanups>(Cur) || isa<ReturnStmt>(Cur) || - isa<CompoundStmt>(Cur) || isa<IfStmt>(Cur) || - isa<DeclStmt>(Cur) || isa<Expr>(Cur)) + isa<CompoundStmt>(Cur) || isa<IfStmt>(Cur) || isa<DeclStmt>(Cur) || + isa<Expr>(Cur)) continue; return Cur; } else { @@ -85,8 +87,7 @@ void ConditionalToIfCheck::registerMatchers(MatchFinder *Finder) { // 2) return cond ? a : b; Finder->addMatcher( - returnStmt(InMain, - hasReturnValue(conditionalOperator().bind("retop"))) + returnStmt(InMain, hasReturnValue(conditionalOperator().bind("retop"))) .bind("ret"), this); } else { @@ -95,45 +96,42 @@ void ConditionalToIfCheck::registerMatchers(MatchFinder *Finder) { // 3a) if (cond) x = A; else x = B; // (Match a simple assignment in each branch; allow it to be wrapped in // an ExprWithCleanups/ExprStmt or a single-statement compound.) - auto AssignThen = - binaryOperator(isAssignmentOperator()) - .bind("thenA"); - auto AssignElse = - binaryOperator(isAssignmentOperator()) - .bind("elseA"); + auto AssignThen = binaryOperator(isAssignmentOperator()).bind("thenA"); + auto AssignElse = binaryOperator(isAssignmentOperator()).bind("elseA"); Finder->addMatcher( ifStmt(InMain, - hasThen(anyOf( - hasDescendant(AssignThen), - compoundStmt(statementCountIs(1), - hasAnySubstatement(hasDescendant(AssignThen))))), - hasElse(anyOf( - hasDescendant(AssignElse), - compoundStmt(statementCountIs(1), - hasAnySubstatement(hasDescendant(AssignElse)))))) + hasThen(anyOf(hasDescendant(AssignThen), + compoundStmt(statementCountIs(1), + hasAnySubstatement( + hasDescendant(AssignThen))))), + hasElse(anyOf(hasDescendant(AssignElse), + compoundStmt(statementCountIs(1), + hasAnySubstatement( + hasDescendant(AssignElse)))))) .bind("ifAssign"), this); // 3b) if (cond) return A; else return B; - auto RetThen = returnStmt(hasReturnValue(expr().bind("thenR"))).bind("thenRet"); - auto RetElse = returnStmt(hasReturnValue(expr().bind("elseR"))).bind("elseRet"); + auto RetThen = + returnStmt(hasReturnValue(expr().bind("thenR"))).bind("thenRet"); + auto RetElse = + returnStmt(hasReturnValue(expr().bind("elseR"))).bind("elseRet"); Finder->addMatcher( - ifStmt(InMain, - hasThen(anyOf(RetThen, - compoundStmt(statementCountIs(1), - hasAnySubstatement(RetThen)))), - hasElse(anyOf(RetElse, - compoundStmt(statementCountIs(1), - hasAnySubstatement(RetElse))))) + ifStmt( + InMain, + hasThen(anyOf(RetThen, compoundStmt(statementCountIs(1), + hasAnySubstatement(RetThen)))), + hasElse(anyOf(RetElse, compoundStmt(statementCountIs(1), + hasAnySubstatement(RetElse))))) .bind("ifReturn"), this); } } -bool ConditionalToIfCheck::locationsAreOK( - const SourceRange &R, const MatchFinder::MatchResult &Rst) { +bool ConditionalToIfCheck::locationsAreOK(const SourceRange &R, + const MatchFinder::MatchResult &Rst) { if (R.isInvalid()) return false; if (isInMacro(R, Rst)) @@ -143,8 +141,8 @@ bool ConditionalToIfCheck::locationsAreOK( return true; } -std::string ConditionalToIfCheck::getText( - const SourceRange &R, const MatchFinder::MatchResult &Rst) { +std::string ConditionalToIfCheck::getText(const SourceRange &R, + const MatchFinder::MatchResult &Rst) { return getTokenText(R, Rst); } @@ -159,7 +157,8 @@ bool ConditionalToIfCheck::hasObviousSideEffects(const Expr *E, return true; // Additional heuristics for common side-effect nodes. - if (isa<CallExpr>(E) || isa<CXXConstructExpr>(E) || isa<CXXOperatorCallExpr>(E)) + if (isa<CallExpr>(E) || isa<CXXConstructExpr>(E) || + isa<CXXOperatorCallExpr>(E)) return true; if (const auto *U = dyn_cast<UnaryOperator>(E)) { @@ -181,7 +180,8 @@ void ConditionalToIfCheck::check(const MatchFinder::MatchResult &Res) { // Handle: return cond ? a : b; if (const auto *Ret = Res.Nodes.getNodeAs<ReturnStmt>("ret")) { const auto *CO = Res.Nodes.getNodeAs<ConditionalOperator>("retop"); - if (!CO) return; + if (!CO) + return; const Expr *Cond = strip(CO->getCond()); const Expr *TrueE = strip(CO->getTrueExpr()); @@ -202,8 +202,7 @@ void ConditionalToIfCheck::check(const MatchFinder::MatchResult &Res) { std::string Replacement = "if (" + CondS + ") {\n return " + TS + ";\n} else {\n return " + FS + ";\n}"; - diag(Ret->getBeginLoc(), - "replace simple conditional return with if/else") + diag(Ret->getBeginLoc(), "replace simple conditional return with if/else") << FixItHint::CreateReplacement(CharSourceRange::getCharRange(SR), Replacement); return; @@ -213,7 +212,8 @@ void ConditionalToIfCheck::check(const MatchFinder::MatchResult &Res) { if (const auto *Assign = Res.Nodes.getNodeAs<BinaryOperator>("assign")) { const auto *CO = Res.Nodes.getNodeAs<ConditionalOperator>("condop"); const auto *LHS = Res.Nodes.getNodeAs<Expr>("assignLHS"); - if (!CO || !LHS) return; + if (!CO || !LHS) + return; const Expr *Cond = strip(CO->getCond()); const Expr *TrueE = strip(CO->getTrueExpr()); @@ -221,8 +221,7 @@ void ConditionalToIfCheck::check(const MatchFinder::MatchResult &Res) { if (hasObviousSideEffects(Cond, Ctx) || hasObviousSideEffects(TrueE, Ctx) || - hasObviousSideEffects(FalseE, Ctx) || - hasObviousSideEffects(LHS, Ctx)) + hasObviousSideEffects(FalseE, Ctx) || hasObviousSideEffects(LHS, Ctx)) return; const Stmt *Carrier = enclosingStmt(Assign, Res); @@ -239,8 +238,7 @@ void ConditionalToIfCheck::check(const MatchFinder::MatchResult &Res) { std::string FS = getText(FalseE->getSourceRange(), Res); std::string Replacement = "if (" + CondS + ") {\n " + LHSS + " = " + TS + - ";\n} else {\n " + LHSS + " = " + FS + - ";\n}"; + ";\n} else {\n " + LHSS + " = " + FS + ";\n}"; diag(Carrier->getBeginLoc(), "replace simple conditional assignment with if/else") << FixItHint::CreateReplacement(CharSourceRange::getCharRange(SR), @@ -257,10 +255,10 @@ void ConditionalToIfCheck::check(const MatchFinder::MatchResult &Res) { const Expr *Cond = strip(IfR->getCond()); const Expr *ThenR = strip(Res.Nodes.getNodeAs<Expr>("thenR")); const Expr *ElseR = strip(Res.Nodes.getNodeAs<Expr>("elseR")); - if (!Cond || !ThenR || !ElseR) return; + if (!Cond || !ThenR || !ElseR) + return; - if (hasObviousSideEffects(Cond, Ctx) || - hasObviousSideEffects(ThenR, Ctx) || + if (hasObviousSideEffects(Cond, Ctx) || hasObviousSideEffects(ThenR, Ctx) || hasObviousSideEffects(ElseR, Ctx)) return; @@ -301,10 +299,8 @@ void ConditionalToIfCheck::check(const MatchFinder::MatchResult &Res) { if (LThen != LElse) return; - if (hasObviousSideEffects(Cond, Ctx) || - hasObviousSideEffects(ThenR, Ctx) || - hasObviousSideEffects(ElseR, Ctx) || - hasObviousSideEffects(ThenL, Ctx)) + if (hasObviousSideEffects(Cond, Ctx) || hasObviousSideEffects(ThenR, Ctx) || + hasObviousSideEffects(ElseR, Ctx) || hasObviousSideEffects(ThenL, Ctx)) return; SourceRange SR = IfA->getSourceRange(); diff --git a/clang-tools-extra/clang-tidy/modernize/ModernizeTidyModule.cpp b/clang-tools-extra/clang-tidy/modernize/ModernizeTidyModule.cpp index 527040a4fadd2..3792395e53117 100644 --- a/clang-tools-extra/clang-tidy/modernize/ModernizeTidyModule.cpp +++ b/clang-tools-extra/clang-tidy/modernize/ModernizeTidyModule.cpp @@ -14,6 +14,7 @@ #include "AvoidSetjmpLongjmpCheck.h" #include "AvoidVariadicFunctionsCheck.h" #include "ConcatNestedNamespacesCheck.h" +#include "ConditionalToIfCheck.h" #include "DeprecatedHeadersCheck.h" #include "DeprecatedIosBaseAliasesCheck.h" #include "LoopConvertCheck.h" @@ -54,7 +55,6 @@ #include "UseTransparentFunctorsCheck.h" #include "UseUncaughtExceptionsCheck.h" #include "UseUsingCheck.h" -#include "ConditionalToIfCheck.h" using namespace clang::ast_matchers; @@ -135,7 +135,8 @@ class ModernizeModule : public ClangTidyModule { CheckFactories.registerCheck<UseUncaughtExceptionsCheck>( "modernize-use-uncaught-exceptions"); CheckFactories.registerCheck<UseUsingCheck>("modernize-use-using"); - CheckFactories.registerCheck<ConditionalToIfCheck>("modernize-conditional-to-if"); + CheckFactories.registerCheck<ConditionalToIfCheck>( + "modernize-conditional-to-if"); } }; >From da4e0b6308894bf4d843f8f88b7e5148adf67cee Mon Sep 17 00:00:00 2001 From: Dheeraj <[email protected]> Date: Fri, 7 Nov 2025 23:46:53 +0530 Subject: [PATCH 4/6] followed the suggested changes --- .../clang-tidy/modernize/CMakeLists.txt | 1 - .../modernize/ConditionalToIfCheck.cpp | 324 ------------------ .../modernize/ConditionalToIfCheck.h | 39 --- .../modernize/ModernizeTidyModule.cpp | 3 - .../clang-tidy/readability/CMakeLists.txt | 1 + .../readability/ConditionalToIfCheck.cpp | 85 +++++ .../readability/ConditionalToIfCheck.h | 35 ++ .../readability/ReadabilityTidyModule.cpp | 3 + 8 files changed, 124 insertions(+), 367 deletions(-) delete mode 100644 clang-tools-extra/clang-tidy/modernize/ConditionalToIfCheck.cpp delete mode 100644 clang-tools-extra/clang-tidy/modernize/ConditionalToIfCheck.h create mode 100644 clang-tools-extra/clang-tidy/readability/ConditionalToIfCheck.cpp create mode 100644 clang-tools-extra/clang-tidy/readability/ConditionalToIfCheck.h diff --git a/clang-tools-extra/clang-tidy/modernize/CMakeLists.txt b/clang-tools-extra/clang-tidy/modernize/CMakeLists.txt index 608a81a7f200c..882f2dc9fb4d8 100644 --- a/clang-tools-extra/clang-tidy/modernize/CMakeLists.txt +++ b/clang-tools-extra/clang-tidy/modernize/CMakeLists.txt @@ -53,7 +53,6 @@ add_clang_library(clangTidyModernizeModule STATIC UseTransparentFunctorsCheck.cpp UseUncaughtExceptionsCheck.cpp UseUsingCheck.cpp - ConditionalToIfCheck.cpp LINK_LIBS clangTidy diff --git a/clang-tools-extra/clang-tidy/modernize/ConditionalToIfCheck.cpp b/clang-tools-extra/clang-tidy/modernize/ConditionalToIfCheck.cpp deleted file mode 100644 index 64de9b1e70af2..0000000000000 --- a/clang-tools-extra/clang-tidy/modernize/ConditionalToIfCheck.cpp +++ /dev/null @@ -1,324 +0,0 @@ -#include "ConditionalToIfCheck.h" - -#include "clang/AST/ASTContext.h" -#include "clang/AST/Expr.h" -#include "clang/ASTMatchers/ASTMatchFinder.h" -#include "clang/Basic/SourceManager.h" -#include "clang/Lex/Lexer.h" -#include "llvm/ADT/StringSwitch.h" - -using namespace clang; -using namespace clang::ast_matchers; - -namespace clang::tidy::modernize { - -namespace { - -bool isInMacro(const SourceRange &R, const MatchFinder::MatchResult &Res) { - return R.getBegin().isMacroID() || R.getEnd().isMacroID(); -} - -CharSourceRange tokenRange(const SourceRange &R, - const MatchFinder::MatchResult &Res) { - return CharSourceRange::getTokenRange(R); -} - -std::string getTokenText(const SourceRange &R, - const MatchFinder::MatchResult &Res) { - return Lexer::getSourceText(tokenRange(R, Res), *Res.SourceManager, - Res.Context->getLangOpts()) - .str(); -} - -const Expr *strip(const Expr *E) { return E ? E->IgnoreParenImpCasts() : E; } - -// Find the statement that directly contains E (best-effort). -const Stmt *enclosingStmt(const Expr *E, const MatchFinder::MatchResult &Res) { - if (!E) - return nullptr; - const Stmt *Cur = E; - while (true) { - auto Parents = Res.Context->getParents(*Cur); - if (Parents.empty()) - return Cur; - if (const Stmt *S = Parents[0].get<Stmt>()) { - Cur = S; - // Stop when Cur itself is a statement that could be replaced wholesale. - if (isa<ExprWithCleanups>(Cur) || isa<ReturnStmt>(Cur) || - isa<CompoundStmt>(Cur) || isa<IfStmt>(Cur) || isa<DeclStmt>(Cur) || - isa<Expr>(Cur)) - continue; - return Cur; - } else { - return Cur; - } - } -} - -} // namespace - -ConditionalToIfCheck::ConditionalToIfCheck(StringRef Name, - ClangTidyContext *Ctx) - : ClangTidyCheck(Name, Ctx) { - StringRef V = Options.get("PreferredForm", "if"); - // Use StringSwitch for broad LLVM version compatibility. - PreferredForm = llvm::StringSwitch<Preferred>(V) - .CaseLower("conditional", Preferred::Conditional) - .Default(Preferred::If); -} - -void ConditionalToIfCheck::storeOptions(ClangTidyOptions::OptionMap &Opts) { - Options.store(Opts, "PreferredForm", - PreferredForm == Preferred::If ? "if" : "conditional"); -} - -void ConditionalToIfCheck::registerMatchers(MatchFinder *Finder) { - // Only match code written in the main file to avoid noisy headers. - auto InMain = isExpansionInMainFile(); - - if (PreferredForm == Preferred::If) { - // 1) x = cond ? a : b; - Finder->addMatcher( - binaryOperator(InMain, isAssignmentOperator(), - hasRHS(conditionalOperator().bind("condop")), - hasLHS(expr().bind("assignLHS"))) - .bind("assign"), - this); - - // 2) return cond ? a : b; - Finder->addMatcher( - returnStmt(InMain, hasReturnValue(conditionalOperator().bind("retop"))) - .bind("ret"), - this); - } else { - // Preferred::Conditional - - // 3a) if (cond) x = A; else x = B; - // (Match a simple assignment in each branch; allow it to be wrapped in - // an ExprWithCleanups/ExprStmt or a single-statement compound.) - auto AssignThen = binaryOperator(isAssignmentOperator()).bind("thenA"); - auto AssignElse = binaryOperator(isAssignmentOperator()).bind("elseA"); - - Finder->addMatcher( - ifStmt(InMain, - hasThen(anyOf(hasDescendant(AssignThen), - compoundStmt(statementCountIs(1), - hasAnySubstatement( - hasDescendant(AssignThen))))), - hasElse(anyOf(hasDescendant(AssignElse), - compoundStmt(statementCountIs(1), - hasAnySubstatement( - hasDescendant(AssignElse)))))) - .bind("ifAssign"), - this); - - // 3b) if (cond) return A; else return B; - auto RetThen = - returnStmt(hasReturnValue(expr().bind("thenR"))).bind("thenRet"); - auto RetElse = - returnStmt(hasReturnValue(expr().bind("elseR"))).bind("elseRet"); - - Finder->addMatcher( - ifStmt( - InMain, - hasThen(anyOf(RetThen, compoundStmt(statementCountIs(1), - hasAnySubstatement(RetThen)))), - hasElse(anyOf(RetElse, compoundStmt(statementCountIs(1), - hasAnySubstatement(RetElse))))) - .bind("ifReturn"), - this); - } -} - -bool ConditionalToIfCheck::locationsAreOK(const SourceRange &R, - const MatchFinder::MatchResult &Rst) { - if (R.isInvalid()) - return false; - if (isInMacro(R, Rst)) - return false; - if (!Rst.SourceManager->isWrittenInMainFile(R.getBegin())) - return false; - return true; -} - -std::string ConditionalToIfCheck::getText(const SourceRange &R, - const MatchFinder::MatchResult &Rst) { - return getTokenText(R, Rst); -} - -bool ConditionalToIfCheck::hasObviousSideEffects(const Expr *E, - ASTContext &Ctx) { - if (!E) - return true; - E = strip(E); - - // Very conservative: rely on Clang's side-effect query. - if (E->HasSideEffects(Ctx)) - return true; - - // Additional heuristics for common side-effect nodes. - if (isa<CallExpr>(E) || isa<CXXConstructExpr>(E) || - isa<CXXOperatorCallExpr>(E)) - return true; - - if (const auto *U = dyn_cast<UnaryOperator>(E)) { - if (U->isIncrementDecrementOp() || U->getOpcode() == UO_AddrOf || - U->getOpcode() == UO_Deref) - return true; - } - if (const auto *B = dyn_cast<BinaryOperator>(E)) { - if (B->isAssignmentOp() || B->getOpcode() == BO_Comma) - return true; - } - return false; -} - -void ConditionalToIfCheck::check(const MatchFinder::MatchResult &Res) { - ASTContext &Ctx = *Res.Context; - - if (PreferredForm == Preferred::If) { - // Handle: return cond ? a : b; - if (const auto *Ret = Res.Nodes.getNodeAs<ReturnStmt>("ret")) { - const auto *CO = Res.Nodes.getNodeAs<ConditionalOperator>("retop"); - if (!CO) - return; - - const Expr *Cond = strip(CO->getCond()); - const Expr *TrueE = strip(CO->getTrueExpr()); - const Expr *FalseE = strip(CO->getFalseExpr()); - - if (hasObviousSideEffects(Cond, Ctx) || - hasObviousSideEffects(TrueE, Ctx) || - hasObviousSideEffects(FalseE, Ctx)) - return; - - SourceRange SR = Ret->getSourceRange(); - if (!locationsAreOK(SR, Res)) - return; - - std::string CondS = getText(Cond->getSourceRange(), Res); - std::string TS = getText(TrueE->getSourceRange(), Res); - std::string FS = getText(FalseE->getSourceRange(), Res); - - std::string Replacement = "if (" + CondS + ") {\n return " + TS + - ";\n} else {\n return " + FS + ";\n}"; - diag(Ret->getBeginLoc(), "replace simple conditional return with if/else") - << FixItHint::CreateReplacement(CharSourceRange::getCharRange(SR), - Replacement); - return; - } - - // Handle: x = cond ? a : b; - if (const auto *Assign = Res.Nodes.getNodeAs<BinaryOperator>("assign")) { - const auto *CO = Res.Nodes.getNodeAs<ConditionalOperator>("condop"); - const auto *LHS = Res.Nodes.getNodeAs<Expr>("assignLHS"); - if (!CO || !LHS) - return; - - const Expr *Cond = strip(CO->getCond()); - const Expr *TrueE = strip(CO->getTrueExpr()); - const Expr *FalseE = strip(CO->getFalseExpr()); - - if (hasObviousSideEffects(Cond, Ctx) || - hasObviousSideEffects(TrueE, Ctx) || - hasObviousSideEffects(FalseE, Ctx) || hasObviousSideEffects(LHS, Ctx)) - return; - - const Stmt *Carrier = enclosingStmt(Assign, Res); - if (!Carrier) - Carrier = Assign; - - SourceRange SR = Carrier->getSourceRange(); - if (!locationsAreOK(SR, Res)) - return; - - std::string LHSS = getText(LHS->getSourceRange(), Res); - std::string CondS = getText(Cond->getSourceRange(), Res); - std::string TS = getText(TrueE->getSourceRange(), Res); - std::string FS = getText(FalseE->getSourceRange(), Res); - - std::string Replacement = "if (" + CondS + ") {\n " + LHSS + " = " + TS + - ";\n} else {\n " + LHSS + " = " + FS + ";\n}"; - diag(Carrier->getBeginLoc(), - "replace simple conditional assignment with if/else") - << FixItHint::CreateReplacement(CharSourceRange::getCharRange(SR), - Replacement); - return; - } - return; - } - - // PreferredForm == Conditional - - // if (cond) return A; else return B; - if (const auto *IfR = Res.Nodes.getNodeAs<IfStmt>("ifReturn")) { - const Expr *Cond = strip(IfR->getCond()); - const Expr *ThenR = strip(Res.Nodes.getNodeAs<Expr>("thenR")); - const Expr *ElseR = strip(Res.Nodes.getNodeAs<Expr>("elseR")); - if (!Cond || !ThenR || !ElseR) - return; - - if (hasObviousSideEffects(Cond, Ctx) || hasObviousSideEffects(ThenR, Ctx) || - hasObviousSideEffects(ElseR, Ctx)) - return; - - SourceRange SR = IfR->getSourceRange(); - if (!locationsAreOK(SR, Res)) - return; - - std::string CondS = getText(Cond->getSourceRange(), Res); - std::string TS = getText(ThenR->getSourceRange(), Res); - std::string FS = getText(ElseR->getSourceRange(), Res); - - std::string Replacement = - "return (" + CondS + ") ? " + TS + " : " + FS + ";"; - diag(IfR->getBeginLoc(), "replace simple if/else with conditional return") - << FixItHint::CreateReplacement(CharSourceRange::getCharRange(SR), - Replacement); - return; - } - - // if (cond) x = A; else x = B; - if (const auto *IfA = Res.Nodes.getNodeAs<IfStmt>("ifAssign")) { - const Expr *Cond = strip(IfA->getCond()); - const auto *ThenA = Res.Nodes.getNodeAs<BinaryOperator>("thenA"); - const auto *ElseA = Res.Nodes.getNodeAs<BinaryOperator>("elseA"); - if (!Cond || !ThenA || !ElseA) - return; - - const Expr *ThenL = strip(ThenA->getLHS()); - const Expr *ElseL = strip(ElseA->getLHS()); - const Expr *ThenR = strip(ThenA->getRHS()); - const Expr *ElseR = strip(ElseA->getRHS()); - if (!ThenL || !ElseL || !ThenR || !ElseR) - return; - - // LHS must be textually identical (safe & simple). - std::string LThen = getText(ThenL->getSourceRange(), Res); - std::string LElse = getText(ElseL->getSourceRange(), Res); - if (LThen != LElse) - return; - - if (hasObviousSideEffects(Cond, Ctx) || hasObviousSideEffects(ThenR, Ctx) || - hasObviousSideEffects(ElseR, Ctx) || hasObviousSideEffects(ThenL, Ctx)) - return; - - SourceRange SR = IfA->getSourceRange(); - if (!locationsAreOK(SR, Res)) - return; - - std::string CondS = getText(Cond->getSourceRange(), Res); - std::string TS = getText(ThenR->getSourceRange(), Res); - std::string FS = getText(ElseR->getSourceRange(), Res); - - std::string Replacement = - LThen + " = (" + CondS + ") ? " + TS + " : " + FS + ";"; - diag(IfA->getBeginLoc(), - "replace simple if/else assignment with conditional expression") - << FixItHint::CreateReplacement(CharSourceRange::getCharRange(SR), - Replacement); - return; - } -} - -} // namespace clang::tidy::modernize diff --git a/clang-tools-extra/clang-tidy/modernize/ConditionalToIfCheck.h b/clang-tools-extra/clang-tidy/modernize/ConditionalToIfCheck.h deleted file mode 100644 index 95d159b580d37..0000000000000 --- a/clang-tools-extra/clang-tidy/modernize/ConditionalToIfCheck.h +++ /dev/null @@ -1,39 +0,0 @@ -#ifndef LLVM_CLANG_TOOLS_EXTRA_CLANG_TIDY_MODERNIZE_CONDITIONALTOIFCHECK_H -#define LLVM_CLANG_TOOLS_EXTRA_CLANG_TIDY_MODERNIZE_CONDITIONALTOIFCHECK_H - -#include "../ClangTidyCheck.h" - -namespace clang::tidy::modernize { - -/// Convert between simple conditional (?:) expressions and equivalent if/else -/// statements in safe, syntactically simple cases. -/// -/// Direction is controlled by the option: -/// - modernize-conditional-to-if.PreferredForm: "if" | "conditional" -class ConditionalToIfCheck : public ClangTidyCheck { -public: - ConditionalToIfCheck(llvm::StringRef Name, ClangTidyContext *Context); - - void storeOptions(ClangTidyOptions::OptionMap &Opts) override; - void registerMatchers(ast_matchers::MatchFinder *Finder) override; - void check(const ast_matchers::MatchFinder::MatchResult &Result) override; - -private: - enum class Preferred { If, Conditional }; - Preferred PreferredForm; - - // Conservative side-effect check (needs ASTContext). - static bool hasObviousSideEffects(const Expr *E, ASTContext &Ctx); - - // Fetch exact source text for a range. - static std::string getText(const SourceRange &R, - const ast_matchers::MatchFinder::MatchResult &Rst); - - // Ensure locations are in main file and not in macros. - static bool locationsAreOK(const SourceRange &R, - const ast_matchers::MatchFinder::MatchResult &Rst); -}; - -} // namespace clang::tidy::modernize - -#endif diff --git a/clang-tools-extra/clang-tidy/modernize/ModernizeTidyModule.cpp b/clang-tools-extra/clang-tidy/modernize/ModernizeTidyModule.cpp index 3792395e53117..360e2b8434d0c 100644 --- a/clang-tools-extra/clang-tidy/modernize/ModernizeTidyModule.cpp +++ b/clang-tools-extra/clang-tidy/modernize/ModernizeTidyModule.cpp @@ -14,7 +14,6 @@ #include "AvoidSetjmpLongjmpCheck.h" #include "AvoidVariadicFunctionsCheck.h" #include "ConcatNestedNamespacesCheck.h" -#include "ConditionalToIfCheck.h" #include "DeprecatedHeadersCheck.h" #include "DeprecatedIosBaseAliasesCheck.h" #include "LoopConvertCheck.h" @@ -135,8 +134,6 @@ class ModernizeModule : public ClangTidyModule { CheckFactories.registerCheck<UseUncaughtExceptionsCheck>( "modernize-use-uncaught-exceptions"); CheckFactories.registerCheck<UseUsingCheck>("modernize-use-using"); - CheckFactories.registerCheck<ConditionalToIfCheck>( - "modernize-conditional-to-if"); } }; diff --git a/clang-tools-extra/clang-tidy/readability/CMakeLists.txt b/clang-tools-extra/clang-tidy/readability/CMakeLists.txt index 91e9354d454d2..d52c96d212ba6 100644 --- a/clang-tools-extra/clang-tidy/readability/CMakeLists.txt +++ b/clang-tools-extra/clang-tidy/readability/CMakeLists.txt @@ -62,6 +62,7 @@ add_clang_library(clangTidyReadabilityModule STATIC UseAnyOfAllOfCheck.cpp UseConcisePreprocessorDirectivesCheck.cpp UseStdMinMaxCheck.cpp + ConditionalToIfCheck.cpp LINK_LIBS clangTidy diff --git a/clang-tools-extra/clang-tidy/readability/ConditionalToIfCheck.cpp b/clang-tools-extra/clang-tidy/readability/ConditionalToIfCheck.cpp new file mode 100644 index 0000000000000..191e18e202a27 --- /dev/null +++ b/clang-tools-extra/clang-tidy/readability/ConditionalToIfCheck.cpp @@ -0,0 +1,85 @@ +//===--- ConditionalToIfCheck.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 +// +//===----------------------------------------------------------------------===// +// +// This check detects ternary conditional (?:) expressions and replaces them +// with equivalent if/else statements to improve code readability. +// +// Example: +// +// int x = cond ? 1 : 2; +// +// Becomes: +// +// int x; +// if (cond) +// x = 1; +// else +// x = 2; +// +//===----------------------------------------------------------------------===// + +#include "ConditionalToIfCheck.h" +#include "clang/AST/ASTContext.h" +#include "clang/ASTMatchers/ASTMatchFinder.h" +#include "clang/Lex/Lexer.h" // <-- ADD THIS INCLUDE +#include "clang/Rewrite/Core/Rewriter.h" + +using namespace clang::ast_matchers; + +namespace clang::tidy::readability { + +void ConditionalToIfCheck::registerMatchers(MatchFinder *Finder) { + // Match ternary conditional operators (?:) + Finder->addMatcher( + conditionalOperator( + hasTrueExpression(expr().bind("trueExpr")), + hasFalseExpression(expr().bind("falseExpr")), + hasCondition(expr().bind("condExpr")) + ).bind("ternary"), + this); +} + +void ConditionalToIfCheck::check(const MatchFinder::MatchResult &Result) { + const auto *Ternary = Result.Nodes.getNodeAs<ConditionalOperator>("ternary"); + if (!Ternary) + return; + + const Expr *Cond = Result.Nodes.getNodeAs<Expr>("condExpr"); + const Expr *TrueExpr = Result.Nodes.getNodeAs<Expr>("trueExpr"); + const Expr *FalseExpr = Result.Nodes.getNodeAs<Expr>("falseExpr"); + + if (!Cond || !TrueExpr || !FalseExpr) + return; + + const SourceManager &SM = *Result.SourceManager; + + auto Diag = diag(Ternary->getBeginLoc(), + "replace ternary operator with if/else statement for readability"); + + // Extract source text for condition, true and false expressions + std::string CondStr = Lexer::getSourceText( + CharSourceRange::getTokenRange(Cond->getSourceRange()), SM, + Result.Context->getLangOpts()).str(); + + std::string TrueStr = Lexer::getSourceText( + CharSourceRange::getTokenRange(TrueExpr->getSourceRange()), SM, + Result.Context->getLangOpts()).str(); + + std::string FalseStr = Lexer::getSourceText( + CharSourceRange::getTokenRange(FalseExpr->getSourceRange()), SM, + Result.Context->getLangOpts()).str(); + + // Construct the replacement code + std::string Replacement = + "{ if (" + CondStr + ") " + TrueStr + "; else " + FalseStr + "; }"; + + Diag << FixItHint::CreateReplacement(Ternary->getSourceRange(), Replacement); +} + +} // namespace clang::tidy::readability + diff --git a/clang-tools-extra/clang-tidy/readability/ConditionalToIfCheck.h b/clang-tools-extra/clang-tidy/readability/ConditionalToIfCheck.h new file mode 100644 index 0000000000000..2c35b8d1a2cbb --- /dev/null +++ b/clang-tools-extra/clang-tidy/readability/ConditionalToIfCheck.h @@ -0,0 +1,35 @@ +//===--- ConditionalToIfCheck.h - 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 +// +//===----------------------------------------------------------------------===// +// +// This file defines the ConditionalToIfCheck class, which converts conditional +// (?:) expressions into equivalent if/else statements for improved readability. +// +//===----------------------------------------------------------------------===// + +#ifndef LLVM_CLANG_TOOLS_EXTRA_CLANG_TIDY_READABILITY_CONDITIONALTOIFCHECK_H +#define LLVM_CLANG_TOOLS_EXTRA_CLANG_TIDY_READABILITY_CONDITIONALTOIFCHECK_H + +#include "../ClangTidyCheck.h" + +namespace clang::tidy::readability { + +/// A readability check that replaces ternary operators with equivalent +/// if/else statements. +class ConditionalToIfCheck : public ClangTidyCheck { +public: + ConditionalToIfCheck(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::readability + +#endif // LLVM_CLANG_TOOLS_EXTRA_CLANG_TIDY_READABILITY_CONDITIONALTOIFCHECK_H + diff --git a/clang-tools-extra/clang-tidy/readability/ReadabilityTidyModule.cpp b/clang-tools-extra/clang-tidy/readability/ReadabilityTidyModule.cpp index 569302e6065f2..cfe69c399efc9 100644 --- a/clang-tools-extra/clang-tidy/readability/ReadabilityTidyModule.cpp +++ b/clang-tools-extra/clang-tidy/readability/ReadabilityTidyModule.cpp @@ -65,6 +65,7 @@ #include "UseAnyOfAllOfCheck.h" #include "UseConcisePreprocessorDirectivesCheck.h" #include "UseStdMinMaxCheck.h" +#include "ConditionalToIfCheck.h" namespace clang::tidy { namespace readability { @@ -184,6 +185,8 @@ class ReadabilityModule : public ClangTidyModule { "readability-use-concise-preprocessor-directives"); CheckFactories.registerCheck<UseStdMinMaxCheck>( "readability-use-std-min-max"); + CheckFactories.registerCheck<ConditionalToIfCheck>( + "readability-conditional-to-if"); } }; >From de20ffd288d57981af13ca65a1dee8b1148e1308 Mon Sep 17 00:00:00 2001 From: Dheeraj <[email protected]> Date: Fri, 7 Nov 2025 23:59:24 +0530 Subject: [PATCH 5/6] changes for alphabetical order --- .../clang-tidy/readability/CMakeLists.txt | 2 +- .../readability/ConditionalToIfCheck.cpp | 41 ++++++++++--------- .../readability/ConditionalToIfCheck.h | 4 +- .../readability/ReadabilityTidyModule.cpp | 6 +-- 4 files changed, 28 insertions(+), 25 deletions(-) diff --git a/clang-tools-extra/clang-tidy/readability/CMakeLists.txt b/clang-tools-extra/clang-tidy/readability/CMakeLists.txt index d52c96d212ba6..8b0f291f5afc9 100644 --- a/clang-tools-extra/clang-tidy/readability/CMakeLists.txt +++ b/clang-tools-extra/clang-tidy/readability/CMakeLists.txt @@ -10,6 +10,7 @@ add_clang_library(clangTidyReadabilityModule STATIC AvoidReturnWithVoidValueCheck.cpp AvoidUnconditionalPreprocessorIfCheck.cpp BracesAroundStatementsCheck.cpp + ConditionalToIfCheck.cpp ConstReturnTypeCheck.cpp ContainerContainsCheck.cpp ContainerDataPointerCheck.cpp @@ -62,7 +63,6 @@ add_clang_library(clangTidyReadabilityModule STATIC UseAnyOfAllOfCheck.cpp UseConcisePreprocessorDirectivesCheck.cpp UseStdMinMaxCheck.cpp - ConditionalToIfCheck.cpp LINK_LIBS clangTidy diff --git a/clang-tools-extra/clang-tidy/readability/ConditionalToIfCheck.cpp b/clang-tools-extra/clang-tidy/readability/ConditionalToIfCheck.cpp index 191e18e202a27..d7715c4d25f99 100644 --- a/clang-tools-extra/clang-tidy/readability/ConditionalToIfCheck.cpp +++ b/clang-tools-extra/clang-tidy/readability/ConditionalToIfCheck.cpp @@ -1,4 +1,5 @@ -//===--- ConditionalToIfCheck.cpp - clang-tidy -------------------*- C++ -*-===// +//===--- ConditionalToIfCheck.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. @@ -26,7 +27,7 @@ #include "ConditionalToIfCheck.h" #include "clang/AST/ASTContext.h" #include "clang/ASTMatchers/ASTMatchFinder.h" -#include "clang/Lex/Lexer.h" // <-- ADD THIS INCLUDE +#include "clang/Lex/Lexer.h" // <-- ADD THIS INCLUDE #include "clang/Rewrite/Core/Rewriter.h" using namespace clang::ast_matchers; @@ -36,11 +37,10 @@ namespace clang::tidy::readability { void ConditionalToIfCheck::registerMatchers(MatchFinder *Finder) { // Match ternary conditional operators (?:) Finder->addMatcher( - conditionalOperator( - hasTrueExpression(expr().bind("trueExpr")), - hasFalseExpression(expr().bind("falseExpr")), - hasCondition(expr().bind("condExpr")) - ).bind("ternary"), + conditionalOperator(hasTrueExpression(expr().bind("trueExpr")), + hasFalseExpression(expr().bind("falseExpr")), + hasCondition(expr().bind("condExpr"))) + .bind("ternary"), this); } @@ -58,21 +58,25 @@ void ConditionalToIfCheck::check(const MatchFinder::MatchResult &Result) { const SourceManager &SM = *Result.SourceManager; - auto Diag = diag(Ternary->getBeginLoc(), - "replace ternary operator with if/else statement for readability"); + auto Diag = + diag(Ternary->getBeginLoc(), + "replace ternary operator with if/else statement for readability"); // Extract source text for condition, true and false expressions - std::string CondStr = Lexer::getSourceText( - CharSourceRange::getTokenRange(Cond->getSourceRange()), SM, - Result.Context->getLangOpts()).str(); + std::string CondStr = Lexer::getSourceText(CharSourceRange::getTokenRange( + Cond->getSourceRange()), + SM, Result.Context->getLangOpts()) + .str(); - std::string TrueStr = Lexer::getSourceText( - CharSourceRange::getTokenRange(TrueExpr->getSourceRange()), SM, - Result.Context->getLangOpts()).str(); + std::string TrueStr = Lexer::getSourceText(CharSourceRange::getTokenRange( + TrueExpr->getSourceRange()), + SM, Result.Context->getLangOpts()) + .str(); - std::string FalseStr = Lexer::getSourceText( - CharSourceRange::getTokenRange(FalseExpr->getSourceRange()), SM, - Result.Context->getLangOpts()).str(); + std::string FalseStr = Lexer::getSourceText(CharSourceRange::getTokenRange( + FalseExpr->getSourceRange()), + SM, Result.Context->getLangOpts()) + .str(); // Construct the replacement code std::string Replacement = @@ -82,4 +86,3 @@ void ConditionalToIfCheck::check(const MatchFinder::MatchResult &Result) { } } // namespace clang::tidy::readability - diff --git a/clang-tools-extra/clang-tidy/readability/ConditionalToIfCheck.h b/clang-tools-extra/clang-tidy/readability/ConditionalToIfCheck.h index 2c35b8d1a2cbb..00510e125c437 100644 --- a/clang-tools-extra/clang-tidy/readability/ConditionalToIfCheck.h +++ b/clang-tools-extra/clang-tidy/readability/ConditionalToIfCheck.h @@ -1,4 +1,5 @@ -//===--- ConditionalToIfCheck.h - clang-tidy ---------------------*- C++ -*-===// +//===--- ConditionalToIfCheck.h - 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. @@ -32,4 +33,3 @@ class ConditionalToIfCheck : public ClangTidyCheck { } // namespace clang::tidy::readability #endif // LLVM_CLANG_TOOLS_EXTRA_CLANG_TIDY_READABILITY_CONDITIONALTOIFCHECK_H - diff --git a/clang-tools-extra/clang-tidy/readability/ReadabilityTidyModule.cpp b/clang-tools-extra/clang-tidy/readability/ReadabilityTidyModule.cpp index cfe69c399efc9..04011227588f5 100644 --- a/clang-tools-extra/clang-tidy/readability/ReadabilityTidyModule.cpp +++ b/clang-tools-extra/clang-tidy/readability/ReadabilityTidyModule.cpp @@ -15,6 +15,7 @@ #include "AvoidReturnWithVoidValueCheck.h" #include "AvoidUnconditionalPreprocessorIfCheck.h" #include "BracesAroundStatementsCheck.h" +#include "ConditionalToIfCheck.h" #include "ConstReturnTypeCheck.h" #include "ContainerContainsCheck.h" #include "ContainerDataPointerCheck.h" @@ -65,7 +66,6 @@ #include "UseAnyOfAllOfCheck.h" #include "UseConcisePreprocessorDirectivesCheck.h" #include "UseStdMinMaxCheck.h" -#include "ConditionalToIfCheck.h" namespace clang::tidy { namespace readability { @@ -85,6 +85,8 @@ class ReadabilityModule : public ClangTidyModule { "readability-avoid-unconditional-preprocessor-if"); CheckFactories.registerCheck<BracesAroundStatementsCheck>( "readability-braces-around-statements"); + CheckFactories.registerCheck<ConditionalToIfCheck>( + "readability-conditional-to-if"); CheckFactories.registerCheck<ConstReturnTypeCheck>( "readability-const-return-type"); CheckFactories.registerCheck<ContainerContainsCheck>( @@ -185,8 +187,6 @@ class ReadabilityModule : public ClangTidyModule { "readability-use-concise-preprocessor-directives"); CheckFactories.registerCheck<UseStdMinMaxCheck>( "readability-use-std-min-max"); - CheckFactories.registerCheck<ConditionalToIfCheck>( - "readability-conditional-to-if"); } }; >From 6c18c8566461e8bc3902b766aaea2a913249bec4 Mon Sep 17 00:00:00 2001 From: Dheeraj <[email protected]> Date: Sat, 8 Nov 2025 00:14:13 +0530 Subject: [PATCH 6/6] apply suggested changes --- .../clang-tidy/readability/ConditionalToIfCheck.cpp | 5 ++--- 1 file changed, 2 insertions(+), 3 deletions(-) diff --git a/clang-tools-extra/clang-tidy/readability/ConditionalToIfCheck.cpp b/clang-tools-extra/clang-tidy/readability/ConditionalToIfCheck.cpp index d7715c4d25f99..9fb3ac241d04e 100644 --- a/clang-tools-extra/clang-tidy/readability/ConditionalToIfCheck.cpp +++ b/clang-tools-extra/clang-tidy/readability/ConditionalToIfCheck.cpp @@ -1,5 +1,4 @@ -//===--- ConditionalToIfCheck.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. @@ -63,7 +62,7 @@ void ConditionalToIfCheck::check(const MatchFinder::MatchResult &Result) { "replace ternary operator with if/else statement for readability"); // Extract source text for condition, true and false expressions - std::string CondStr = Lexer::getSourceText(CharSourceRange::getTokenRange( + const std::string CondStr = Lexer::getSourceText(CharSourceRange::getTokenRange( Cond->getSourceRange()), SM, Result.Context->getLangOpts()) .str(); _______________________________________________ cfe-commits mailing list [email protected] https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
