[clang-tools-extra] [clang-tidy] Add `readability-redundant-nested-if` check (PR #181558)
Daniil Dudkin via cfe-commits
cfe-commits at lists.llvm.org
Sat May 9 15:55:12 PDT 2026
https://github.com/unterumarmung updated https://github.com/llvm/llvm-project/pull/181558
>From b7a2b3c01dbfa71735b1e27962cd0b244e04332f Mon Sep 17 00:00:00 2001
From: Daniil Dudkin <unterumarmung at yandex.ru>
Date: Sun, 15 Feb 2026 20:33:54 +0300
Subject: [PATCH 1/8] [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 0dfdbe69c880d..ce43e1b52ccfd 100644
--- a/clang-tools-extra/clang-tidy/readability/CMakeLists.txt
+++ b/clang-tools-extra/clang-tidy/readability/CMakeLists.txt
@@ -46,6 +46,7 @@ add_clang_library(clangTidyReadabilityModule STATIC
RedundantFunctionPtrDereferenceCheck.cpp
RedundantLambdaParameterListCheck.cpp
RedundantMemberInitCheck.cpp
+ RedundantNestedIfCheck.cpp
RedundantParenthesesCheck.cpp
RedundantPreprocessorCheck.cpp
RedundantQualifiedAliasCheck.cpp
diff --git a/clang-tools-extra/clang-tidy/readability/ReadabilityTidyModule.cpp b/clang-tools-extra/clang-tidy/readability/ReadabilityTidyModule.cpp
index 86e4f4dcc1de1..f202453559d99 100644
--- a/clang-tools-extra/clang-tidy/readability/ReadabilityTidyModule.cpp
+++ b/clang-tools-extra/clang-tidy/readability/ReadabilityTidyModule.cpp
@@ -48,6 +48,7 @@
#include "RedundantInlineSpecifierCheck.h"
#include "RedundantLambdaParameterListCheck.h"
#include "RedundantMemberInitCheck.h"
+#include "RedundantNestedIfCheck.h"
#include "RedundantParenthesesCheck.h"
#include "RedundantPreprocessorCheck.h"
#include "RedundantQualifiedAliasCheck.h"
@@ -180,6 +181,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 51251eacbcd5e..07b254bccc526 100644
--- a/clang-tools-extra/docs/ReleaseNotes.rst
+++ b/clang-tools-extra/docs/ReleaseNotes.rst
@@ -188,6 +188,12 @@ New checks
Finds lambda expressions with a redundant empty parameter list and removes it.
+- 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-redundant-qualified-alias
<clang-tidy/checks/readability/redundant-qualified-alias>` check.
diff --git a/clang-tools-extra/docs/clang-tidy/checks/list.rst b/clang-tools-extra/docs/clang-tidy/checks/list.rst
index 3e3cd92374ee9..3a0e0d3aeaef4 100644
--- a/clang-tools-extra/docs/clang-tidy/checks/list.rst
+++ b/clang-tools-extra/docs/clang-tidy/checks/list.rst
@@ -416,6 +416,7 @@ Clang-Tidy Checks
:doc:`readability-redundant-inline-specifier <readability/redundant-inline-specifier>`, "Yes"
:doc:`readability-redundant-lambda-parameter-list <readability/redundant-lambda-parameter-list>`, "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-qualified-alias <readability/redundant-qualified-alias>`, "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
>From 159fa1bd33282f4e1953a7042d77abb4d125ab60 Mon Sep 17 00:00:00 2001
From: Daniil Dudkin <unterumarmung at yandex.ru>
Date: Sun, 15 Feb 2026 20:55:00 +0300
Subject: [PATCH 2/8] fix review comments
---
.../clang-tidy/readability/RedundantNestedIfCheck.h | 2 +-
.../checks/readability/redundant-nested-if.rst | 12 +++++++++++-
2 files changed, 12 insertions(+), 2 deletions(-)
diff --git a/clang-tools-extra/clang-tidy/readability/RedundantNestedIfCheck.h b/clang-tools-extra/clang-tidy/readability/RedundantNestedIfCheck.h
index 33ead69ebc2b4..4e85017a70107 100644
--- a/clang-tools-extra/clang-tidy/readability/RedundantNestedIfCheck.h
+++ b/clang-tools-extra/clang-tidy/readability/RedundantNestedIfCheck.h
@@ -1,4 +1,4 @@
-//===--- 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.
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
index 847c5a70d0be2..451f16eb83544 100644
--- 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
@@ -68,21 +68,31 @@ 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`.
+ :option:`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.
>From 19b9b32d17a478fc4bf5712fd158403654a479fe Mon Sep 17 00:00:00 2001
From: Daniil Dudkin <unterumarmung at yandex.ru>
Date: Sun, 15 Feb 2026 21:10:31 +0300
Subject: [PATCH 3/8] fix docs build
---
.../clang-tidy/checks/readability/redundant-nested-if.rst | 4 ++--
1 file changed, 2 insertions(+), 2 deletions(-)
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
index 451f16eb83544..51c502114b55b 100644
--- 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
@@ -105,7 +105,7 @@ constant ``true``.
Options
-------
-.. option:: `UserDefinedBoolConversionMode`
+.. option:: UserDefinedBoolConversionMode
Controls how chains with an outer condition that relies on user-defined
``bool`` conversion are handled.
@@ -119,7 +119,7 @@ Options
Default is `None`.
-.. option:: `WarnOnDependentConstexprIf`
+.. option:: WarnOnDependentConstexprIf
When set to `true`, the check also emits diagnostics for remaining unsafe
``if constexpr`` chains (for example, with instantiation-dependent nested
>From f17e8128748388401f92f92c14ba929ed3e49e71 Mon Sep 17 00:00:00 2001
From: Daniil Dudkin <unterumarmung at yandex.ru>
Date: Fri, 3 Apr 2026 23:59:29 +0300
Subject: [PATCH 4/8] Fix review comments
---
.../readability/RedundantNestedIfCheck.cpp | 888 ++++++++----------
.../readability/RedundantNestedIfCheck.h | 8 +-
.../readability/redundant-nested-if.rst | 40 +-
.../readability/redundant-nested-if.cpp | 190 ++--
4 files changed, 488 insertions(+), 638 deletions(-)
diff --git a/clang-tools-extra/clang-tidy/readability/RedundantNestedIfCheck.cpp b/clang-tools-extra/clang-tidy/readability/RedundantNestedIfCheck.cpp
index 2fc9c5fdc5ff3..371c5cf31b779 100644
--- a/clang-tools-extra/clang-tidy/readability/RedundantNestedIfCheck.cpp
+++ b/clang-tools-extra/clang-tidy/readability/RedundantNestedIfCheck.cpp
@@ -11,45 +11,36 @@
#include "clang/AST/ASTContext.h"
#include "clang/AST/Stmt.h"
#include "clang/ASTMatchers/ASTMatchFinder.h"
+#include "clang/Basic/Diagnostic.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 <cassert>
#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";
+static constexpr llvm::StringLiteral AllowUserDefinedBoolConversionStr =
+ "AllowUserDefinedBoolConversion";
+static constexpr llvm::StringLiteral MergeableIfDiag =
+ "nested if statements can be merged";
+static constexpr llvm::StringLiteral DependentConstexprDiag =
+ "nested instantiation-dependent if constexpr statements can be merged";
+static constexpr llvm::StringLiteral NestedIfNote =
+ "nested if statement to merge is here";
namespace {
+
+using IfChain = llvm::SmallVector<const IfStmt *>;
+
enum class ChainHandling {
None,
WarnOnly,
@@ -67,156 +58,109 @@ 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;
+// on contextual user-defined conversion to `bool`.
+static bool containsUserDefinedBoolConversion(const Expr *Expression) {
+ assert(Expression);
- if (const auto *Cast = dyn_cast<ImplicitCastExpr>(ExprNode);
+ if (const auto *Cast = dyn_cast<ImplicitCastExpr>(Expression);
Cast && Cast->getCastKind() == CK_UserDefinedConversion)
return true;
- return llvm::any_of(ExprNode->children(), [](const Stmt *Child) {
+ return llvm::any_of(Expression->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) {
+static bool conditionNeedsBoolCast(const Expr *Condition) {
+ assert(Condition);
+
+ if (!containsUserDefinedBoolConversion(Condition))
return false;
- }
- const Expr *Unwrapped = Cond->IgnoreParenImpCasts();
+
+ const Expr *const Unwrapped =
+ Condition->IgnoreImplicitAsWritten()->IgnoreParens();
if (!Unwrapped)
+ return true;
+
+ const QualType ConditionType = Unwrapped->getType();
+ return ConditionType.isNull() || !ConditionType->isScalarType();
+}
+
+static bool
+isConditionExpressionMergeable(const Expr *Condition,
+ bool AllowUserDefinedBoolConversion) {
+ assert(Condition);
+
+ if (Condition->isTypeDependent())
return false;
- const QualType CondType = Unwrapped->getType();
- if (CondType.isNull())
+
+ if (containsUserDefinedBoolConversion(Condition))
+ return AllowUserDefinedBoolConversion;
+
+ const Expr *const Unwrapped = Condition->IgnoreParenImpCasts();
+ if (!Unwrapped)
return false;
- if (CondType->isScalarType())
- return true;
- return UserBoolConversionMode ==
- RedundantNestedIfCheck::UserDefinedBoolConversionMode::WarnAndFix;
+
+ const QualType ConditionType = Unwrapped->getType();
+ return !ConditionType.isNull() && ConditionType->isScalarType();
}
static std::optional<CharSourceRange>
-getConditionPayloadRange(const IfStmt *If, const SourceManager &SM,
- const LangOptions &LangOpts) {
- if (!If)
- return std::nullopt;
- const SourceLocation PayloadBegin =
+getIfConditionRange(const IfStmt *If, const SourceManager &SM,
+ const LangOptions &LangOpts) {
+ assert(If);
+
+ const SourceLocation ConditionBegin =
Lexer::getLocForEndOfToken(If->getLParenLoc(), 0, SM, LangOpts);
- if (PayloadBegin.isInvalid() || If->getRParenLoc().isInvalid())
+ if (ConditionBegin.isInvalid() || If->getRParenLoc().isInvalid())
return std::nullopt;
- const CharSourceRange PayloadRange =
- CharSourceRange::getCharRange(PayloadBegin, If->getRParenLoc());
+ const CharSourceRange ConditionRange =
+ CharSourceRange::getCharRange(ConditionBegin, If->getRParenLoc());
const CharSourceRange FileRange =
- Lexer::makeFileCharRange(PayloadRange, SM, LangOpts);
+ Lexer::makeFileCharRange(ConditionRange, 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)
+getIfConditionText(const IfStmt *If, const SourceManager &SM,
+ const LangOptions &LangOpts) {
+ const std::optional<CharSourceRange> ConditionRange =
+ getIfConditionRange(If, SM, LangOpts);
+ if (!ConditionRange)
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;
+ const StringRef ConditionText =
+ Lexer::getSourceText(*ConditionRange, SM, LangOpts, &Invalid);
+ return Invalid || ConditionText.empty()
+ ? std::nullopt
+ : std::optional<std::string>(ConditionText.str());
}
-static bool locationInCharRange(SourceLocation Loc, CharSourceRange Range,
- const SourceManager &SM) {
- if (Loc.isInvalid() || Range.isInvalid())
- return false;
- return !SM.isBeforeInTranslationUnit(Loc, Range.getBegin()) &&
+static bool isLocationInCharRange(SourceLocation Loc, CharSourceRange Range,
+ const SourceManager &SM) {
+ return Loc.isValid() && Range.isValid() &&
+ !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.
+// Keep fix-its only when comments in removed nested headers stay inside the
+// preserved condition text. Other comment placements keep the diagnostic but
+// suppress the rewrite.
static bool hasOnlyPayloadCommentsInNestedHeader(const IfStmt *Nested,
const SourceManager &SM,
const LangOptions &LangOpts) {
- if (!Nested || !Nested->getThen())
- return false;
+ assert(Nested);
+ assert(Nested->getThen());
const CharSourceRange HeaderRange = CharSourceRange::getCharRange(
Nested->getBeginLoc(), Nested->getThen()->getBeginLoc());
@@ -226,44 +170,38 @@ static bool hasOnlyPayloadCommentsInNestedHeader(const IfStmt *Nested,
return false;
const std::optional<CharSourceRange> PayloadRange =
- getConditionPayloadRange(Nested, SM, LangOpts);
+ getIfConditionRange(Nested, SM, LangOpts);
if (!PayloadRange)
return false;
const std::vector<utils::lexer::CommentToken> Comments =
- getCommentTokensInRange(HeaderFileRange, SM, LangOpts);
+ utils::lexer::getCommentsInRange(HeaderFileRange, SM, LangOpts);
return llvm::all_of(Comments, [&](const utils::lexer::CommentToken &Comment) {
- return locationInCharRange(Comment.Loc, *PayloadRange, SM);
+ return isLocationInCharRange(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())
+// Only an outer condition variable can be rewritten safely by moving the
+// declaration into an init-statement and conjoining the condition variable.
+static bool
+canRewriteOuterConditionVariable(const IfStmt *If,
+ bool AllowUserDefinedBoolConversion) {
+ assert(If);
+ assert(If->hasVarStorage());
+
+ if (If->hasInitStorage())
return false;
- if (UserBoolConversionMode !=
- RedundantNestedIfCheck::UserDefinedBoolConversionMode::WarnAndFix &&
- !VarType->isScalarType()) {
+
+ const auto *const ConditionVariable = If->getConditionVariable();
+ const auto *const ConditionVariableDeclStmt =
+ If->getConditionVariableDeclStmt();
+ if (!ConditionVariable || !ConditionVariableDeclStmt ||
+ !ConditionVariableDeclStmt->isSingleDecl() ||
+ ConditionVariable->getName().empty())
return false;
- }
- return isConditionExpressionSafeToConjoin(If->getCond(),
- UserBoolConversionMode);
+
+ return If->getCond() && isConditionExpressionMergeable(
+ If->getCond(), AllowUserDefinedBoolConversion);
}
// Accept either `if (...) if (...)` or `if (...) { if (...) }` where the
@@ -273,169 +211,92 @@ static const IfStmt *getOnlyNestedIf(const Stmt *Then) {
return nullptr;
if (const auto *NestedIf = dyn_cast<IfStmt>(Then))
return NestedIf;
- const auto *Compound = dyn_cast<CompoundStmt>(Then);
+
+ const auto *const 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);
+static bool hasMergeableStructure(const IfStmt *If, bool AllowInitStorage,
+ bool RequireConstexpr) {
+ assert(If);
- return If->getCond() && isConditionExpressionSafeToConjoin(
- If->getCond(), UserBoolConversionMode);
+ return If->getThen() && !If->isConsteval() && !If->getElse() &&
+ (AllowInitStorage || !If->hasInitStorage()) &&
+ If->isConstexpr() == RequireConstexpr;
}
-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)
+static bool isMergeCandidate(const IfStmt *If, bool AllowInitStorage,
+ bool RequireConstexpr, bool AllowConditionVariable,
+ bool AllowUserDefinedBoolConversion,
+ const LangOptions &LangOpts) {
+ assert(If);
+
+ if (!hasMergeableStructure(If, AllowInitStorage, RequireConstexpr))
return false;
+
if (If->hasVarStorage())
- return AllowConditionVariable && LangOpts.CPlusPlus17;
- return If->getCond() != nullptr;
+ return AllowConditionVariable && LangOpts.CPlusPlus17 &&
+ canRewriteOuterConditionVariable(If, AllowUserDefinedBoolConversion);
+
+ return If->getCond() && isConditionExpressionMergeable(
+ If->getCond(), AllowUserDefinedBoolConversion);
}
-// Statement attributes are attached outside of the `if` token range; removing
-// nested `if` tokens can make attribute placement invalid, so skip them.
+// Statement attributes wrap the `if` in an `AttributedStmt`, so removing nested
+// `if` tokens can invalidate attribute placement.
static bool isAttributedIf(const IfStmt *If, ASTContext &Context) {
- if (!If)
- return false;
+ assert(If);
+
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;
+static IfChain getMergeChain(const IfStmt *Root, ASTContext &Context,
+ bool AllowUserDefinedBoolConversion) {
+ assert(Root);
+ IfChain Chain;
const LangOptions &LangOpts = Context.getLangOpts();
- const bool IsConstexpr = Root->isConstexpr();
- if (!isMergeCandidate(Root, /*AllowInitStorage=*/true, IsConstexpr,
- /*AllowConditionVariable=*/true, LangOpts,
- UserBoolConversionMode) ||
+ const bool RequireConstexpr = Root->isConstexpr();
+ if (!isMergeCandidate(Root, /*AllowInitStorage=*/true,
+ /*RequireConstexpr=*/RequireConstexpr,
+ /*AllowConditionVariable=*/true,
+ AllowUserDefinedBoolConversion, LangOpts) ||
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) ||
+ while (const IfStmt *const Nested = getOnlyNestedIf(Current->getThen())) {
+ if (!isMergeCandidate(Nested, /*AllowInitStorage=*/false,
+ /*RequireConstexpr=*/RequireConstexpr,
+ /*AllowConditionVariable=*/false,
+ AllowUserDefinedBoolConversion, LangOpts) ||
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,
+static bool isConstantBooleanCondition(const Expr *Condition,
+ const ASTContext &Context,
bool RequiredValue) {
- if (!Cond || Cond->isValueDependent() || Cond->isInstantiationDependent())
+ if (!Condition || Condition->isValueDependent() ||
+ Condition->isInstantiationDependent())
return false;
- bool Evaluated = false;
- if (!Cond->EvaluateAsBooleanCondition(Evaluated, Ctx))
- return false;
- return Evaluated == RequiredValue;
+ bool EvaluatedValue = false;
+ return Condition->EvaluateAsBooleanCondition(EvaluatedValue, Context) &&
+ EvaluatedValue == RequiredValue;
}
static bool
@@ -448,51 +309,46 @@ isConstexprChainSemanticallySafe(llvm::ArrayRef<const IfStmt *> Chain,
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())
+ // non-dependent constant `true` expression. This preserves discarded-branch
+ // behavior for template-dependent `if constexpr`.
+ return llvm::all_of(llvm::drop_begin(Chain), [&](const IfStmt *Nested) {
+ const Expr *const NestedCondition = Nested->getCond();
+ if (NestedCondition->isInstantiationDependent())
return false;
- if (OuterIsDependent &&
- !isConstantBooleanCondition(NestedCond, Context,
- /*RequiredValue=*/true)) {
- return false;
- }
- }
- return true;
+ return !OuterIsDependent ||
+ isConstantBooleanCondition(NestedCondition, Context,
+ /*RequiredValue=*/true);
+ });
}
// A range is unsafe for text edits if it crosses macro expansions or
// preprocessor directives.
+template <typename RangeT>
+static bool isUnsafeRange(RangeT Range, CharSourceRange FileRange,
+ bool ContainsExpansionsOrDirectives) {
+ return Range.isInvalid() || Range.getBegin().isMacroID() ||
+ Range.getEnd().isMacroID() || FileRange.isInvalid() ||
+ ContainsExpansionsOrDirectives;
+}
+
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);
+ return isUnsafeRange(
+ Range,
+ Lexer::makeFileCharRange(CharSourceRange::getTokenRange(Range), SM,
+ LangOpts),
+ 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);
+ return isUnsafeRange(Range, Lexer::makeFileCharRange(Range, SM, LangOpts),
+ 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.
+// fix-its. If any range is unsafe, keep looking for a diagnosable child chain.
static bool isFixitSafeForChain(llvm::ArrayRef<const IfStmt *> Chain,
const SourceManager &SM,
const LangOptions &LangOpts) {
@@ -500,133 +356,148 @@ static bool isFixitSafeForChain(llvm::ArrayRef<const IfStmt *> Chain,
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;
- }
- }
+ const std::optional<CharSourceRange> RootConditionRange =
+ Root->hasInitStorage() && !Root->hasVarStorage()
+ ? std::optional<CharSourceRange>(CharSourceRange::getTokenRange(
+ Root->getCond()->getSourceRange()))
+ : getIfConditionRange(Root, SM, LangOpts);
+ if (!RootConditionRange ||
+ isUnsafeCharRange(*RootConditionRange, SM, LangOpts))
+ return false;
+ if (!Root->hasVarStorage() &&
+ isUnsafeTokenRange(Root->getCond()->getSourceRange(), SM, LangOpts))
+ return false;
if (Root->hasVarStorage()) {
- const auto *CondVarDeclStmt = Root->getConditionVariableDeclStmt();
- if (!CondVarDeclStmt ||
- isUnsafeTokenRange(CondVarDeclStmt->getSourceRange(), SM, LangOpts)) {
+ const auto *const ConditionVariableDeclStmt =
+ Root->getConditionVariableDeclStmt();
+ if (!ConditionVariableDeclStmt ||
+ isUnsafeTokenRange(ConditionVariableDeclStmt->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 llvm::ArrayRef<const IfStmt *> ChainRef(Chain);
+ return llvm::all_of(
+ llvm::zip(ChainRef.drop_back(), ChainRef.drop_front()),
+ [&](const auto &ParentAndChild) {
+ const auto &[Parent, Child] = ParentAndChild;
+ if (isUnsafeTokenRange(Child->getCond()->getSourceRange(), SM,
+ LangOpts))
+ return false;
+
+ const CharSourceRange ChildHeaderRange = CharSourceRange::getCharRange(
+ Child->getBeginLoc(), Child->getThen()->getBeginLoc());
+ if (isUnsafeCharRange(ChildHeaderRange, SM, LangOpts))
+ return false;
+
+ const auto *const Wrapper = dyn_cast<CompoundStmt>(Parent->getThen());
+ return !Wrapper ||
+ (!isUnsafeTokenRange(
+ SourceRange(Wrapper->getLBracLoc(), Wrapper->getLBracLoc()),
+ SM, LangOpts) &&
+ !isUnsafeTokenRange(
+ SourceRange(Wrapper->getRBracLoc(), Wrapper->getRBracLoc()),
+ SM, LangOpts));
+ });
+}
- const CharSourceRange NestedHeaderRange = CharSourceRange::getCharRange(
- Nested->getBeginLoc(), Nested->getThen()->getBeginLoc());
- if (isUnsafeCharRange(NestedHeaderRange, SM, LangOpts))
- return false;
+static std::string wrapConditionText(StringRef ConditionText,
+ bool NeedBoolCast) {
+ if (!NeedBoolCast)
+ return ConditionText.str();
- const auto *Wrapper = dyn_cast<CompoundStmt>(Chain[Index - 1]->getThen());
- if (!Wrapper)
- continue;
+ std::string Result("static_cast<bool>(");
+ Result += ConditionText;
+ Result += ")";
+ return Result;
+}
- if (isUnsafeTokenRange(Wrapper->getSourceRange(), SM, LangOpts) ||
- isUnsafeTokenRange(
- SourceRange(Wrapper->getLBracLoc(), Wrapper->getLBracLoc()), SM,
- LangOpts) ||
- isUnsafeTokenRange(
- SourceRange(Wrapper->getRBracLoc(), Wrapper->getRBracLoc()), SM,
- LangOpts)) {
- return false;
- }
+static std::optional<std::string> getConjunctText(const IfStmt *If,
+ const ASTContext &Context,
+ bool UseConditionExprText) {
+ assert(If);
+
+ const SourceManager &SM = Context.getSourceManager();
+ const LangOptions &LangOpts = Context.getLangOpts();
+
+ std::optional<std::string> ConditionText;
+ if (UseConditionExprText) {
+ const StringRef ConditionExprText =
+ tooling::fixit::getText(*If->getCond(), Context);
+ if (ConditionExprText.empty())
+ return std::nullopt;
+ ConditionText = ConditionExprText.str();
+ } else {
+ ConditionText = getIfConditionText(If, SM, LangOpts);
+ if (!ConditionText)
+ return std::nullopt;
}
- return true;
+ return wrapConditionText(*ConditionText,
+ conditionNeedsBoolCast(If->getCond()));
}
static CombinedConditionBuildResult
buildCombinedCondition(llvm::ArrayRef<const IfStmt *> Chain,
const ASTContext &Context) {
- CombinedConditionBuildResult Result;
if (Chain.empty())
- return Result;
+ return {};
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::string CombinedCondition;
- 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;
+ for (const auto &[Index, If] : llvm::enumerate(Chain)) {
+ const bool IsRoot = Index == 0;
+ if (!IsRoot && !hasOnlyPayloadCommentsInNestedHeader(If, SM, LangOpts)) {
+ return {CombinedConditionBuildStatus::UnsupportedCommentPlacement, {}};
}
- // 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 (IsRoot && If->hasVarStorage()) {
+ const auto *const ConditionVariable = If->getConditionVariable();
+ if (!ConditionVariable)
+ return {};
+
+ const std::optional<std::string> ConditionText =
+ getIfConditionText(If, SM, LangOpts);
+ if (!ConditionText)
+ return {};
+
+ CombinedCondition = *ConditionText;
+ CombinedCondition += "; ";
+ CombinedCondition += wrapConditionText(
+ ConditionVariable->getName(), conditionNeedsBoolCast(If->getCond()));
+ continue;
}
- if (!Combined.empty())
- Combined += " && ";
- // Parenthesize each condition to preserve precedence in the rewritten
- // condition regardless of original operator binding.
- Combined += "(";
- Combined += *CondText;
- Combined += ")";
+ const std::optional<std::string> ConjunctText =
+ getConjunctText(If, Context,
+ /*UseConditionExprText=*/IsRoot &&
+ If->hasInitStorage() && !If->hasVarStorage());
+ if (!ConjunctText)
+ return {};
+
+ if (!CombinedCondition.empty())
+ CombinedCondition += " && ";
+ CombinedCondition += '(';
+ CombinedCondition += *ConjunctText;
+ CombinedCondition += ')';
}
- Result.Status = CombinedConditionBuildStatus::Success;
- Result.Text = std::move(Combined);
- return Result;
+
+ return {CombinedConditionBuildStatus::Success, std::move(CombinedCondition)};
}
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);
+ assert(If);
+
+ return If->hasInitStorage() && !If->hasVarStorage()
+ ? std::optional<CharSourceRange>(CharSourceRange::getTokenRange(
+ If->getCond()->getSourceRange()))
+ : getIfConditionRange(If, SM, LangOpts);
}
static ChainHandling
@@ -634,12 +505,9 @@ 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))
+ if (Chain.size() < 2 || !isFixitSafeForChain(Chain, SM, LangOpts))
return ChainHandling::None;
+
if (!isConstexprChainSemanticallySafe(Chain, Context)) {
return WarnOnDependentConstexprIf
? ChainHandling::WarnOnlyDependentConstexpr
@@ -653,132 +521,122 @@ getChainHandling(llvm::ArrayRef<const IfStmt *> Chain,
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;
+static void emitNestedIfNotes(RedundantNestedIfCheck &Check,
+ llvm::ArrayRef<const IfStmt *> Chain) {
+ for (const IfStmt *Nested : llvm::drop_begin(Chain))
+ Check.diag(Nested->getIfLoc(), NestedIfNote, DiagnosticIDs::Note);
+}
- return getChainHandling(ParentChain, Context, SM, LangOpts,
- WarnOnDependentConstexprIf,
- /*CombinedCondition=*/nullptr) != ChainHandling::None;
+static void diagnoseChain(RedundantNestedIfCheck &Check, const IfStmt *If,
+ ASTContext &Context, bool WarnOnDependentConstexprIf,
+ bool AllowUserDefinedBoolConversion);
+
+static void diagnoseChildChain(RedundantNestedIfCheck &Check,
+ const Stmt *Branch, ASTContext &Context,
+ bool WarnOnDependentConstexprIf,
+ bool AllowUserDefinedBoolConversion) {
+ if (const IfStmt *const Nested = getOnlyNestedIf(Branch))
+ diagnoseChain(Check, Nested, Context, WarnOnDependentConstexprIf,
+ AllowUserDefinedBoolConversion);
}
-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;
+// Match only syntactic chain roots. If a root cannot be diagnosed because it is
+// unsafe to rewrite, descend into excluded single-child nested `if` statements
+// in both branches and try again there.
+static void diagnoseChain(RedundantNestedIfCheck &Check, const IfStmt *If,
+ ASTContext &Context, bool WarnOnDependentConstexprIf,
+ bool AllowUserDefinedBoolConversion) {
+ assert(If);
- const auto ParentChain = getUserDefinedBoolWarnChain(ParentIf, Context);
- if (ParentChain.size() < 2 || ParentChain[1] != If)
- return false;
- return isFixitSafeForChain(ParentChain, SM, LangOpts);
+ const SourceManager &SM = Context.getSourceManager();
+ const LangOptions &LangOpts = Context.getLangOpts();
+ const IfChain Chain =
+ getMergeChain(If, Context, AllowUserDefinedBoolConversion);
+
+ std::optional<std::string> CombinedCondition;
+ const ChainHandling Handling =
+ getChainHandling(Chain, Context, SM, LangOpts, WarnOnDependentConstexprIf,
+ &CombinedCondition);
+ if (Handling == ChainHandling::None) {
+ diagnoseChildChain(Check, If->getThen(), Context,
+ WarnOnDependentConstexprIf,
+ AllowUserDefinedBoolConversion);
+ diagnoseChildChain(Check, If->getElse(), Context,
+ WarnOnDependentConstexprIf,
+ AllowUserDefinedBoolConversion);
+ return;
+ }
+
+ if (Handling == ChainHandling::WarnOnlyDependentConstexpr) {
+ Check.diag(If->getIfLoc(), DependentConstexprDiag);
+ emitNestedIfNotes(Check, Chain);
+ return;
+ }
+
+ {
+ const DiagnosticBuilder Diag = Check.diag(If->getIfLoc(), MergeableIfDiag);
+ if (Handling == ChainHandling::WarnAndFix) {
+ const std::optional<CharSourceRange> ConditionRange =
+ getConditionReplacementRange(If, SM, LangOpts);
+ if (!ConditionRange || !CombinedCondition)
+ return;
+
+ Diag << FixItHint::CreateReplacement(*ConditionRange, *CombinedCondition);
+ const llvm::ArrayRef<const IfStmt *> ChainRef(Chain);
+ for (const auto &[Parent, Child] :
+ llvm::zip(ChainRef.drop_back(), ChainRef.drop_front())) {
+ Diag << FixItHint::CreateRemoval(CharSourceRange::getCharRange(
+ Child->getBeginLoc(), Child->getThen()->getBeginLoc()));
+
+ const auto *const Wrapper = dyn_cast<CompoundStmt>(Parent->getThen());
+ if (!Wrapper)
+ continue;
+
+ Diag << FixItHint::CreateRemoval(Wrapper->getLBracLoc())
+ << FixItHint::CreateRemoval(Wrapper->getRBracLoc());
+ }
+ }
+ }
+
+ emitNestedIfNotes(Check, Chain);
}
RedundantNestedIfCheck::RedundantNestedIfCheck(StringRef Name,
ClangTidyContext *Context)
: ClangTidyCheck(Name, Context), WarnOnDependentConstexprIf(Options.get(
WarnOnDependentConstexprIfStr, false)),
- UserBoolConversionMode(Options.get(UserDefinedBoolConversionModeStr,
- UserDefinedBoolConversionMode::None)) {
-}
+ AllowUserDefinedBoolConversion(
+ Options.get(AllowUserDefinedBoolConversionStr, false)) {}
void RedundantNestedIfCheck::storeOptions(ClangTidyOptions::OptionMap &Opts) {
Options.store(Opts, WarnOnDependentConstexprIfStr,
WarnOnDependentConstexprIf);
- Options.store(Opts, UserDefinedBoolConversionModeStr, UserBoolConversionMode);
+ Options.store(Opts, AllowUserDefinedBoolConversionStr,
+ AllowUserDefinedBoolConversion);
}
void RedundantNestedIfCheck::registerMatchers(MatchFinder *Finder) {
- Finder->addMatcher(ifStmt().bind("if"), this);
+ Finder->addMatcher(
+ ifStmt(unless(anyOf(hasParent(ifStmt()),
+ hasParent(compoundStmt(statementCountIs(1),
+ hasParent(ifStmt()))))))
+ .bind("if"),
+ this);
}
void RedundantNestedIfCheck::check(const MatchFinder::MatchResult &Result) {
- const auto *If = Result.Nodes.getNodeAs<IfStmt>("if");
- if (!If)
- return;
+ const auto *const If = Result.Nodes.getNodeAs<IfStmt>("if");
+ assert(If);
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());
- }
- }
+ diagnoseChain(*this, If, Context, WarnOnDependentConstexprIf,
+ AllowUserDefinedBoolConversion);
}
} // namespace clang::tidy::readability
diff --git a/clang-tools-extra/clang-tidy/readability/RedundantNestedIfCheck.h b/clang-tools-extra/clang-tidy/readability/RedundantNestedIfCheck.h
index 4e85017a70107..c8cabf2972e51 100644
--- a/clang-tools-extra/clang-tidy/readability/RedundantNestedIfCheck.h
+++ b/clang-tools-extra/clang-tidy/readability/RedundantNestedIfCheck.h
@@ -17,12 +17,6 @@ namespace clang::tidy::readability {
/// 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;
@@ -31,7 +25,7 @@ class RedundantNestedIfCheck : public ClangTidyCheck {
private:
const bool WarnOnDependentConstexprIf;
- const UserDefinedBoolConversionMode UserBoolConversionMode;
+ const bool AllowUserDefinedBoolConversion;
};
} // namespace clang::tidy::readability
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
index 51c502114b55b..225098f2470d1 100644
--- 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
@@ -3,8 +3,8 @@
readability-redundant-nested-if
===============================
-Finds nested ``if`` statements that can be merged by combining their conditions
-with ``&&``.
+Finds nested ``if`` statements that can be merged by combining their
+conditions with ``&&``.
Example:
@@ -48,7 +48,7 @@ The check also supports outer declaration conditions in C++17 and later:
.. code-block:: c++
- if (bool x = ready()) {
+ if (bool X = ready()) {
if (can_run()) {
work();
}
@@ -58,7 +58,7 @@ becomes
.. code-block:: c++
- if (bool x = ready(); x && (can_run())) {
+ if (bool X = ready(); X && (can_run())) {
work();
}
@@ -73,14 +73,14 @@ The check only transforms chains where:
- 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 && ...)``.
+ ``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
- :option:`UserDefinedBoolConversionMode`.
+ preserve built-in ``&&`` semantics. This can be changed with
+ :option:`AllowUserDefinedBoolConversion`.
- Only the outermost ``if`` may have an init-statement.
@@ -91,11 +91,11 @@ The check only transforms chains where:
- No merged ``if`` statement has statement attributes.
-- All rewritten ranges are free of macro/preprocessor-sensitive edits.
+- All rewritten ranges are free of macro- and 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.
+ preserved safely. Comments inside conditions are preserved, while other
+ comments between the ``if`` statements disable fix-its.
For ``if constexpr``, nested merged conditions must be
non-instantiation-dependent to avoid template semantic changes. The outermost
@@ -105,24 +105,20 @@ constant ``true``.
Options
-------
-.. option:: UserDefinedBoolConversionMode
+.. option:: AllowUserDefinedBoolConversion
- Controls how chains with an outer condition that relies on user-defined
- ``bool`` conversion are handled.
+ When set to `true`, the check also merges chains whose conditions rely on
+ user-defined conversion to ``bool``.
- - `None`
- No diagnostic is emitted for those chains.
- - `WarnOnly`
- Emit diagnostics, but do not provide fix-its.
- - `WarnAndFix`
- Emit diagnostics and provide fix-its.
+ The fix-it inserts ``static_cast<bool>(...)`` where needed so the merged
+ condition still uses built-in ``&&`` semantics.
- Default is `None`.
+ Default is `false`.
.. 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.
+ ``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
index 7c26883395718..df4d74b84427d 100644
--- 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
@@ -2,26 +2,29 @@
// 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
+// RUN: %check_clang_tidy -check-suffixes=BASE,CXX17,CXX20,CXX23,CXX26 -std=c++26-or-later %s readability-redundant-nested-if %t -- -- -fno-delayed-template-parsing
+// RUN: %check_clang_tidy -check-suffixes=BASE,CXX17,CXX20,CXX23,CXX26,DEPWARN -std=c++26-or-later %s readability-redundant-nested-if %t -- -config='{CheckOptions: {readability-redundant-nested-if.WarnOnDependentConstexprIf: true}}' -- -fno-delayed-template-parsing
+// RUN: %check_clang_tidy -check-suffixes=BASE,CXX17,CXX20,CXX23,CXX26,ALLOWBOOL -std=c++26-or-later %s readability-redundant-nested-if %t -- -config='{CheckOptions: {readability-redundant-nested-if.AllowUserDefinedBoolConversion: true}}' -- -fno-delayed-template-parsing
bool cond(int X = 0);
+int side_effect();
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()) {
+ // CHECK-MESSAGES-BASE: :[[@LINE+1]]:5: note: nested if statement to merge is here
if (cond(1)) {
sink();
}
@@ -29,22 +32,25 @@ void positive_cases() {
// 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)) {
+ // CHECK-MESSAGES-BASE: :[[@LINE+1]]:5: note: nested if statement to merge is here
+ if (cond(1)) {
+ // CHECK-MESSAGES-BASE: :[[@LINE+1]]:7: note: nested if statement to merge is here
+ 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() || cond(1))
+ // CHECK-MESSAGES-BASE: :[[@LINE+1]]:5: note: nested if statement to merge is here
+ if (cond(2))
+ sink();
+ // CHECK-FIXES-BASE: if ((cond() || cond(1)) && (cond(2)))
+ // CHECK-FIXES-BASE: sink();
}
void stress_long_chain_case() {
@@ -78,12 +84,14 @@ void stress_long_chain_case() {
}
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
+ // CHECK-MESSAGES-BASE: :[[@LINE+1]]:3: warning: nested if statements can be merged
if (cond()) {
+ // CHECK-MESSAGES-BASE: :[[@LINE+1]]:5: note: nested if statement to merge is here
if (cond(1)) {
sink();
+ // CHECK-MESSAGES-BASE: :[[@LINE+1]]:7: warning: nested if statements can be merged
if (cond(2)) {
+ // CHECK-MESSAGES-BASE: :[[@LINE+1]]:9: note: nested if statement to merge is here
if (cond(3))
sink();
}
@@ -93,10 +101,11 @@ void nested_chains_are_diagnosed_once_per_chain() {
// CHECK-FIXES-BASE: if ((cond(2)) && (cond(3)))
}
-void child_chain_is_reported_when_macro_parent_is_unfixable() {
+void child_chain_is_reported_when_parent_is_not_diagnosable() {
// CHECK-MESSAGES-BASE: :[[@LINE+2]]:5: warning: nested if statements can be merged
OUTER_IF {
if (cond(1)) {
+ // CHECK-MESSAGES-BASE: :[[@LINE+1]]:7: note: nested if statement to merge is here
if (cond(2))
sink();
}
@@ -106,16 +115,23 @@ void child_chain_is_reported_when_macro_parent_is_unfixable() {
// 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();
+void else_branch_child_chain_is_reported_when_parent_is_not_diagnosable() {
+ // CHECK-MESSAGES-BASE: :[[@LINE+4]]:5: warning: nested if statements can be merged
+ if (cond()) {
+ sink();
+ } else {
+ if (cond(1)) {
+ // CHECK-MESSAGES-BASE: :[[@LINE+1]]:7: note: nested if statement to merge is here
+ if (cond(2))
+ sink();
+ }
}
- // CHECK-FIXES-CXX17: if (bool B = cond(); B && (cond(1)))
- // CHECK-FIXES-CXX17: sink();
+ // CHECK-FIXES-BASE: } else {
+ // 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
if (cond()) {
if (bool B = cond(1))
@@ -151,20 +167,6 @@ void negative_cases() {
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() {
@@ -192,37 +194,59 @@ 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 */) {
+ // CHECK-MESSAGES-BASE: :[[@LINE+1]]:5: note: nested if statement to merge is here
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).
+ // Trailing comments in nested headers keep the diagnostic but suppress fix-its.
// CHECK-MESSAGES-BASE: :[[@LINE+1]]:3: warning: nested if statements can be merged
if (cond()) // outer trailing
+ // CHECK-MESSAGES-BASE: :[[@LINE+1]]:5: note: nested if statement to merge is here
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();
+ // CHECK-FIXES-BASE: if (cond(1)) // inner trailing
+ // CHECK-FIXES-BASE: 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()) {
+ // CHECK-MESSAGES-BASE: :[[@LINE+1]]:5: note: nested if statement to merge is here
if /* nested header comment */ (cond(1))
sink();
}
// CHECK-FIXES-BASE: if /* nested header comment */ (cond(1))
}
-#if __cplusplus >= 201703L
-int side_effect();
+void user_defined_bool_conversion_cases() {
+ // CHECK-MESSAGES-BASE-NOT: :[[@LINE+3]]:3: warning: nested if statements can be merged
+ // CHECK-MESSAGES-ALLOWBOOL: :[[@LINE+2]]:3: warning: nested if statements can be merged
+ // CHECK-MESSAGES-ALLOWBOOL: :[[@LINE+2]]:5: note: nested if statement to merge is here
+ if (make_bool_like()) {
+ if (cond(1))
+ sink();
+ }
+ // CHECK-FIXES-ALLOWBOOL: if ((static_cast<bool>(make_bool_like())) && (cond(1)))
+ // CHECK-FIXES-ALLOWBOOL: sink();
-// C++17 language feature coverage.
+ // CHECK-MESSAGES-BASE-NOT: :[[@LINE+3]]:3: warning: nested if statements can be merged
+ // CHECK-MESSAGES-ALLOWBOOL: :[[@LINE+2]]:3: warning: nested if statements can be merged
+ // CHECK-MESSAGES-ALLOWBOOL: :[[@LINE+2]]:5: note: nested if statement to merge is here
+ if (cond(1)) {
+ if (make_bool_like())
+ sink();
+ }
+ // CHECK-FIXES-ALLOWBOOL: if ((cond(1)) && (static_cast<bool>(make_bool_like())))
+ // CHECK-FIXES-ALLOWBOOL: sink();
+}
+
+#if __cplusplus >= 201703L
void init_statement_cases() {
// CHECK-MESSAGES-CXX17: :[[@LINE+1]]:3: warning: nested if statements can be merged
if (side_effect(); cond()) {
+ // CHECK-MESSAGES-CXX17: :[[@LINE+1]]:5: note: nested if statement to merge is here
if (cond(1))
sink();
}
@@ -230,30 +254,19 @@ void init_statement_cases() {
// CHECK-FIXES-CXX17: sink();
// CHECK-MESSAGES-CXX17: :[[@LINE+1]]:3: warning: nested if statements can be merged
- if (auto Guard = side_effect()) {
+ if (bool B = cond()) {
+ // CHECK-MESSAGES-CXX17: :[[@LINE+1]]:5: note: nested if statement to merge is here
if (cond(1))
sink();
}
- // CHECK-FIXES-CXX17: if (auto Guard = side_effect(); Guard && (cond(1)))
+ // CHECK-FIXES-CXX17: if (bool B = cond(); B && (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) {
+ // CHECK-MESSAGES-CXX17: :[[@LINE+1]]:5: note: nested if statement to merge is here
if (cond()) {
+ // CHECK-MESSAGES-CXX17: :[[@LINE+1]]:7: note: nested if statement to merge is here
if (cond(1))
bar();
}
@@ -263,43 +276,30 @@ void init_statement_cases() {
// CHECK-MESSAGES-CXX17: :[[@LINE+1]]:3: warning: nested if statements can be merged
if (bool X = cond() /* here */) {
- if (cond()) {
+ // CHECK-MESSAGES-CXX17: :[[@LINE+1]]:5: note: nested if statement to merge is here
+ if (cond())
bar();
- }
}
// CHECK-FIXES-CXX17: if (bool X = cond() /* here */; X && (cond()))
// CHECK-FIXES-CXX17: bar();
}
-void declaration_condition_type_safety() {
+void declaration_condition_boollike_cases() {
// 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
+ // CHECK-MESSAGES-ALLOWBOOL: :[[@LINE+2]]:3: warning: nested if statements can be merged
+ // CHECK-MESSAGES-ALLOWBOOL: :[[@LINE+2]]:5: note: nested if statement to merge is here
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-FIXES-ALLOWBOOL: if (auto Guard = make_bool_like(); static_cast<bool>(Guard) && (cond(1)))
+ // CHECK-FIXES-ALLOWBOOL: 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;
@@ -308,9 +308,9 @@ 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) {
+ // CHECK-MESSAGES-CXX17: :[[@LINE+1]]:5: note: nested if statement to merge is here
+ if constexpr (C2)
sink();
- }
}
// CHECK-FIXES-CXX17: if constexpr ((C1) && (C2))
// CHECK-FIXES-CXX17: sink();
@@ -319,6 +319,7 @@ void constexpr_non_template() {
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) {
+ // CHECK-MESSAGES-CXX17: :[[@LINE+1]]:5: note: nested if statement to merge is here
if constexpr (true)
sink();
}
@@ -327,18 +328,18 @@ template <bool B> void dependent_constexpr_outer_is_fixable() {
}
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
+ // CHECK-MESSAGES-DEPWARN: :[[@LINE+1]]:3: warning: nested instantiation-dependent if constexpr statements can be merged
if constexpr (B) {
+ // CHECK-MESSAGES-DEPWARN: :[[@LINE+1]]:5: note: nested if statement to merge is here
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
+ // CHECK-MESSAGES-DEPWARN: :[[@LINE+1]]:3: warning: nested instantiation-dependent if constexpr statements can be merged
if constexpr (true) {
+ // CHECK-MESSAGES-DEPWARN: :[[@LINE+1]]:5: note: nested if statement to merge is here
if constexpr (sizeof(T) == 4)
sink();
}
@@ -347,6 +348,7 @@ template <typename T> void dependent_constexpr_operand_warn_only_under_option()
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) {
+ // CHECK-MESSAGES-CXX17: :[[@LINE+1]]:5: note: nested if statement to merge is here
if constexpr (true)
sink();
}
@@ -371,8 +373,9 @@ void mixed_constexpr_and_non_constexpr(bool B) {
#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
+ // CHECK-MESSAGES-DEPWARN: :[[@LINE+1]]:3: warning: nested instantiation-dependent if constexpr statements can be merged
if constexpr (sizeof(T) == 1) {
+ // CHECK-MESSAGES-DEPWARN: :[[@LINE+1]]:5: note: nested if statement to merge is here
if constexpr (false) {
static_assert(false, "discarded in template context");
}
@@ -386,6 +389,7 @@ template <typename T> void constexpr_template_static_assert() {
void constexpr_requires_expression_cases() {
// CHECK-MESSAGES-CXX20: :[[@LINE+1]]:3: warning: nested if statements can be merged
if constexpr (requires { sizeof(int); }) {
+ // CHECK-MESSAGES-CXX20: :[[@LINE+1]]:5: note: nested if statement to merge is here
if constexpr (requires { sizeof(long); })
sink();
}
@@ -396,6 +400,7 @@ void constexpr_requires_expression_cases() {
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) {
+ // CHECK-MESSAGES-CXX20: :[[@LINE+1]]:5: note: nested if statement to merge is here
if constexpr (requires { sizeof(long); })
sink();
}
@@ -406,6 +411,7 @@ void constexpr_init_statement_cases() {
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; }) {
+ // CHECK-MESSAGES-CXX20: :[[@LINE+1]]:5: note: nested if statement to merge is here
if constexpr (true)
sink();
}
@@ -426,11 +432,10 @@ void attribute_cases(bool B1, bool B2) {
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) {
+ // CHECK-MESSAGES-CXX20: :[[@LINE+1]]:5: note: nested if statement to merge is here
if (B2)
sink();
}
@@ -440,8 +445,6 @@ void still_merges_without_attributes(bool B1, bool B2) {
#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 {
@@ -460,7 +463,6 @@ void consteval_cases(bool B1, bool B2) {
sink();
}
}
-
}
template <bool B> constexpr void consteval_nested_in_constexpr() {
>From 31674b702af1cba8caea53422daf71c5b3700823 Mon Sep 17 00:00:00 2001
From: Daniil Dudkin <unterumarmung at yandex.ru>
Date: Sat, 4 Apr 2026 00:05:22 +0300
Subject: [PATCH 5/8] Fix format
---
.../clang-tidy/readability/RedundantNestedIfCheck.cpp | 3 +--
1 file changed, 1 insertion(+), 2 deletions(-)
diff --git a/clang-tools-extra/clang-tidy/readability/RedundantNestedIfCheck.cpp b/clang-tools-extra/clang-tidy/readability/RedundantNestedIfCheck.cpp
index 371c5cf31b779..57e31df0f530a 100644
--- a/clang-tools-extra/clang-tidy/readability/RedundantNestedIfCheck.cpp
+++ b/clang-tools-extra/clang-tidy/readability/RedundantNestedIfCheck.cpp
@@ -451,9 +451,8 @@ buildCombinedCondition(llvm::ArrayRef<const IfStmt *> Chain,
for (const auto &[Index, If] : llvm::enumerate(Chain)) {
const bool IsRoot = Index == 0;
- if (!IsRoot && !hasOnlyPayloadCommentsInNestedHeader(If, SM, LangOpts)) {
+ if (!IsRoot && !hasOnlyPayloadCommentsInNestedHeader(If, SM, LangOpts))
return {CombinedConditionBuildStatus::UnsupportedCommentPlacement, {}};
- }
if (IsRoot && If->hasVarStorage()) {
const auto *const ConditionVariable = If->getConditionVariable();
>From 0254ebd17d71e35a4646a31795b090998d2d9d25 Mon Sep 17 00:00:00 2001
From: Daniil Dudkin <unterumarmung at yandex.ru>
Date: Thu, 7 May 2026 00:02:04 +0300
Subject: [PATCH 6/8] fix review comments
---
.../readability/ReadabilityTidyModule.cpp | 4 +-
.../readability/RedundantNestedIfCheck.cpp | 37 ++++++++-----------
.../readability/RedundantNestedIfCheck.h | 6 +++
.../readability/redundant-nested-if.rst | 4 +-
.../readability/redundant-nested-if.cpp | 12 +++++-
5 files changed, 35 insertions(+), 28 deletions(-)
diff --git a/clang-tools-extra/clang-tidy/readability/ReadabilityTidyModule.cpp b/clang-tools-extra/clang-tidy/readability/ReadabilityTidyModule.cpp
index f202453559d99..e3ca779fd12ad 100644
--- a/clang-tools-extra/clang-tidy/readability/ReadabilityTidyModule.cpp
+++ b/clang-tools-extra/clang-tidy/readability/ReadabilityTidyModule.cpp
@@ -149,6 +149,8 @@ class ReadabilityModule : public ClangTidyModule {
"readability-redundant-lambda-parameter-list");
CheckFactories.registerCheck<RedundantMemberInitCheck>(
"readability-redundant-member-init");
+ CheckFactories.registerCheck<RedundantNestedIfCheck>(
+ "readability-redundant-nested-if");
CheckFactories.registerCheck<RedundantParenthesesCheck>(
"readability-redundant-parentheses");
CheckFactories.registerCheck<RedundantPreprocessorCheck>(
@@ -181,8 +183,6 @@ 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
index 57e31df0f530a..91ad3e9ea632b 100644
--- a/clang-tools-extra/clang-tidy/readability/RedundantNestedIfCheck.cpp
+++ b/clang-tools-extra/clang-tidy/readability/RedundantNestedIfCheck.cpp
@@ -189,6 +189,10 @@ canRewriteOuterConditionVariable(const IfStmt *If,
assert(If);
assert(If->hasVarStorage());
+ // `if (init; bool X = cond())` cannot generally become
+ // `if (init; bool X = cond(); X)`.
+ // Same-type declaration merging, like `if (bool Y = init(), X = cond(); X)`,
+ // is possible but too narrow to be worth supporting.
if (If->hasInitStorage())
return false;
@@ -219,22 +223,17 @@ static const IfStmt *getOnlyNestedIf(const Stmt *Then) {
return dyn_cast<IfStmt>(Compound->body_front());
}
-static bool hasMergeableStructure(const IfStmt *If, bool AllowInitStorage,
- bool RequireConstexpr) {
- assert(If);
-
- return If->getThen() && !If->isConsteval() && !If->getElse() &&
- (AllowInitStorage || !If->hasInitStorage()) &&
- If->isConstexpr() == RequireConstexpr;
-}
-
static bool isMergeCandidate(const IfStmt *If, bool AllowInitStorage,
bool RequireConstexpr, bool AllowConditionVariable,
bool AllowUserDefinedBoolConversion,
const LangOptions &LangOpts) {
assert(If);
- if (!hasMergeableStructure(If, AllowInitStorage, RequireConstexpr))
+ const bool IsMergeableStructure =
+ If->getThen() && !If->isConsteval() && !If->getElse() &&
+ (AllowInitStorage || !If->hasInitStorage()) &&
+ If->isConstexpr() == RequireConstexpr;
+ if (!IsMergeableStructure)
return false;
if (If->hasVarStorage())
@@ -256,8 +255,6 @@ static bool isAttributedIf(const IfStmt *If, ASTContext &Context) {
static IfChain getMergeChain(const IfStmt *Root, ASTContext &Context,
bool AllowUserDefinedBoolConversion) {
- assert(Root);
-
IfChain Chain;
const LangOptions &LangOpts = Context.getLangOpts();
const bool RequireConstexpr = Root->isConstexpr();
@@ -265,9 +262,8 @@ static IfChain getMergeChain(const IfStmt *Root, ASTContext &Context,
/*RequireConstexpr=*/RequireConstexpr,
/*AllowConditionVariable=*/true,
AllowUserDefinedBoolConversion, LangOpts) ||
- isAttributedIf(Root, Context)) {
+ isAttributedIf(Root, Context))
return Chain;
- }
Chain.push_back(Root);
const IfStmt *Current = Root;
@@ -276,9 +272,8 @@ static IfChain getMergeChain(const IfStmt *Root, ASTContext &Context,
/*RequireConstexpr=*/RequireConstexpr,
/*AllowConditionVariable=*/false,
AllowUserDefinedBoolConversion, LangOpts) ||
- isAttributedIf(Nested, Context)) {
+ isAttributedIf(Nested, Context))
break;
- }
Chain.push_back(Nested);
Current = Nested;
@@ -551,8 +546,6 @@ static void diagnoseChildChain(RedundantNestedIfCheck &Check,
static void diagnoseChain(RedundantNestedIfCheck &Check, const IfStmt *If,
ASTContext &Context, bool WarnOnDependentConstexprIf,
bool AllowUserDefinedBoolConversion) {
- assert(If);
-
const SourceManager &SM = Context.getSourceManager();
const LangOptions &LangOpts = Context.getLangOpts();
const IfChain Chain =
@@ -622,9 +615,11 @@ void RedundantNestedIfCheck::storeOptions(ClangTidyOptions::OptionMap &Opts) {
void RedundantNestedIfCheck::registerMatchers(MatchFinder *Finder) {
Finder->addMatcher(
- ifStmt(unless(anyOf(hasParent(ifStmt()),
- hasParent(compoundStmt(statementCountIs(1),
- hasParent(ifStmt()))))))
+ ifStmt(unless(hasElse(stmt())),
+ unless(anyOf(hasParent(ifStmt(unless(hasElse(stmt())))),
+ hasParent(compoundStmt(
+ statementCountIs(1),
+ hasParent(ifStmt(unless(hasElse(stmt())))))))))
.bind("if"),
this);
}
diff --git a/clang-tools-extra/clang-tidy/readability/RedundantNestedIfCheck.h b/clang-tools-extra/clang-tidy/readability/RedundantNestedIfCheck.h
index c8cabf2972e51..ac6e53afc089b 100644
--- a/clang-tools-extra/clang-tidy/readability/RedundantNestedIfCheck.h
+++ b/clang-tools-extra/clang-tidy/readability/RedundantNestedIfCheck.h
@@ -15,6 +15,9 @@ namespace clang::tidy::readability {
/// Detects nested ``if`` statements that can be merged into one by
/// concatenating conditions with ``&&``.
+///
+/// For the user-facing documentation see:
+/// https://clang.llvm.org/extra/clang-tidy/checks/readability/redundant-nested-if.html
class RedundantNestedIfCheck : public ClangTidyCheck {
public:
RedundantNestedIfCheck(StringRef Name, ClangTidyContext *Context);
@@ -22,6 +25,9 @@ class RedundantNestedIfCheck : public ClangTidyCheck {
void storeOptions(ClangTidyOptions::OptionMap &Opts) override;
void registerMatchers(ast_matchers::MatchFinder *Finder) override;
void check(const ast_matchers::MatchFinder::MatchResult &Result) override;
+ std::optional<TraversalKind> getCheckTraversalKind() const override {
+ return TK_IgnoreUnlessSpelledInSource;
+ }
private:
const bool WarnOnDependentConstexprIf;
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
index 225098f2470d1..e7dab860ac338 100644
--- 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
@@ -119,6 +119,4 @@ Options
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`.
+ 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
index df4d74b84427d..d0552419f5877 100644
--- 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
@@ -1,4 +1,10 @@
-// RUN: %check_clang_tidy -check-suffixes=BASE -std=c++11 %s readability-redundant-nested-if %t -- -- -fno-delayed-template-parsing
+// Use exact standards because this file has version-specific FileCheck
+// suffixes. `check_clang_tidy.py` expands `-std=c++17-or-later` into separate
+// C++17, C++20, ... runs, but reuses the same suffix list for each run. A
+// suffix list for one language mode would either miss warnings from later
+// `#if __cplusplus` blocks, or look for CHECK lines from blocks preprocessed
+// away in earlier modes.
+// RUN: %check_clang_tidy -check-suffixes=BASE -std=c++98,c++11,c++14 %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
@@ -12,7 +18,7 @@ void sink();
void bar();
struct BoolLike {
- explicit operator bool() const;
+ operator bool() const;
};
BoolLike make_bool_like();
@@ -328,6 +334,7 @@ template <bool B> void dependent_constexpr_outer_is_fixable() {
}
template <bool B> void dependent_constexpr_outer_is_unsafe_when_nested_false() {
+ // CHECK-MESSAGES-CXX17-NOT: :[[@LINE+2]]:3: warning: nested if statements can be merged
// CHECK-MESSAGES-DEPWARN: :[[@LINE+1]]:3: warning: nested instantiation-dependent if constexpr statements can be merged
if constexpr (B) {
// CHECK-MESSAGES-DEPWARN: :[[@LINE+1]]:5: note: nested if statement to merge is here
@@ -337,6 +344,7 @@ template <bool B> void dependent_constexpr_outer_is_unsafe_when_nested_false() {
}
template <typename T> void dependent_constexpr_operand_warn_only_under_option() {
+ // CHECK-MESSAGES-CXX17-NOT: :[[@LINE+2]]:3: warning: nested if statements can be merged
// CHECK-MESSAGES-DEPWARN: :[[@LINE+1]]:3: warning: nested instantiation-dependent if constexpr statements can be merged
if constexpr (true) {
// CHECK-MESSAGES-DEPWARN: :[[@LINE+1]]:5: note: nested if statement to merge is here
>From 6775d2afdc72ff57deca9fa653f5ce3781f7475e Mon Sep 17 00:00:00 2001
From: Daniil Dudkin <unterumarmung at yandex.ru>
Date: Thu, 7 May 2026 00:10:49 +0300
Subject: [PATCH 7/8] nuke WarnOnDependentConstexprIf
---
.../readability/RedundantNestedIfCheck.cpp | 65 ++++++-------------
.../readability/RedundantNestedIfCheck.h | 1 -
.../readability/redundant-nested-if.rst | 14 ++--
.../readability/redundant-nested-if.cpp | 35 ++++++----
4 files changed, 47 insertions(+), 68 deletions(-)
diff --git a/clang-tools-extra/clang-tidy/readability/RedundantNestedIfCheck.cpp b/clang-tools-extra/clang-tidy/readability/RedundantNestedIfCheck.cpp
index 91ad3e9ea632b..b9ad1d678072d 100644
--- a/clang-tools-extra/clang-tidy/readability/RedundantNestedIfCheck.cpp
+++ b/clang-tools-extra/clang-tidy/readability/RedundantNestedIfCheck.cpp
@@ -26,14 +26,10 @@ using namespace clang::ast_matchers;
namespace clang::tidy::readability {
-static constexpr llvm::StringLiteral WarnOnDependentConstexprIfStr =
- "WarnOnDependentConstexprIf";
static constexpr llvm::StringLiteral AllowUserDefinedBoolConversionStr =
"AllowUserDefinedBoolConversion";
static constexpr llvm::StringLiteral MergeableIfDiag =
"nested if statements can be merged";
-static constexpr llvm::StringLiteral DependentConstexprDiag =
- "nested instantiation-dependent if constexpr statements can be merged";
static constexpr llvm::StringLiteral NestedIfNote =
"nested if statement to merge is here";
@@ -44,7 +40,6 @@ using IfChain = llvm::SmallVector<const IfStmt *>;
enum class ChainHandling {
None,
WarnOnly,
- WarnOnlyDependentConstexpr,
WarnAndFix,
};
@@ -189,7 +184,7 @@ canRewriteOuterConditionVariable(const IfStmt *If,
assert(If);
assert(If->hasVarStorage());
- // `if (init; bool X = cond())` cannot generally become
+ // `if (init; bool X = cond())` cannot generally become
// `if (init; bool X = cond(); X)`.
// Same-type declaration merging, like `if (bool Y = init(), X = cond(); X)`,
// is possible but too narrow to be worth supporting.
@@ -300,19 +295,17 @@ isConstexprChainSemanticallySafe(llvm::ArrayRef<const IfStmt *> Chain,
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 discarded-branch
- // behavior for template-dependent `if constexpr`.
- return llvm::all_of(llvm::drop_begin(Chain), [&](const IfStmt *Nested) {
- const Expr *const NestedCondition = Nested->getCond();
- if (NestedCondition->isInstantiationDependent())
+ bool PreviousConditionsAreConstantTrue = true;
+ return llvm::all_of(Chain, [&](const IfStmt *If) {
+ const Expr *const Condition = If->getCond();
+ if (Condition->isInstantiationDependent() &&
+ !PreviousConditionsAreConstantTrue)
return false;
- return !OuterIsDependent ||
- isConstantBooleanCondition(NestedCondition, Context,
- /*RequiredValue=*/true);
+
+ PreviousConditionsAreConstantTrue =
+ PreviousConditionsAreConstantTrue &&
+ isConstantBooleanCondition(Condition, Context, /*RequiredValue=*/true);
+ return true;
});
}
@@ -497,16 +490,13 @@ getConditionReplacementRange(const IfStmt *If, const SourceManager &SM,
static ChainHandling
getChainHandling(llvm::ArrayRef<const IfStmt *> Chain,
const ASTContext &Context, const SourceManager &SM,
- const LangOptions &LangOpts, bool WarnOnDependentConstexprIf,
+ const LangOptions &LangOpts,
std::optional<std::string> *CombinedCondition) {
if (Chain.size() < 2 || !isFixitSafeForChain(Chain, SM, LangOpts))
return ChainHandling::None;
- if (!isConstexprChainSemanticallySafe(Chain, Context)) {
- return WarnOnDependentConstexprIf
- ? ChainHandling::WarnOnlyDependentConstexpr
- : ChainHandling::None;
- }
+ if (!isConstexprChainSemanticallySafe(Chain, Context))
+ return ChainHandling::None;
const CombinedConditionBuildResult Combined =
buildCombinedCondition(Chain, Context);
@@ -528,23 +518,21 @@ static void emitNestedIfNotes(RedundantNestedIfCheck &Check,
}
static void diagnoseChain(RedundantNestedIfCheck &Check, const IfStmt *If,
- ASTContext &Context, bool WarnOnDependentConstexprIf,
+ ASTContext &Context,
bool AllowUserDefinedBoolConversion);
static void diagnoseChildChain(RedundantNestedIfCheck &Check,
const Stmt *Branch, ASTContext &Context,
- bool WarnOnDependentConstexprIf,
bool AllowUserDefinedBoolConversion) {
if (const IfStmt *const Nested = getOnlyNestedIf(Branch))
- diagnoseChain(Check, Nested, Context, WarnOnDependentConstexprIf,
- AllowUserDefinedBoolConversion);
+ diagnoseChain(Check, Nested, Context, AllowUserDefinedBoolConversion);
}
// Match only syntactic chain roots. If a root cannot be diagnosed because it is
// unsafe to rewrite, descend into excluded single-child nested `if` statements
// in both branches and try again there.
static void diagnoseChain(RedundantNestedIfCheck &Check, const IfStmt *If,
- ASTContext &Context, bool WarnOnDependentConstexprIf,
+ ASTContext &Context,
bool AllowUserDefinedBoolConversion) {
const SourceManager &SM = Context.getSourceManager();
const LangOptions &LangOpts = Context.getLangOpts();
@@ -553,24 +541,15 @@ static void diagnoseChain(RedundantNestedIfCheck &Check, const IfStmt *If,
std::optional<std::string> CombinedCondition;
const ChainHandling Handling =
- getChainHandling(Chain, Context, SM, LangOpts, WarnOnDependentConstexprIf,
- &CombinedCondition);
+ getChainHandling(Chain, Context, SM, LangOpts, &CombinedCondition);
if (Handling == ChainHandling::None) {
diagnoseChildChain(Check, If->getThen(), Context,
- WarnOnDependentConstexprIf,
AllowUserDefinedBoolConversion);
diagnoseChildChain(Check, If->getElse(), Context,
- WarnOnDependentConstexprIf,
AllowUserDefinedBoolConversion);
return;
}
- if (Handling == ChainHandling::WarnOnlyDependentConstexpr) {
- Check.diag(If->getIfLoc(), DependentConstexprDiag);
- emitNestedIfNotes(Check, Chain);
- return;
- }
-
{
const DiagnosticBuilder Diag = Check.diag(If->getIfLoc(), MergeableIfDiag);
if (Handling == ChainHandling::WarnAndFix) {
@@ -601,14 +580,11 @@ static void diagnoseChain(RedundantNestedIfCheck &Check, const IfStmt *If,
RedundantNestedIfCheck::RedundantNestedIfCheck(StringRef Name,
ClangTidyContext *Context)
- : ClangTidyCheck(Name, Context), WarnOnDependentConstexprIf(Options.get(
- WarnOnDependentConstexprIfStr, false)),
+ : ClangTidyCheck(Name, Context),
AllowUserDefinedBoolConversion(
Options.get(AllowUserDefinedBoolConversionStr, false)) {}
void RedundantNestedIfCheck::storeOptions(ClangTidyOptions::OptionMap &Opts) {
- Options.store(Opts, WarnOnDependentConstexprIfStr,
- WarnOnDependentConstexprIf);
Options.store(Opts, AllowUserDefinedBoolConversionStr,
AllowUserDefinedBoolConversion);
}
@@ -629,8 +605,7 @@ void RedundantNestedIfCheck::check(const MatchFinder::MatchResult &Result) {
assert(If);
ASTContext &Context = *Result.Context;
- diagnoseChain(*this, If, Context, WarnOnDependentConstexprIf,
- AllowUserDefinedBoolConversion);
+ diagnoseChain(*this, If, Context, AllowUserDefinedBoolConversion);
}
} // namespace clang::tidy::readability
diff --git a/clang-tools-extra/clang-tidy/readability/RedundantNestedIfCheck.h b/clang-tools-extra/clang-tidy/readability/RedundantNestedIfCheck.h
index ac6e53afc089b..c1219388a3dbe 100644
--- a/clang-tools-extra/clang-tidy/readability/RedundantNestedIfCheck.h
+++ b/clang-tools-extra/clang-tidy/readability/RedundantNestedIfCheck.h
@@ -30,7 +30,6 @@ class RedundantNestedIfCheck : public ClangTidyCheck {
}
private:
- const bool WarnOnDependentConstexprIf;
const bool AllowUserDefinedBoolConversion;
};
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
index e7dab860ac338..51f6517d01261 100644
--- 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
@@ -97,10 +97,10 @@ The check only transforms chains where:
preserved safely. Comments inside conditions are preserved, while other
comments between the ``if`` statements 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``.
+For ``if constexpr``, an instantiation-dependent condition is merged only when
+all preceding merged conditions are non-dependent constant ``true`` expressions.
+This preserves discarded-branch behavior for nested template-dependent
+``if constexpr`` statements.
Options
-------
@@ -114,9 +114,3 @@ Options
condition still uses built-in ``&&`` semantics.
Default is `false`.
-
-.. 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
index d0552419f5877..87ab471e2a231 100644
--- 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
@@ -9,7 +9,6 @@
// 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-or-later %s readability-redundant-nested-if %t -- -- -fno-delayed-template-parsing
-// RUN: %check_clang_tidy -check-suffixes=BASE,CXX17,CXX20,CXX23,CXX26,DEPWARN -std=c++26-or-later %s readability-redundant-nested-if %t -- -config='{CheckOptions: {readability-redundant-nested-if.WarnOnDependentConstexprIf: true}}' -- -fno-delayed-template-parsing
// RUN: %check_clang_tidy -check-suffixes=BASE,CXX17,CXX20,CXX23,CXX26,ALLOWBOOL -std=c++26-or-later %s readability-redundant-nested-if %t -- -config='{CheckOptions: {readability-redundant-nested-if.AllowUserDefinedBoolConversion: true}}' -- -fno-delayed-template-parsing
bool cond(int X = 0);
@@ -333,24 +332,35 @@ template <bool B> void dependent_constexpr_outer_is_fixable() {
// CHECK-FIXES-CXX17: sink();
}
-template <bool B> void dependent_constexpr_outer_is_unsafe_when_nested_false() {
- // CHECK-MESSAGES-CXX17-NOT: :[[@LINE+2]]:3: warning: nested if statements can be merged
- // CHECK-MESSAGES-DEPWARN: :[[@LINE+1]]:3: warning: nested instantiation-dependent if constexpr statements can be merged
+template <bool B> void dependent_constexpr_outer_with_nested_false_is_fixable() {
+ // CHECK-MESSAGES-CXX17: :[[@LINE+1]]:3: warning: nested if statements can be merged
if constexpr (B) {
- // CHECK-MESSAGES-DEPWARN: :[[@LINE+1]]:5: note: nested if statement to merge is here
+ // CHECK-MESSAGES-CXX17: :[[@LINE+1]]:5: note: nested if statement to merge is here
if constexpr (false)
sink();
}
+ // CHECK-FIXES-CXX17: if constexpr ((B) && (false))
+ // CHECK-FIXES-CXX17: sink();
}
-template <typename T> void dependent_constexpr_operand_warn_only_under_option() {
- // CHECK-MESSAGES-CXX17-NOT: :[[@LINE+2]]:3: warning: nested if statements can be merged
- // CHECK-MESSAGES-DEPWARN: :[[@LINE+1]]:3: warning: nested instantiation-dependent if constexpr statements can be merged
+template <typename T> void dependent_constexpr_operand_after_true_is_fixable() {
+ // CHECK-MESSAGES-CXX17: :[[@LINE+1]]:3: warning: nested if statements can be merged
if constexpr (true) {
- // CHECK-MESSAGES-DEPWARN: :[[@LINE+1]]:5: note: nested if statement to merge is here
+ // CHECK-MESSAGES-CXX17: :[[@LINE+1]]:5: note: nested if statement to merge is here
if constexpr (sizeof(T) == 4)
sink();
}
+ // CHECK-FIXES-CXX17: if constexpr ((true) && (sizeof(T) == 4))
+ // CHECK-FIXES-CXX17: sink();
+}
+
+template <bool B, typename T>
+void dependent_constexpr_operand_after_dependent_is_unsafe() {
+ // CHECK-MESSAGES-CXX17-NOT: :[[@LINE+1]]:3: warning: nested if statements can be merged
+ if constexpr (B) {
+ if constexpr (sizeof(typename T::type) == 4)
+ sink();
+ }
}
template <typename T> void dependent_constexpr_type_chain_outer_is_fixable() {
@@ -380,14 +390,15 @@ void mixed_constexpr_and_non_constexpr(bool B) {
#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-DEPWARN: :[[@LINE+1]]:3: warning: nested instantiation-dependent if constexpr statements can be merged
+ // CHECK-MESSAGES-CXX26: :[[@LINE+1]]:3: warning: nested if statements can be merged
if constexpr (sizeof(T) == 1) {
- // CHECK-MESSAGES-DEPWARN: :[[@LINE+1]]:5: note: nested if statement to merge is here
+ // CHECK-MESSAGES-CXX26: :[[@LINE+1]]:5: note: nested if statement to merge is here
if constexpr (false) {
static_assert(false, "discarded in template context");
}
}
+ // CHECK-FIXES-CXX26: if constexpr ((sizeof(T) == 1) && (false))
+ // CHECK-FIXES-CXX26: static_assert(false, "discarded in template context");
}
#endif
>From 4f8497e673b856b0598cbf36d3e3478de7193063 Mon Sep 17 00:00:00 2001
From: Daniil Dudkin <unterumarmung at yandex.ru>
Date: Sun, 10 May 2026 01:54:22 +0300
Subject: [PATCH 8/8] simplfy
---
.../readability/RedundantNestedIfCheck.cpp | 77 ++++++++++++++-----
.../readability/redundant-nested-if.rst | 75 +++---------------
.../readability/redundant-nested-if-notes.cpp | 25 ++++++
.../readability/redundant-nested-if.cpp | 22 ++++++
4 files changed, 112 insertions(+), 87 deletions(-)
create mode 100644 clang-tools-extra/test/clang-tidy/checkers/readability/redundant-nested-if-notes.cpp
diff --git a/clang-tools-extra/clang-tidy/readability/RedundantNestedIfCheck.cpp b/clang-tools-extra/clang-tidy/readability/RedundantNestedIfCheck.cpp
index b9ad1d678072d..6f2f40c30c2ff 100644
--- a/clang-tools-extra/clang-tidy/readability/RedundantNestedIfCheck.cpp
+++ b/clang-tools-extra/clang-tidy/readability/RedundantNestedIfCheck.cpp
@@ -9,6 +9,7 @@
#include "RedundantNestedIfCheck.h"
#include "../utils/LexerUtils.h"
#include "clang/AST/ASTContext.h"
+#include "clang/AST/DeclTemplate.h"
#include "clang/AST/Stmt.h"
#include "clang/ASTMatchers/ASTMatchFinder.h"
#include "clang/Basic/Diagnostic.h"
@@ -155,7 +156,6 @@ static bool hasOnlyPayloadCommentsInNestedHeader(const IfStmt *Nested,
const SourceManager &SM,
const LangOptions &LangOpts) {
assert(Nested);
- assert(Nested->getThen());
const CharSourceRange HeaderRange = CharSourceRange::getCharRange(
Nested->getBeginLoc(), Nested->getThen()->getBeginLoc());
@@ -224,19 +224,22 @@ static bool isMergeCandidate(const IfStmt *If, bool AllowInitStorage,
const LangOptions &LangOpts) {
assert(If);
- const bool IsMergeableStructure =
- If->getThen() && !If->isConsteval() && !If->getElse() &&
- (AllowInitStorage || !If->hasInitStorage()) &&
- If->isConstexpr() == RequireConstexpr;
- if (!IsMergeableStructure)
- return false;
-
- if (If->hasVarStorage())
- return AllowConditionVariable && LangOpts.CPlusPlus17 &&
- canRewriteOuterConditionVariable(If, AllowUserDefinedBoolConversion);
-
- return If->getCond() && isConditionExpressionMergeable(
- If->getCond(), AllowUserDefinedBoolConversion);
+ const bool HasAllowedInitStorage = AllowInitStorage || !If->hasInitStorage();
+ const bool HasRequiredConstexpr = If->isConstexpr() == RequireConstexpr;
+ const bool IsMergeableStructure = If->getThen() && !If->isConsteval() &&
+ !If->getElse() && HasAllowedInitStorage &&
+ HasRequiredConstexpr;
+
+ const bool HasMergeableConditionVariable =
+ If->hasVarStorage() && AllowConditionVariable && LangOpts.CPlusPlus17 &&
+ canRewriteOuterConditionVariable(If, AllowUserDefinedBoolConversion);
+ const bool HasMergeableConditionExpression =
+ !If->hasVarStorage() && If->getCond() &&
+ isConditionExpressionMergeable(If->getCond(),
+ AllowUserDefinedBoolConversion);
+
+ return IsMergeableStructure &&
+ (HasMergeableConditionVariable || HasMergeableConditionExpression);
}
// Statement attributes wrap the `if` in an `AttributedStmt`, so removing nested
@@ -250,6 +253,8 @@ static bool isAttributedIf(const IfStmt *If, ASTContext &Context) {
static IfChain getMergeChain(const IfStmt *Root, ASTContext &Context,
bool AllowUserDefinedBoolConversion) {
+ assert(Root);
+
IfChain Chain;
const LangOptions &LangOpts = Context.getLangOpts();
const bool RequireConstexpr = Root->isConstexpr();
@@ -289,24 +294,54 @@ static bool isConstantBooleanCondition(const Expr *Condition,
EvaluatedValue == RequiredValue;
}
+// Some instantiation-dependent conditions are safe to form even when an
+// earlier `if constexpr` condition is not known to be `true`. For example,
+// non-type template parameters and `requires` expressions do not depend on a
+// discarded branch to avoid hard substitution errors.
+static bool isAlwaysFormableDependentConstexprCondition(const Expr *Condition) {
+ assert(Condition);
+
+ Condition = Condition->IgnoreParenImpCasts();
+ if (isa<CXXBoolLiteralExpr, IntegerLiteral, RequiresExpr>(Condition))
+ return true;
+
+ if (const auto *DeclRef = dyn_cast<DeclRefExpr>(Condition))
+ return isa<NonTypeTemplateParmDecl>(DeclRef->getDecl());
+
+ if (const auto *Unary = dyn_cast<UnaryOperator>(Condition))
+ return isAlwaysFormableDependentConstexprCondition(Unary->getSubExpr());
+
+ if (const auto *Binary = dyn_cast<BinaryOperator>(Condition)) {
+ if (Binary->isAssignmentOp() || Binary->getOpcode() == BO_Comma)
+ return false;
+
+ return isAlwaysFormableDependentConstexprCondition(Binary->getLHS()) &&
+ isAlwaysFormableDependentConstexprCondition(Binary->getRHS());
+ }
+
+ return false;
+}
+
static bool
isConstexprChainSemanticallySafe(llvm::ArrayRef<const IfStmt *> Chain,
const ASTContext &Context) {
if (Chain.empty() || !Chain.front()->isConstexpr())
return true;
- bool PreviousConditionsAreConstantTrue = true;
- return llvm::all_of(Chain, [&](const IfStmt *If) {
+ bool AllPreviousConditionsAreConstantTrue = true;
+ for (const IfStmt *If : Chain) {
const Expr *const Condition = If->getCond();
if (Condition->isInstantiationDependent() &&
- !PreviousConditionsAreConstantTrue)
+ !AllPreviousConditionsAreConstantTrue &&
+ !isAlwaysFormableDependentConstexprCondition(Condition))
return false;
- PreviousConditionsAreConstantTrue =
- PreviousConditionsAreConstantTrue &&
+ AllPreviousConditionsAreConstantTrue =
+ AllPreviousConditionsAreConstantTrue &&
isConstantBooleanCondition(Condition, Context, /*RequiredValue=*/true);
- return true;
- });
+ }
+
+ return true;
}
// A range is unsafe for text edits if it crosses macro expansions or
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
index 51f6517d01261..d307f4f963988 100644
--- 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
@@ -24,26 +24,6 @@ becomes
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++
@@ -62,55 +42,18 @@ becomes
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 built-in ``&&`` semantics. This can be changed with
- :option:`AllowUserDefinedBoolConversion`.
-
-- 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- and 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 ``if`` statements disable fix-its.
-
-For ``if constexpr``, an instantiation-dependent condition is merged only when
-all preceding merged conditions are non-dependent constant ``true`` expressions.
-This preserves discarded-branch behavior for nested template-dependent
-``if constexpr`` statements.
+For ``if constexpr``, dependent nested conditions are merged only when they can
+be formed outside the discarded branch. This includes conditions such as
+non-type template parameters and ``requires`` expressions, but excludes
+conditions such as ``sizeof(typename T::type)`` after an earlier dependent
+condition.
Options
-------
.. option:: AllowUserDefinedBoolConversion
- When set to `true`, the check also merges chains whose conditions rely on
- user-defined conversion to ``bool``.
-
- The fix-it inserts ``static_cast<bool>(...)`` where needed so the merged
- condition still uses built-in ``&&`` semantics.
-
- Default is `false`.
+ When set to `true`, the check also diagnoses chains whose merged conditions
+ require user-defined conversion to ``bool``. Fix-its insert
+ ``static_cast<bool>(...)`` where needed so the merged condition still uses
+ built-in ``&&`` semantics. Default is `false`.
diff --git a/clang-tools-extra/test/clang-tidy/checkers/readability/redundant-nested-if-notes.cpp b/clang-tools-extra/test/clang-tidy/checkers/readability/redundant-nested-if-notes.cpp
new file mode 100644
index 0000000000000..23eed57e827cd
--- /dev/null
+++ b/clang-tools-extra/test/clang-tidy/checkers/readability/redundant-nested-if-notes.cpp
@@ -0,0 +1,25 @@
+// RUN: %check_clang_tidy -std=c++11-or-later %s readability-redundant-nested-if %t -- -- -fno-delayed-template-parsing
+
+bool cond(int X = 0);
+void sink();
+
+void two_if_chain() {
+ // CHECK-NOTES: :[[@LINE+1]]:3: warning: nested if statements can be merged
+ if (cond()) {
+ // CHECK-NOTES: :[[@LINE+1]]:5: note: nested if statement to merge is here
+ if (cond(1))
+ sink();
+ }
+}
+
+void long_if_chain() {
+ // CHECK-NOTES: :[[@LINE+1]]:3: warning: nested if statements can be merged
+ if (cond()) {
+ // CHECK-NOTES: :[[@LINE+1]]:5: note: nested if statement to merge is here
+ if (cond(1)) {
+ // CHECK-NOTES: :[[@LINE+1]]:7: note: nested if statement to merge is here
+ if (cond(2))
+ sink();
+ }
+ }
+}
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
index 87ab471e2a231..cc4a90d436ef6 100644
--- 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
@@ -343,6 +343,17 @@ template <bool B> void dependent_constexpr_outer_with_nested_false_is_fixable()
// CHECK-FIXES-CXX17: sink();
}
+template <bool B, bool C> void dependent_constexpr_bool_operands_are_fixable() {
+ // CHECK-MESSAGES-CXX17: :[[@LINE+1]]:3: warning: nested if statements can be merged
+ if constexpr (B) {
+ // CHECK-MESSAGES-CXX17: :[[@LINE+1]]:5: note: nested if statement to merge is here
+ if constexpr (C)
+ sink();
+ }
+ // CHECK-FIXES-CXX17: if constexpr ((B) && (C))
+ // CHECK-FIXES-CXX17: sink();
+}
+
template <typename T> void dependent_constexpr_operand_after_true_is_fixable() {
// CHECK-MESSAGES-CXX17: :[[@LINE+1]]:3: warning: nested if statements can be merged
if constexpr (true) {
@@ -438,6 +449,17 @@ template <typename T> void dependent_requires_outer_is_fixable() {
// CHECK-FIXES-CXX20: sink();
}
+template <bool B, typename T> void dependent_requires_after_bool_is_fixable() {
+ // CHECK-MESSAGES-CXX20: :[[@LINE+1]]:3: warning: nested if statements can be merged
+ if constexpr (B) {
+ // CHECK-MESSAGES-CXX20: :[[@LINE+1]]:5: note: nested if statement to merge is here
+ if constexpr (requires { typename T::type; })
+ sink();
+ }
+ // CHECK-FIXES-CXX20: if constexpr ((B) && (requires { typename T::type; }))
+ // 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) {
More information about the cfe-commits
mailing list