https://github.com/unterumarmung updated https://github.com/llvm/llvm-project/pull/181558
>From 119033991c61a64e61178b75cc99386bfd56d2c6 Mon Sep 17 00:00:00 2001 From: Daniil Dudkin <[email protected]> Date: Sun, 15 Feb 2026 20:33:54 +0300 Subject: [PATCH] [clang-tidy] Add `readability-redundant-nested-if` check Introduce a readability check that merges nested `if`/`if constexpr` chains by combining conditions with `&&`. This resurrects the earlier patch at https://reviews.llvm.org/D130630. The implementation keeps fix-its conservative around macros, preprocessor directives, attributes, user-defined bool conversions, and comment placement in removable nested headers. It also supports C++17 declaration conditions by rewriting them into init-statement form when safe. --- .../clang-tidy/readability/CMakeLists.txt | 1 + .../readability/ReadabilityTidyModule.cpp | 3 + .../readability/RedundantNestedIfCheck.cpp | 784 ++++++++++++++++++ .../readability/RedundantNestedIfCheck.h | 39 + clang-tools-extra/docs/ReleaseNotes.rst | 6 + .../docs/clang-tidy/checks/list.rst | 1 + .../readability/redundant-nested-if.rst | 118 +++ .../readability/redundant-nested-if.cpp | 474 +++++++++++ 8 files changed, 1426 insertions(+) create mode 100644 clang-tools-extra/clang-tidy/readability/RedundantNestedIfCheck.cpp create mode 100644 clang-tools-extra/clang-tidy/readability/RedundantNestedIfCheck.h create mode 100644 clang-tools-extra/docs/clang-tidy/checks/readability/redundant-nested-if.rst create mode 100644 clang-tools-extra/test/clang-tidy/checkers/readability/redundant-nested-if.cpp diff --git a/clang-tools-extra/clang-tidy/readability/CMakeLists.txt b/clang-tools-extra/clang-tidy/readability/CMakeLists.txt index f1f3cde32feff..2aa422320f81c 100644 --- a/clang-tools-extra/clang-tidy/readability/CMakeLists.txt +++ b/clang-tools-extra/clang-tidy/readability/CMakeLists.txt @@ -45,6 +45,7 @@ add_clang_library(clangTidyReadabilityModule STATIC RedundantDeclarationCheck.cpp RedundantFunctionPtrDereferenceCheck.cpp RedundantMemberInitCheck.cpp + RedundantNestedIfCheck.cpp RedundantParenthesesCheck.cpp RedundantPreprocessorCheck.cpp RedundantSmartptrGetCheck.cpp diff --git a/clang-tools-extra/clang-tidy/readability/ReadabilityTidyModule.cpp b/clang-tools-extra/clang-tidy/readability/ReadabilityTidyModule.cpp index c582dc98eac6b..5cd1a923b23d2 100644 --- a/clang-tools-extra/clang-tidy/readability/ReadabilityTidyModule.cpp +++ b/clang-tools-extra/clang-tidy/readability/ReadabilityTidyModule.cpp @@ -47,6 +47,7 @@ #include "RedundantFunctionPtrDereferenceCheck.h" #include "RedundantInlineSpecifierCheck.h" #include "RedundantMemberInitCheck.h" +#include "RedundantNestedIfCheck.h" #include "RedundantParenthesesCheck.h" #include "RedundantPreprocessorCheck.h" #include "RedundantSmartptrGetCheck.h" @@ -174,6 +175,8 @@ class ReadabilityModule : public ClangTidyModule { "readability-redundant-string-cstr"); CheckFactories.registerCheck<RedundantStringInitCheck>( "readability-redundant-string-init"); + CheckFactories.registerCheck<RedundantNestedIfCheck>( + "readability-redundant-nested-if"); CheckFactories.registerCheck<SimplifyBooleanExprCheck>( "readability-simplify-boolean-expr"); CheckFactories.registerCheck<SuspiciousCallArgumentCheck>( diff --git a/clang-tools-extra/clang-tidy/readability/RedundantNestedIfCheck.cpp b/clang-tools-extra/clang-tidy/readability/RedundantNestedIfCheck.cpp new file mode 100644 index 0000000000000..2fc9c5fdc5ff3 --- /dev/null +++ b/clang-tools-extra/clang-tidy/readability/RedundantNestedIfCheck.cpp @@ -0,0 +1,784 @@ +//===--- RedundantNestedIfCheck.cpp - clang-tidy -------------------------===// +// +// 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 "RedundantNestedIfCheck.h" +#include "../utils/LexerUtils.h" +#include "clang/AST/ASTContext.h" +#include "clang/AST/Stmt.h" +#include "clang/ASTMatchers/ASTMatchFinder.h" +#include "clang/Lex/Lexer.h" +#include "clang/Tooling/FixIt.h" +#include "llvm/ADT/ArrayRef.h" +#include "llvm/ADT/STLExtras.h" +#include "llvm/ADT/SmallVector.h" +#include <optional> +#include <string> +#include <vector> + +using namespace clang::ast_matchers; + +namespace clang::tidy { +template <> +struct OptionEnumMapping< + readability::RedundantNestedIfCheck::UserDefinedBoolConversionMode> { + static llvm::ArrayRef<std::pair< + readability::RedundantNestedIfCheck::UserDefinedBoolConversionMode, + StringRef>> + getEnumMapping() { + using Mode = + readability::RedundantNestedIfCheck::UserDefinedBoolConversionMode; + static constexpr std::pair<Mode, StringRef> Mapping[] = { + {Mode::None, "None"}, + {Mode::WarnOnly, "WarnOnly"}, + {Mode::WarnAndFix, "WarnAndFix"}, + }; + return {Mapping}; + } +}; +} // namespace clang::tidy + +namespace clang::tidy::readability { + +static constexpr llvm::StringLiteral WarnOnDependentConstexprIfStr = + "WarnOnDependentConstexprIf"; +static constexpr llvm::StringLiteral UserDefinedBoolConversionModeStr = + "UserDefinedBoolConversionMode"; + +namespace { +enum class ChainHandling { + None, + WarnOnly, + WarnOnlyDependentConstexpr, + WarnAndFix, +}; + +enum class CombinedConditionBuildStatus { + Success, + UnsupportedCommentPlacement, + Failure, +}; + +struct CombinedConditionBuildResult { + CombinedConditionBuildStatus Status = CombinedConditionBuildStatus::Failure; + std::string Text; +}; +} // namespace + +// Conjoining conditions with `&&` can change behavior when a condition relies +// on user-defined bool conversion. Keep the check conservative and reject such +// conditions for automatic merging. +static bool containsUserDefinedBoolConversion(const Expr *ExprNode) { + if (!ExprNode) + return false; + + if (const auto *Cast = dyn_cast<ImplicitCastExpr>(ExprNode); + Cast && Cast->getCastKind() == CK_UserDefinedConversion) + return true; + + return llvm::any_of(ExprNode->children(), [](const Stmt *Child) { + const auto *ChildExpr = dyn_cast_or_null<Expr>(Child); + return ChildExpr && containsUserDefinedBoolConversion(ChildExpr); + }); +} + +static bool isConditionExpressionSafeToConjoin( + const Expr *Cond, RedundantNestedIfCheck::UserDefinedBoolConversionMode + UserBoolConversionMode) { + if (!Cond || Cond->isTypeDependent()) + return false; + const bool HasUserDefinedBoolConversion = + containsUserDefinedBoolConversion(Cond); + if (UserBoolConversionMode != + RedundantNestedIfCheck::UserDefinedBoolConversionMode::WarnAndFix && + HasUserDefinedBoolConversion) { + return false; + } + const Expr *Unwrapped = Cond->IgnoreParenImpCasts(); + if (!Unwrapped) + return false; + const QualType CondType = Unwrapped->getType(); + if (CondType.isNull()) + return false; + if (CondType->isScalarType()) + return true; + return UserBoolConversionMode == + RedundantNestedIfCheck::UserDefinedBoolConversionMode::WarnAndFix; +} + +static std::optional<CharSourceRange> +getConditionPayloadRange(const IfStmt *If, const SourceManager &SM, + const LangOptions &LangOpts) { + if (!If) + return std::nullopt; + const SourceLocation PayloadBegin = + Lexer::getLocForEndOfToken(If->getLParenLoc(), 0, SM, LangOpts); + if (PayloadBegin.isInvalid() || If->getRParenLoc().isInvalid()) + return std::nullopt; + + const CharSourceRange PayloadRange = + CharSourceRange::getCharRange(PayloadBegin, If->getRParenLoc()); + const CharSourceRange FileRange = + Lexer::makeFileCharRange(PayloadRange, SM, LangOpts); + if (FileRange.isInvalid()) + return std::nullopt; + return FileRange; +} + +static std::optional<std::string> +getConditionPayloadText(const IfStmt *If, const SourceManager &SM, + const LangOptions &LangOpts) { + const std::optional<CharSourceRange> PayloadRange = + getConditionPayloadRange(If, SM, LangOpts); + if (!PayloadRange) + return std::nullopt; + + bool Invalid = false; + const StringRef PayloadText = + Lexer::getSourceText(*PayloadRange, SM, LangOpts, &Invalid); + if (Invalid || PayloadText.empty()) + return std::nullopt; + return PayloadText.str(); +} + +static std::vector<utils::lexer::CommentToken> +getCommentTokensInRange(CharSourceRange Range, const SourceManager &SM, + const LangOptions &LangOpts) { + std::vector<utils::lexer::CommentToken> Comments; + if (Range.isInvalid()) + return Comments; + + const CharSourceRange FileRange = + Lexer::makeFileCharRange(Range, SM, LangOpts); + if (FileRange.isInvalid()) + return Comments; + + const std::pair<FileID, unsigned> BeginLoc = + SM.getDecomposedLoc(FileRange.getBegin()); + const std::pair<FileID, unsigned> EndLoc = + SM.getDecomposedLoc(FileRange.getEnd()); + if (BeginLoc.first != EndLoc.first) + return Comments; + + bool Invalid = false; + const StringRef Buffer = SM.getBufferData(BeginLoc.first, &Invalid); + if (Invalid) + return Comments; + + const char *LexStart = Buffer.data() + BeginLoc.second; + Lexer TheLexer(SM.getLocForStartOfFile(BeginLoc.first), LangOpts, + Buffer.begin(), LexStart, Buffer.end()); + TheLexer.SetCommentRetentionState(true); + + while (true) { + Token Tok; + if (TheLexer.LexFromRawLexer(Tok)) + break; + if (Tok.is(tok::eof) || Tok.getLocation() == FileRange.getEnd() || + SM.isBeforeInTranslationUnit(FileRange.getEnd(), Tok.getLocation())) { + break; + } + + if (!Tok.is(tok::comment)) + continue; + + const std::pair<FileID, unsigned> CommentLoc = + SM.getDecomposedLoc(Tok.getLocation()); + if (CommentLoc.first != BeginLoc.first) + continue; + + Comments.push_back(utils::lexer::CommentToken{ + Tok.getLocation(), + StringRef(Buffer.begin() + CommentLoc.second, Tok.getLength()), + }); + } + + return Comments; +} + +static bool locationInCharRange(SourceLocation Loc, CharSourceRange Range, + const SourceManager &SM) { + if (Loc.isInvalid() || Range.isInvalid()) + return false; + return !SM.isBeforeInTranslationUnit(Loc, Range.getBegin()) && + SM.isBeforeInTranslationUnit(Loc, Range.getEnd()); +} + +// Validate comments in the nested-if header we remove. Comments are fix-safe +// only if they are all inside the condition payload, which is preserved +// verbatim. Any other nested-header comment placement keeps the diagnostic but +// suppresses fix-its. +static bool hasOnlyPayloadCommentsInNestedHeader(const IfStmt *Nested, + const SourceManager &SM, + const LangOptions &LangOpts) { + if (!Nested || !Nested->getThen()) + return false; + + const CharSourceRange HeaderRange = CharSourceRange::getCharRange( + Nested->getBeginLoc(), Nested->getThen()->getBeginLoc()); + const CharSourceRange HeaderFileRange = + Lexer::makeFileCharRange(HeaderRange, SM, LangOpts); + if (HeaderFileRange.isInvalid()) + return false; + + const std::optional<CharSourceRange> PayloadRange = + getConditionPayloadRange(Nested, SM, LangOpts); + if (!PayloadRange) + return false; + + const std::vector<utils::lexer::CommentToken> Comments = + getCommentTokensInRange(HeaderFileRange, SM, LangOpts); + return llvm::all_of(Comments, [&](const utils::lexer::CommentToken &Comment) { + return locationInCharRange(Comment.Loc, *PayloadRange, SM); + }); +} + +// Only an outer condition variable can be rewritten safely by moving it into +// an init-statement and using the declared variable as the first conjunct. +static bool canRewriteOuterConditionVariable( + const IfStmt *If, const LangOptions &LangOpts, + RedundantNestedIfCheck::UserDefinedBoolConversionMode + UserBoolConversionMode) { + if (!If || !If->hasVarStorage() || If->hasInitStorage()) + return false; + // `if (init; cond)` syntax is available in C++17 and later only. + if (!LangOpts.CPlusPlus17) + return false; + const auto *CondVar = If->getConditionVariable(); + const auto *CondVarDeclStmt = If->getConditionVariableDeclStmt(); + if (!CondVar || !CondVarDeclStmt || !CondVarDeclStmt->isSingleDecl() || + CondVar->getName().empty()) { + return false; + } + const QualType VarType = CondVar->getType(); + if (VarType.isNull()) + return false; + if (UserBoolConversionMode != + RedundantNestedIfCheck::UserDefinedBoolConversionMode::WarnAndFix && + !VarType->isScalarType()) { + return false; + } + return isConditionExpressionSafeToConjoin(If->getCond(), + UserBoolConversionMode); +} + +// Accept either `if (...) if (...)` or `if (...) { if (...) }` where the +// compound contains exactly one statement. +static const IfStmt *getOnlyNestedIf(const Stmt *Then) { + if (!Then) + return nullptr; + if (const auto *NestedIf = dyn_cast<IfStmt>(Then)) + return NestedIf; + const auto *Compound = dyn_cast<CompoundStmt>(Then); + if (!Compound || Compound->size() != 1) + return nullptr; + return dyn_cast<IfStmt>(Compound->body_front()); +} + +static bool +isMergeCandidate(const IfStmt *If, bool AllowInitStorage, bool RequireConstexpr, + bool AllowConditionVariable, const LangOptions &LangOpts, + RedundantNestedIfCheck::UserDefinedBoolConversionMode + UserBoolConversionMode) { + if (!If || !If->getThen()) + return false; + if (If->isConsteval() || If->getElse()) + return false; + if (!AllowInitStorage && If->hasInitStorage()) + return false; + if (If->isConstexpr() != RequireConstexpr) + return false; + if (If->hasVarStorage()) + return AllowConditionVariable && canRewriteOuterConditionVariable( + If, LangOpts, UserBoolConversionMode); + + return If->getCond() && isConditionExpressionSafeToConjoin( + If->getCond(), UserBoolConversionMode); +} + +static bool isMergeShapeCandidate(const IfStmt *If, bool AllowInitStorage, + bool RequireConstexpr, + bool AllowConditionVariable, + const LangOptions &LangOpts) { + if (!If || !If->getThen()) + return false; + if (If->isConsteval() || If->getElse()) + return false; + if (!AllowInitStorage && If->hasInitStorage()) + return false; + if (If->isConstexpr() != RequireConstexpr) + return false; + if (If->hasVarStorage()) + return AllowConditionVariable && LangOpts.CPlusPlus17; + return If->getCond() != nullptr; +} + +// Statement attributes are attached outside of the `if` token range; removing +// nested `if` tokens can make attribute placement invalid, so skip them. +static bool isAttributedIf(const IfStmt *If, ASTContext &Context) { + if (!If) + return false; + const DynTypedNodeList Parents = Context.getParents(*If); + return !Parents.empty() && Parents[0].get<AttributedStmt>() != nullptr; +} + +// Build the maximal top-down chain of mergeable nested if statements. +static llvm::SmallVector<const IfStmt *> +getMergeChain(const IfStmt *Root, ASTContext &Context, + RedundantNestedIfCheck::UserDefinedBoolConversionMode + UserBoolConversionMode) { + llvm::SmallVector<const IfStmt *> Chain; + if (!Root) + return Chain; + + const LangOptions &LangOpts = Context.getLangOpts(); + const bool IsConstexpr = Root->isConstexpr(); + if (!isMergeCandidate(Root, /*AllowInitStorage=*/true, IsConstexpr, + /*AllowConditionVariable=*/true, LangOpts, + UserBoolConversionMode) || + isAttributedIf(Root, Context)) { + return Chain; + } + + Chain.push_back(Root); + const IfStmt *Current = Root; + while (const IfStmt *Nested = getOnlyNestedIf(Current->getThen())) { + if (!isMergeCandidate(Nested, /*AllowInitStorage=*/false, IsConstexpr, + /*AllowConditionVariable=*/false, LangOpts, + UserBoolConversionMode) || + isAttributedIf(Nested, Context)) { + break; + } + Chain.push_back(Nested); + Current = Nested; + } + return Chain; +} + +// Warn-only mode for chains blocked specifically by user-defined bool +// conversion in the outer condition. +static llvm::SmallVector<const IfStmt *> +getUserDefinedBoolWarnChain(const IfStmt *Root, ASTContext &Context) { + llvm::SmallVector<const IfStmt *> Chain; + if (!Root) + return Chain; + + const LangOptions &LangOpts = Context.getLangOpts(); + const bool IsConstexpr = Root->isConstexpr(); + if (!isMergeShapeCandidate(Root, /*AllowInitStorage=*/true, IsConstexpr, + /*AllowConditionVariable=*/true, LangOpts) || + isAttributedIf(Root, Context) || + !containsUserDefinedBoolConversion(Root->getCond())) { + return Chain; + } + + Chain.push_back(Root); + const IfStmt *Current = Root; + while (const IfStmt *Nested = getOnlyNestedIf(Current->getThen())) { + if (!isMergeCandidate( + Nested, /*AllowInitStorage=*/false, IsConstexpr, + /*AllowConditionVariable=*/false, LangOpts, + RedundantNestedIfCheck::UserDefinedBoolConversionMode::None) || + isAttributedIf(Nested, Context)) { + break; + } + Chain.push_back(Nested); + Current = Nested; + } + + if (Chain.size() < 2) + Chain.clear(); + return Chain; +} + +// Locate the parent `if` that owns this node in its then-branch. This lets us +// suppress duplicate diagnostics when the parent chain is already handled. +static const IfStmt *getParentThenIf(const IfStmt *If, ASTContext &Context) { + if (!If) + return nullptr; + + const DynTypedNodeList Parents = Context.getParents(*If); + if (Parents.empty()) + return nullptr; + + if (const auto *ParentIf = Parents[0].get<IfStmt>()) { + if (ParentIf->getThen() == If) + return ParentIf; + return nullptr; + } + + const auto *ParentCompound = Parents[0].get<CompoundStmt>(); + if (!ParentCompound || ParentCompound->size() != 1 || + ParentCompound->body_front() != If) { + return nullptr; + } + + const DynTypedNodeList GrandParents = Context.getParents(*ParentCompound); + if (GrandParents.empty()) + return nullptr; + + const auto *ParentIf = GrandParents[0].get<IfStmt>(); + if (!ParentIf || ParentIf->getThen() != ParentCompound) + return nullptr; + return ParentIf; +} + +static bool isConstantBooleanCondition(const Expr *Cond, const ASTContext &Ctx, + bool RequiredValue) { + if (!Cond || Cond->isValueDependent() || Cond->isInstantiationDependent()) + return false; + + bool Evaluated = false; + if (!Cond->EvaluateAsBooleanCondition(Evaluated, Ctx)) + return false; + return Evaluated == RequiredValue; +} + +static bool +isConstexprChainSemanticallySafe(llvm::ArrayRef<const IfStmt *> Chain, + const ASTContext &Context) { + if (Chain.empty() || !Chain.front()->isConstexpr()) + return true; + + const bool OuterIsDependent = + Chain.front()->getCond()->isInstantiationDependent(); + + // Allow outer instantiation-dependence only when every nested condition is a + // non-dependent constant true expression. This preserves constexpr discard + // behavior for template branches. + for (std::size_t Index = 1; Index < Chain.size(); ++Index) { + const Expr *NestedCond = Chain[Index]->getCond(); + if (NestedCond->isInstantiationDependent()) + return false; + if (OuterIsDependent && + !isConstantBooleanCondition(NestedCond, Context, + /*RequiredValue=*/true)) { + return false; + } + } + return true; +} + +// A range is unsafe for text edits if it crosses macro expansions or +// preprocessor directives. +static bool isUnsafeTokenRange(SourceRange Range, const SourceManager &SM, + const LangOptions &LangOpts) { + if (!Range.isValid()) + return true; + if (Range.getBegin().isMacroID() || Range.getEnd().isMacroID()) + return true; + if (Lexer::makeFileCharRange(CharSourceRange::getTokenRange(Range), SM, + LangOpts) + .isInvalid()) { + return true; + } + return utils::lexer::rangeContainsExpansionsOrDirectives(Range, SM, LangOpts); +} + +static bool isUnsafeCharRange(CharSourceRange Range, const SourceManager &SM, + const LangOptions &LangOpts) { + if (Range.isInvalid()) + return true; + if (Range.getBegin().isMacroID() || Range.getEnd().isMacroID()) + return true; + if (Lexer::makeFileCharRange(Range, SM, LangOpts).isInvalid()) + return true; + return utils::lexer::rangeContainsExpansionsOrDirectives(Range.getAsRange(), + SM, LangOpts); +} + +// Validate every range that contributes to the final edit set before offering +// fix-its. If any range is unsafe, keep diagnostics but do not rewrite. +static bool isFixitSafeForChain(llvm::ArrayRef<const IfStmt *> Chain, + const SourceManager &SM, + const LangOptions &LangOpts) { + if (Chain.empty()) + return false; + + const IfStmt *const Root = Chain.front(); + if (Root->hasInitStorage() && !Root->hasVarStorage()) { + if (isUnsafeTokenRange(Chain.front()->getCond()->getSourceRange(), SM, + LangOpts)) { + return false; + } + } else { + const std::optional<CharSourceRange> RootConditionRange = + getConditionPayloadRange(Root, SM, LangOpts); + if (!RootConditionRange || + isUnsafeCharRange(*RootConditionRange, SM, LangOpts)) { + return false; + } + } + + if (Root->hasVarStorage()) { + const auto *CondVarDeclStmt = Root->getConditionVariableDeclStmt(); + if (!CondVarDeclStmt || + isUnsafeTokenRange(CondVarDeclStmt->getSourceRange(), SM, LangOpts)) { + return false; + } + } else if (!Root->hasInitStorage() && + isUnsafeTokenRange(Root->getCond()->getSourceRange(), SM, + LangOpts)) { + return false; + } + + for (std::size_t Index = 1; Index < Chain.size(); ++Index) { + const IfStmt *const Nested = Chain[Index]; + if (isUnsafeTokenRange(Nested->getCond()->getSourceRange(), SM, LangOpts)) + return false; + + const CharSourceRange NestedHeaderRange = CharSourceRange::getCharRange( + Nested->getBeginLoc(), Nested->getThen()->getBeginLoc()); + if (isUnsafeCharRange(NestedHeaderRange, SM, LangOpts)) + return false; + + const auto *Wrapper = dyn_cast<CompoundStmt>(Chain[Index - 1]->getThen()); + if (!Wrapper) + continue; + + if (isUnsafeTokenRange(Wrapper->getSourceRange(), SM, LangOpts) || + isUnsafeTokenRange( + SourceRange(Wrapper->getLBracLoc(), Wrapper->getLBracLoc()), SM, + LangOpts) || + isUnsafeTokenRange( + SourceRange(Wrapper->getRBracLoc(), Wrapper->getRBracLoc()), SM, + LangOpts)) { + return false; + } + } + + return true; +} + +static CombinedConditionBuildResult +buildCombinedCondition(llvm::ArrayRef<const IfStmt *> Chain, + const ASTContext &Context) { + CombinedConditionBuildResult Result; + if (Chain.empty()) + return Result; + + const SourceManager &SM = Context.getSourceManager(); + const LangOptions &LangOpts = Context.getLangOpts(); + std::string Combined; + + for (std::size_t Index = 0; Index < Chain.size(); ++Index) { + const IfStmt *If = Chain[Index]; + if (Index == 0 && If->hasVarStorage()) { + const auto *CondVar = If->getConditionVariable(); + if (!CondVar) + return Result; + + // Preserve comments and spelling in declaration conditions by using the + // full payload text between parentheses instead of the AST declaration + // source range. + const std::optional<std::string> CondPayloadText = + getConditionPayloadText(If, SM, LangOpts); + if (!CondPayloadText) + return Result; + Combined += *CondPayloadText; + Combined += "; "; + const llvm::StringRef CondVarName = CondVar->getName(); + Combined.append(CondVarName.begin(), CondVarName.end()); + continue; + } + + std::optional<std::string> CondText; + if (Index == 0 && If->hasInitStorage()) { + const llvm::StringRef CondExprText = + tooling::fixit::getText(*If->getCond(), Context); + if (CondExprText.empty()) + return Result; + CondText = CondExprText.str(); + } else { + CondText = getConditionPayloadText(If, SM, LangOpts); + if (!CondText) + return Result; + } + + // Nested headers are removed by the fix-it. Keep only comments that are + // semantically local to the removed header and can be preserved safely. + if (Index > 0 && !hasOnlyPayloadCommentsInNestedHeader(If, SM, LangOpts)) { + Result.Status = CombinedConditionBuildStatus::UnsupportedCommentPlacement; + return Result; + } + + if (!Combined.empty()) + Combined += " && "; + // Parenthesize each condition to preserve precedence in the rewritten + // condition regardless of original operator binding. + Combined += "("; + Combined += *CondText; + Combined += ")"; + } + Result.Status = CombinedConditionBuildStatus::Success; + Result.Text = std::move(Combined); + return Result; +} + +static std::optional<CharSourceRange> +getConditionReplacementRange(const IfStmt *If, const SourceManager &SM, + const LangOptions &LangOpts) { + if (!If) + return std::nullopt; + if (If->hasInitStorage() && !If->hasVarStorage()) + return CharSourceRange::getTokenRange(If->getCond()->getSourceRange()); + return getConditionPayloadRange(If, SM, LangOpts); +} + +static ChainHandling +getChainHandling(llvm::ArrayRef<const IfStmt *> Chain, + const ASTContext &Context, const SourceManager &SM, + const LangOptions &LangOpts, bool WarnOnDependentConstexprIf, + std::optional<std::string> *CombinedCondition) { + // One place for all gating logic: chain shape, edit safety, constexpr + // semantic safety, and final condition synthesis. + if (Chain.size() < 2) + return ChainHandling::None; + if (!isFixitSafeForChain(Chain, SM, LangOpts)) + return ChainHandling::None; + if (!isConstexprChainSemanticallySafe(Chain, Context)) { + return WarnOnDependentConstexprIf + ? ChainHandling::WarnOnlyDependentConstexpr + : ChainHandling::None; + } + + const CombinedConditionBuildResult Combined = + buildCombinedCondition(Chain, Context); + if (Combined.Status == + CombinedConditionBuildStatus::UnsupportedCommentPlacement) + return ChainHandling::WarnOnly; + if (Combined.Status != CombinedConditionBuildStatus::Success) + return ChainHandling::None; + if (CombinedCondition) + *CombinedCondition = Combined.Text; + return ChainHandling::WarnAndFix; +} + +static bool +parentWillHandleThisIf(const IfStmt *If, ASTContext &Context, + const SourceManager &SM, const LangOptions &LangOpts, + bool WarnOnDependentConstexprIf, + RedundantNestedIfCheck::UserDefinedBoolConversionMode + UserBoolConversionMode) { + const IfStmt *const ParentIf = getParentThenIf(If, Context); + if (!ParentIf) + return false; + + const auto ParentChain = + getMergeChain(ParentIf, Context, UserBoolConversionMode); + if (ParentChain.size() < 2 || ParentChain[1] != If) + return false; + + return getChainHandling(ParentChain, Context, SM, LangOpts, + WarnOnDependentConstexprIf, + /*CombinedCondition=*/nullptr) != ChainHandling::None; +} + +static bool +parentWillHandleUserDefinedBoolWarning(const IfStmt *If, ASTContext &Context, + const SourceManager &SM, + const LangOptions &LangOpts) { + const IfStmt *const ParentIf = getParentThenIf(If, Context); + if (!ParentIf) + return false; + + const auto ParentChain = getUserDefinedBoolWarnChain(ParentIf, Context); + if (ParentChain.size() < 2 || ParentChain[1] != If) + return false; + return isFixitSafeForChain(ParentChain, SM, LangOpts); +} + +RedundantNestedIfCheck::RedundantNestedIfCheck(StringRef Name, + ClangTidyContext *Context) + : ClangTidyCheck(Name, Context), WarnOnDependentConstexprIf(Options.get( + WarnOnDependentConstexprIfStr, false)), + UserBoolConversionMode(Options.get(UserDefinedBoolConversionModeStr, + UserDefinedBoolConversionMode::None)) { +} + +void RedundantNestedIfCheck::storeOptions(ClangTidyOptions::OptionMap &Opts) { + Options.store(Opts, WarnOnDependentConstexprIfStr, + WarnOnDependentConstexprIf); + Options.store(Opts, UserDefinedBoolConversionModeStr, UserBoolConversionMode); +} + +void RedundantNestedIfCheck::registerMatchers(MatchFinder *Finder) { + Finder->addMatcher(ifStmt().bind("if"), this); +} + +void RedundantNestedIfCheck::check(const MatchFinder::MatchResult &Result) { + const auto *If = Result.Nodes.getNodeAs<IfStmt>("if"); + if (!If) + return; + + ASTContext &Context = *Result.Context; + const SourceManager &SM = *Result.SourceManager; + const LangOptions &LangOpts = Context.getLangOpts(); + + const auto Chain = getMergeChain(If, Context, UserBoolConversionMode); + if (Chain.size() < 2) { + if (UserBoolConversionMode != UserDefinedBoolConversionMode::WarnOnly) + return; + + if (parentWillHandleUserDefinedBoolWarning(If, Context, SM, LangOpts)) + return; + + const auto WarnChain = getUserDefinedBoolWarnChain(If, Context); + if (WarnChain.size() < 2 || !isFixitSafeForChain(WarnChain, SM, LangOpts)) + return; + + diag(If->getIfLoc(), "nested if statements can be merged"); + return; + } + if (parentWillHandleThisIf(If, Context, SM, LangOpts, + WarnOnDependentConstexprIf, + UserBoolConversionMode)) { + return; + } + + std::optional<std::string> CombinedCondition; + const ChainHandling Handling = + getChainHandling(Chain, Context, SM, LangOpts, WarnOnDependentConstexprIf, + &CombinedCondition); + if (Handling == ChainHandling::None) + return; + + if (Handling == ChainHandling::WarnOnlyDependentConstexpr) { + // Keep this as diagnostic-only: the option explicitly asks for awareness + // in templates even when rewrite safety cannot be guaranteed. + diag(If->getIfLoc(), + "nested instantiation-dependent if constexpr statements can be " + "merged"); + return; + } + + if (Handling == ChainHandling::WarnOnly) { + diag(If->getIfLoc(), "nested if statements can be merged"); + return; + } + + auto Diag = diag(If->getIfLoc(), "nested if statements can be merged"); + const std::optional<CharSourceRange> CondRange = + getConditionReplacementRange(If, SM, LangOpts); + if (!CondRange || !CombinedCondition) + return; + Diag << FixItHint::CreateReplacement(*CondRange, *CombinedCondition); + + for (std::size_t Index = 1; Index < Chain.size(); ++Index) { + const IfStmt *const Nested = Chain[Index]; + Diag << FixItHint::CreateRemoval(CharSourceRange::getCharRange( + Nested->getBeginLoc(), Nested->getThen()->getBeginLoc())); + + if (const auto *Wrapper = + dyn_cast<CompoundStmt>(Chain[Index - 1]->getThen())) { + Diag << FixItHint::CreateRemoval(Wrapper->getLBracLoc()) + << FixItHint::CreateRemoval(Wrapper->getRBracLoc()); + } + } +} + +} // namespace clang::tidy::readability diff --git a/clang-tools-extra/clang-tidy/readability/RedundantNestedIfCheck.h b/clang-tools-extra/clang-tidy/readability/RedundantNestedIfCheck.h new file mode 100644 index 0000000000000..33ead69ebc2b4 --- /dev/null +++ b/clang-tools-extra/clang-tidy/readability/RedundantNestedIfCheck.h @@ -0,0 +1,39 @@ +//===--- RedundantNestedIfCheck.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 +// +//===----------------------------------------------------------------------===// + +#ifndef LLVM_CLANG_TOOLS_EXTRA_CLANG_TIDY_READABILITY_REDUNDANTNESTEDIFCHECK_H +#define LLVM_CLANG_TOOLS_EXTRA_CLANG_TIDY_READABILITY_REDUNDANTNESTEDIFCHECK_H + +#include "../ClangTidyCheck.h" + +namespace clang::tidy::readability { + +/// Detects nested ``if`` statements that can be merged into one by +/// concatenating conditions with ``&&``. +class RedundantNestedIfCheck : public ClangTidyCheck { +public: + enum class UserDefinedBoolConversionMode { + None, + WarnOnly, + WarnAndFix, + }; + + RedundantNestedIfCheck(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: + const bool WarnOnDependentConstexprIf; + const UserDefinedBoolConversionMode UserBoolConversionMode; +}; + +} // namespace clang::tidy::readability + +#endif // LLVM_CLANG_TOOLS_EXTRA_CLANG_TIDY_READABILITY_REDUNDANTNESTEDIFCHECK_H diff --git a/clang-tools-extra/docs/ReleaseNotes.rst b/clang-tools-extra/docs/ReleaseNotes.rst index 0c487524390e3..b10d25b51105a 100644 --- a/clang-tools-extra/docs/ReleaseNotes.rst +++ b/clang-tools-extra/docs/ReleaseNotes.rst @@ -133,6 +133,12 @@ New checks Finds and removes redundant conversions from ``std::[w|u8|u16|u32]string_view`` to ``std::[...]string`` in call expressions expecting ``std::[...]string_view``. +- New :doc:`readability-redundant-nested-if + <clang-tidy/checks/readability/redundant-nested-if>` check. + + Finds nested ``if`` statements that can be merged into a single ``if`` by + combining conditions with ``&&``. + - New :doc:`readability-trailing-comma <clang-tidy/checks/readability/trailing-comma>` check. diff --git a/clang-tools-extra/docs/clang-tidy/checks/list.rst b/clang-tools-extra/docs/clang-tidy/checks/list.rst index 0eabd9929dc39..d86b96e8f32e9 100644 --- a/clang-tools-extra/docs/clang-tidy/checks/list.rst +++ b/clang-tools-extra/docs/clang-tidy/checks/list.rst @@ -412,6 +412,7 @@ Clang-Tidy Checks :doc:`readability-redundant-function-ptr-dereference <readability/redundant-function-ptr-dereference>`, "Yes" :doc:`readability-redundant-inline-specifier <readability/redundant-inline-specifier>`, "Yes" :doc:`readability-redundant-member-init <readability/redundant-member-init>`, "Yes" + :doc:`readability-redundant-nested-if <readability/redundant-nested-if>`, "Yes" :doc:`readability-redundant-parentheses <readability/redundant-parentheses>`, "Yes" :doc:`readability-redundant-preprocessor <readability/redundant-preprocessor>`, :doc:`readability-redundant-smartptr-get <readability/redundant-smartptr-get>`, "Yes" diff --git a/clang-tools-extra/docs/clang-tidy/checks/readability/redundant-nested-if.rst b/clang-tools-extra/docs/clang-tidy/checks/readability/redundant-nested-if.rst new file mode 100644 index 0000000000000..847c5a70d0be2 --- /dev/null +++ b/clang-tools-extra/docs/clang-tidy/checks/readability/redundant-nested-if.rst @@ -0,0 +1,118 @@ +.. title:: clang-tidy - readability-redundant-nested-if + +readability-redundant-nested-if +=============================== + +Finds nested ``if`` statements that can be merged by combining their conditions +with ``&&``. + +Example: + +.. code-block:: c++ + + if (a) { + if (b) { + work(); + } + } + +becomes + +.. code-block:: c++ + + if ((a) && (b)) { + work(); + } + +The check can merge longer chains as well: + +.. code-block:: c++ + + if (a) { + if (b) { + if (c) { + work(); + } + } + } + +becomes + +.. code-block:: c++ + + if ((a) && (b) && (c)) { + work(); + } + +The check also supports outer declaration conditions in C++17 and later: + +.. code-block:: c++ + + if (bool x = ready()) { + if (can_run()) { + work(); + } + } + +becomes + +.. code-block:: c++ + + if (bool x = ready(); x && (can_run())) { + work(); + } + +Safety rules +------------ + +The check only transforms chains where: + +- Neither outer nor nested ``if`` has an ``else`` branch. +- Nested merged ``if`` statements do not use condition variables. +- In C++17 and later, the outermost ``if`` may use a condition variable if it + can be rewritten to an init-statement form, for example + ``if (auto v = f())`` to ``if (auto v = f(); v && ...)``. +- When the outermost statement is already in ``if (init; cond)`` form, the + check keeps ``init`` unchanged and merges only into ``cond``. +- By default, merged conditions avoid user-defined ``bool`` conversions to + preserve short-circuit semantics. This can be changed with + `UserDefinedBoolConversionMode`. +- Only the outermost ``if`` may have an init-statement. +- No merged ``if`` is ``if consteval``. +- All merged ``if`` statements are either all ``if constexpr`` or all regular + ``if``. +- No merged ``if`` statement has statement attributes. +- All rewritten ranges are free of macro/preprocessor-sensitive edits. +- Fix-its are suppressed when comments in removed nested headers cannot be + preserved safely. Comments inside conditions are preserved, while + other comments between the ``ifs`` disable fix-its. + +For ``if constexpr``, nested merged conditions must be +non-instantiation-dependent to avoid template semantic changes. The outermost +condition may be instantiation-dependent when all nested merged conditions are +constant ``true``. + +Options +------- + +.. option:: `UserDefinedBoolConversionMode` + + Controls how chains with an outer condition that relies on user-defined + ``bool`` conversion are handled. + + - `None` + No diagnostic is emitted for those chains. + - `WarnOnly` + Emit diagnostics, but do not provide fix-its. + - `WarnAndFix` + Emit diagnostics and provide fix-its. + + Default is `None`. + +.. option:: `WarnOnDependentConstexprIf` + + When set to `true`, the check also emits diagnostics for remaining unsafe + ``if constexpr`` chains (for example, with instantiation-dependent nested + conditions), but does not provide a fix-it for them. + + Default is `false`. diff --git a/clang-tools-extra/test/clang-tidy/checkers/readability/redundant-nested-if.cpp b/clang-tools-extra/test/clang-tidy/checkers/readability/redundant-nested-if.cpp new file mode 100644 index 0000000000000..7c26883395718 --- /dev/null +++ b/clang-tools-extra/test/clang-tidy/checkers/readability/redundant-nested-if.cpp @@ -0,0 +1,474 @@ +// RUN: %check_clang_tidy -check-suffixes=BASE -std=c++11 %s readability-redundant-nested-if %t -- -- -fno-delayed-template-parsing +// RUN: %check_clang_tidy -check-suffixes=BASE,CXX17 -std=c++17 %s readability-redundant-nested-if %t -- -- -fno-delayed-template-parsing +// RUN: %check_clang_tidy -check-suffixes=BASE,CXX17,CXX20 -std=c++20 %s readability-redundant-nested-if %t -- -- -fno-delayed-template-parsing +// RUN: %check_clang_tidy -check-suffixes=BASE,CXX17,CXX20,CXX23 -std=c++23 %s readability-redundant-nested-if %t -- -- -fno-delayed-template-parsing +// RUN: %check_clang_tidy -check-suffixes=BASE,CXX17,CXX20,CXX23,CXX26 -std=c++26 %s readability-redundant-nested-if %t -- -- -fno-delayed-template-parsing +// RUN: %check_clang_tidy -check-suffixes=BASE,CXX17,CXX20,CXX23,CXX26,OPTWARN -std=c++26 %s readability-redundant-nested-if %t -- -config='{CheckOptions: {readability-redundant-nested-if.WarnOnDependentConstexprIf: true, readability-redundant-nested-if.UserDefinedBoolConversionMode: WarnOnly}}' -- -fno-delayed-template-parsing +// RUN: %check_clang_tidy -check-suffixes=BASE,CXX17,OPTFIX -std=c++17 %s readability-redundant-nested-if %t -- -config='{CheckOptions: {readability-redundant-nested-if.UserDefinedBoolConversionMode: WarnAndFix}}' -- -fno-delayed-template-parsing + +bool cond(int X = 0); +void sink(); +void bar(); +struct BoolLike { + explicit operator bool() const; +}; +BoolLike make_bool_like(); + +#define INNER_IF(C) if (C) sink() +#define COND_MACRO cond() +#define OUTER_IF if (cond()) + +// Core coverage under default options. +void positive_cases() { + // CHECK-MESSAGES-BASE: :[[@LINE+1]]:3: warning: nested if statements can be merged + if (cond()) { + if (cond(1)) { + sink(); + } + } + // CHECK-FIXES-BASE: if ((cond()) && (cond(1))) + // CHECK-FIXES-BASE: sink(); + + // CHECK-MESSAGES-BASE: :[[@LINE+1]]:3: warning: nested if statements can be merged + if (cond() || cond(1)) + if (cond(2)) + sink(); + // CHECK-FIXES-BASE: if ((cond() || cond(1)) && (cond(2))) + // CHECK-FIXES-BASE: sink(); + + // CHECK-MESSAGES-BASE: :[[@LINE+1]]:3: warning: nested if statements can be merged + if (cond()) { + if (cond(1)) + if (cond(2)) { + sink(); + } + } + // CHECK-FIXES-BASE: if ((cond()) && (cond(1)) && (cond(2))) + // CHECK-FIXES-BASE: sink(); +} + +void stress_long_chain_case() { + // CHECK-MESSAGES-BASE: :[[@LINE+1]]:3: warning: nested if statements can be merged + if (cond(0)) { + if (cond(1)) { + if (cond(2)) { + if (cond(3)) { + if (cond(4)) { + if (cond(5)) { + if (cond(6)) { + if (cond(7)) { + if (cond(8)) { + if (cond(9)) { + if (cond(10)) { + if (cond(11)) + sink(); + } + } + } + } + } + } + } + } + } + } + } + // CHECK-FIXES-BASE: if ((cond(0)) && (cond(1)) && (cond(2)) && (cond(3)) && (cond(4)) && (cond(5)) && (cond(6)) && (cond(7)) && (cond(8)) && (cond(9)) && (cond(10)) && (cond(11))) + // CHECK-FIXES-BASE: sink(); +} + +void nested_chains_are_diagnosed_once_per_chain() { + // CHECK-MESSAGES-BASE: :[[@LINE+2]]:3: warning: nested if statements can be merged + // CHECK-MESSAGES-BASE: :[[@LINE+4]]:7: warning: nested if statements can be merged + if (cond()) { + if (cond(1)) { + sink(); + if (cond(2)) { + if (cond(3)) + sink(); + } + } + } + // CHECK-FIXES-BASE: if ((cond()) && (cond(1))) + // CHECK-FIXES-BASE: if ((cond(2)) && (cond(3))) +} + +void child_chain_is_reported_when_macro_parent_is_unfixable() { + // CHECK-MESSAGES-BASE: :[[@LINE+2]]:5: warning: nested if statements can be merged + OUTER_IF { + if (cond(1)) { + if (cond(2)) + sink(); + } + } + // CHECK-FIXES-BASE: OUTER_IF { + // CHECK-FIXES-BASE: if ((cond(1)) && (cond(2))) + // CHECK-FIXES-BASE: sink(); +} + +void negative_cases() { + // CHECK-MESSAGES-BASE-NOT: :[[@LINE+1]]:3: warning: nested if statements can be merged + // CHECK-MESSAGES-CXX17: :[[@LINE+1]]:3: warning: nested if statements can be merged + if (bool B = cond()) { + if (cond(1)) + sink(); + } + // CHECK-FIXES-CXX17: if (bool B = cond(); B && (cond(1))) + // CHECK-FIXES-CXX17: sink(); + + // CHECK-MESSAGES-BASE-NOT: :[[@LINE+1]]:3: warning: nested if statements can be merged + if (cond()) { + if (bool B = cond(1)) + sink(); + } + + // CHECK-MESSAGES-BASE-NOT: :[[@LINE+1]]:3: warning: nested if statements can be merged + if (cond()) { + if (cond(1)) + sink(); + else + sink(); + } + + // CHECK-MESSAGES-BASE-NOT: :[[@LINE+1]]:3: warning: nested if statements can be merged + if (cond()) { + if (cond(1)) + sink(); + } else { + sink(); + } + + // CHECK-MESSAGES-BASE-NOT: :[[@LINE+1]]:3: warning: nested if statements can be merged + if (cond()) { + sink(); + if (cond(1)) + sink(); + } + + // CHECK-MESSAGES-BASE-NOT: :[[@LINE+1]]:3: warning: nested if statements can be merged + if (cond()) { + if (cond(1)) + sink(); + sink(); + } + +} + +// Option-specific behavior for UserDefinedBoolConversionMode. +void user_defined_bool_conversion_mode_cases() { + // CHECK-MESSAGES-BASE-NOT: :[[@LINE+3]]:3: warning: nested if statements can be merged + // CHECK-MESSAGES-OPTWARN: :[[@LINE+2]]:3: warning: nested if statements can be merged + // CHECK-MESSAGES-OPTFIX: :[[@LINE+1]]:3: warning: nested if statements can be merged + if (make_bool_like()) { + if (cond(1)) + sink(); + } + // CHECK-FIXES-OPTFIX: if ((make_bool_like()) && (cond(1))) + // CHECK-FIXES-OPTFIX: sink(); +} + +void macro_and_preprocessor_cases() { + // CHECK-MESSAGES-BASE-NOT: :[[@LINE+1]]:3: warning: nested if statements can be merged + if (cond()) { + INNER_IF(cond(1)); + } + + // CHECK-MESSAGES-BASE-NOT: :[[@LINE+1]]:3: warning: nested if statements can be merged + if (COND_MACRO) { + if (cond(1)) + sink(); + } + + // CHECK-MESSAGES-BASE-NOT: :[[@LINE+1]]:3: warning: nested if statements can be merged + if (cond()) { +#if 1 + if (cond(1)) + sink(); +#endif + } +} + +void comment_handling_cases() { + // Comments inside condition payloads are preserved by the merged condition. + // CHECK-MESSAGES-BASE: :[[@LINE+1]]:3: warning: nested if statements can be merged + if (cond() /* outer payload */) { + if (/* inner payload */ cond(1)) + sink(); + } + // CHECK-FIXES-BASE: if ((cond() /* outer payload */) && (/* inner payload */ cond(1))) + // CHECK-FIXES-BASE: sink(); + + // Trailing comments in nested headers are warning-only (no fix-it). + // CHECK-MESSAGES-BASE: :[[@LINE+1]]:3: warning: nested if statements can be merged + if (cond()) // outer trailing + if (cond(1)) // inner trailing + sink(); + // CHECK-FIXES-BASE: if (cond()) // outer trailing + // CHECK-FIXES-BASE-NEXT: if (cond(1)) // inner trailing + // CHECK-FIXES-BASE-NEXT: sink(); + + // Comments in other nested-header locations are warning-only (no fix-it). + // CHECK-MESSAGES-BASE: :[[@LINE+1]]:3: warning: nested if statements can be merged + if (cond()) { + if /* nested header comment */ (cond(1)) + sink(); + } + // CHECK-FIXES-BASE: if /* nested header comment */ (cond(1)) +} + +#if __cplusplus >= 201703L +int side_effect(); + +// C++17 language feature coverage. +void init_statement_cases() { + // CHECK-MESSAGES-CXX17: :[[@LINE+1]]:3: warning: nested if statements can be merged + if (side_effect(); cond()) { + if (cond(1)) + sink(); + } + // CHECK-FIXES-CXX17: if (side_effect(); (cond()) && (cond(1))) + // CHECK-FIXES-CXX17: sink(); + + // CHECK-MESSAGES-CXX17: :[[@LINE+1]]:3: warning: nested if statements can be merged + if (auto Guard = side_effect()) { + if (cond(1)) + sink(); + } + // CHECK-FIXES-CXX17: if (auto Guard = side_effect(); Guard && (cond(1))) + // CHECK-FIXES-CXX17: sink(); + + // CHECK-MESSAGES-CXX17-NOT: :[[@LINE+1]]:3: warning: nested if statements can be merged + if (cond()) { + if (bool InnerInit = cond(1); InnerInit) + sink(); + } + + // Macro-expanded root conditions are diagnostic-only unsafe, so no warning + // with fix-it is emitted. + // CHECK-MESSAGES-CXX17-NOT: :[[@LINE+1]]:3: warning: nested if statements can be merged + if (side_effect(); COND_MACRO) { + if (cond(1)) + sink(); + } + + // CHECK-MESSAGES-CXX17: :[[@LINE+1]]:3: warning: nested if statements can be merged + if (bool X = cond(); X) { + if (cond()) { + if (cond(1)) + bar(); + } + } + // CHECK-FIXES-CXX17: if (bool X = cond(); (X) && (cond()) && (cond(1))) + // CHECK-FIXES-CXX17: bar(); + + // CHECK-MESSAGES-CXX17: :[[@LINE+1]]:3: warning: nested if statements can be merged + if (bool X = cond() /* here */) { + if (cond()) { + bar(); + } + } + // CHECK-FIXES-CXX17: if (bool X = cond() /* here */; X && (cond())) + // CHECK-FIXES-CXX17: bar(); +} + +void declaration_condition_type_safety() { + // CHECK-MESSAGES-CXX17-NOT: :[[@LINE+3]]:3: warning: nested if statements can be merged + // CHECK-MESSAGES-OPTWARN: :[[@LINE+2]]:3: warning: nested if statements can be merged + // CHECK-MESSAGES-OPTFIX: :[[@LINE+1]]:3: warning: nested if statements can be merged + if (auto Guard = make_bool_like()) { + if (cond(1)) + sink(); + } + // CHECK-FIXES-OPTFIX: if (auto Guard = make_bool_like(); Guard && (cond(1))) + // CHECK-FIXES-OPTFIX: sink(); + + // CHECK-MESSAGES-CXX17: :[[@LINE+1]]:3: warning: nested if statements can be merged + if (bool X = cond()) { + if (cond()) { + if (cond(1)) + bar(); + } + } + // CHECK-FIXES-CXX17: if (bool X = cond(); X && (cond()) && (cond(1))) + // CHECK-FIXES-CXX17: bar(); + + // Macro-expanded declaration conditions are diagnostic-only unsafe, so no + // warning with fix-it is emitted. + // CHECK-MESSAGES-CXX17-NOT: :[[@LINE+1]]:3: warning: nested if statements can be merged + if (bool X = COND_MACRO) { + if (cond(1)) + sink(); + } + +} + +constexpr bool C1 = true; +constexpr bool C2 = false; + +void constexpr_non_template() { + // CHECK-MESSAGES-CXX17: :[[@LINE+1]]:3: warning: nested if statements can be merged + if constexpr (C1) { + if constexpr (C2) { + sink(); + } + } + // CHECK-FIXES-CXX17: if constexpr ((C1) && (C2)) + // CHECK-FIXES-CXX17: sink(); +} + +template <bool B> void dependent_constexpr_outer_is_fixable() { + // CHECK-MESSAGES-CXX17: :[[@LINE+1]]:3: warning: nested if statements can be merged + if constexpr (B) { + if constexpr (true) + sink(); + } + // CHECK-FIXES-CXX17: if constexpr ((B) && (true)) + // CHECK-FIXES-CXX17: sink(); +} + +template <bool B> void dependent_constexpr_outer_is_unsafe_when_nested_false() { + // CHECK-MESSAGES-CXX17-NOT: :[[@LINE+1]]:3: warning: nested if statements can be merged + // CHECK-MESSAGES-OPTWARN: :[[@LINE+1]]:3: warning: nested instantiation-dependent if constexpr statements can be merged + if constexpr (B) { + if constexpr (false) + sink(); + } +} + +template <typename T> void dependent_constexpr_operand_warn_only_under_option() { + // CHECK-MESSAGES-CXX17-NOT: :[[@LINE+1]]:3: warning: nested if statements can be merged + // CHECK-MESSAGES-OPTWARN: :[[@LINE+1]]:3: warning: nested instantiation-dependent if constexpr statements can be merged + if constexpr (true) { + if constexpr (sizeof(T) == 4) + sink(); + } +} + +template <typename T> void dependent_constexpr_type_chain_outer_is_fixable() { + // CHECK-MESSAGES-CXX17: :[[@LINE+1]]:3: warning: nested if statements can be merged + if constexpr (sizeof(T) == 4) { + if constexpr (true) + sink(); + } + // CHECK-FIXES-CXX17: if constexpr ((sizeof(T) == 4) && (true)) + // CHECK-FIXES-CXX17: sink(); +} + +void mixed_constexpr_and_non_constexpr(bool B) { + // CHECK-MESSAGES-CXX17-NOT: :[[@LINE+1]]:3: warning: nested if statements can be merged + if constexpr (C1) { + if (B) + sink(); + } + + // CHECK-MESSAGES-CXX17-NOT: :[[@LINE+1]]:3: warning: nested if statements can be merged + if (B) { + if constexpr (C1) + sink(); + } +} + +#if __cplusplus >= 202400L +template <typename T> void constexpr_template_static_assert() { + // CHECK-MESSAGES-CXX26-NOT: :[[@LINE+2]]:3: warning: nested if statements can be merged + // CHECK-MESSAGES-OPTWARN: :[[@LINE+1]]:3: warning: nested instantiation-dependent if constexpr statements can be merged + if constexpr (sizeof(T) == 1) { + if constexpr (false) { + static_assert(false, "discarded in template context"); + } + } +} +#endif + +#endif + +#if __cplusplus >= 202002L +void constexpr_requires_expression_cases() { + // CHECK-MESSAGES-CXX20: :[[@LINE+1]]:3: warning: nested if statements can be merged + if constexpr (requires { sizeof(int); }) { + if constexpr (requires { sizeof(long); }) + sink(); + } + // CHECK-FIXES-CXX20: if constexpr ((requires { sizeof(int); }) && (requires { sizeof(long); })) + // CHECK-FIXES-CXX20: sink(); +} + +void constexpr_init_statement_cases() { + // CHECK-MESSAGES-CXX20: :[[@LINE+1]]:3: warning: nested if statements can be merged + if constexpr (constexpr bool HasInt = requires { sizeof(int); }; HasInt) { + if constexpr (requires { sizeof(long); }) + sink(); + } + // CHECK-FIXES-CXX20: if constexpr (constexpr bool HasInt = requires { sizeof(int); }; (HasInt) && (requires { sizeof(long); })) + // CHECK-FIXES-CXX20: sink(); +} + +template <typename T> void dependent_requires_outer_is_fixable() { + // CHECK-MESSAGES-CXX20: :[[@LINE+1]]:3: warning: nested if statements can be merged + if constexpr (requires { typename T::type; }) { + if constexpr (true) + sink(); + } + // CHECK-FIXES-CXX20: if constexpr ((requires { typename T::type; }) && (true)) + // CHECK-FIXES-CXX20: sink(); +} + +void attribute_cases(bool B1, bool B2) { + // CHECK-MESSAGES-CXX20-NOT: :[[@LINE+1]]:3: warning: nested if statements can be merged + if (B1) { + // CHECK-MESSAGES-CXX20-NOT: :[[@LINE+1]]:5: warning: nested if statements can be merged + [[likely]] if (B2) + sink(); + } + + // CHECK-MESSAGES-CXX20-NOT: :[[@LINE+1]]:3: warning: nested if statements can be merged + [[likely]] if (B1) { + if (B2) + sink(); + } +} + +void still_merges_without_attributes(bool B1, bool B2) { + // CHECK-MESSAGES-CXX20: :[[@LINE+1]]:3: warning: nested if statements can be merged + if (B1) { + if (B2) + sink(); + } + // CHECK-FIXES-CXX20: if ((B1) && (B2)) + // CHECK-FIXES-CXX20: sink(); +} +#endif + +#ifdef __cpp_if_consteval +consteval bool compile_time_true() { return true; } + +void consteval_cases(bool B1, bool B2) { + // CHECK-MESSAGES-CXX23-NOT: :[[@LINE+1]]:3: warning: nested if statements can be merged + if consteval { + if (B1) + sink(); + } else { + sink(); + } + + // CHECK-MESSAGES-CXX23-NOT: :[[@LINE+1]]:3: warning: nested if statements can be merged + if (!B1) { + if consteval { + sink(); + } else { + if (B2) + sink(); + } + } + +} + +template <bool B> constexpr void consteval_nested_in_constexpr() { + // CHECK-MESSAGES-CXX23-NOT: :[[@LINE+1]]:3: warning: nested if statements can be merged + if constexpr (B) { + if consteval { + sink(); + } + } +} +#endif _______________________________________________ cfe-commits mailing list [email protected] https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
