Skip to content
Open
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
1 change: 0 additions & 1 deletion clang-tools-extra/clang-tidy/.clang-tidy
Original file line number Diff line number Diff line change
Expand Up @@ -29,7 +29,6 @@ Checks: >
-readability-magic-numbers,
-readability-named-parameter,
-readability-qualified-auto,
-readability-simplify-boolean-expr,
-readability-static-definition-in-anonymous-namespace,
-readability-suspicious-call-argument,
-readability-use-anyofallof
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -1880,12 +1880,9 @@ static bool prefixSuffixCoverUnderThreshold(std::size_t Threshold,
padStringAtBegin(S1PadB, BiggerLength);
padStringAtBegin(S2PadB, BiggerLength);

if (isCommonSuffixWithoutSomeCharacters(
Threshold, StringRef{S1PadB.begin(), BiggerLength},
StringRef{S2PadB.begin(), BiggerLength}))
return true;

return false;
return isCommonSuffixWithoutSomeCharacters(
Threshold, StringRef{S1PadB.begin(), BiggerLength},
StringRef{S2PadB.begin(), BiggerLength});
}

} // namespace filter
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -268,10 +268,8 @@ static bool isDestCapacityOverflows(const MatchFinder::MatchResult &Result) {
// Assume that the destination array's capacity cannot overflow if the
// expression of the memory allocation contains '+ 1'.
StringRef DestCapacityExprStr = exprToStr(DestCapacityExpr, Result);
if (DestCapacityExprStr.contains("+1") || DestCapacityExprStr.contains("+ 1"))
return false;

return true;
return !(DestCapacityExprStr.contains("+1") ||
DestCapacityExprStr.contains("+ 1"));
}

static bool
Expand Down Expand Up @@ -533,10 +531,7 @@ AST_MATCHER_P(Expr, hasDefinition, ast_matchers::internal::Matcher<Expr>,
hasLHS(declRefExpr(to(varDecl(equalsBoundNode(VarDeclName))))),
hasRHS(ignoringImpCasts(InnerMatcher))))))));

if (DREHasDefinition.matches(*SimpleNode, Finder, Builder))
return true;

return false;
return DREHasDefinition.matches(*SimpleNode, Finder, Builder);
}
} // namespace

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -100,7 +100,7 @@ static bool isPossiblyBitMask(const EnumDecl *EnumDec) {
return NonPowOfTwoCounter >= 1 && NonPowOfTwoCounter <= 2 &&
NonPowOfTwoCounter < EnumLen / 2 &&
(VR.MaxVal - VR.MinVal != EnumLen - 1) &&
!(NonPowOfTwoCounter == 1 && isMaxValAllBitSetLiteral(EnumDec));
(NonPowOfTwoCounter != 1 || !isMaxValAllBitSetLiteral(EnumDec));
}

SuspiciousEnumUsageCheck::SuspiciousEnumUsageCheck(StringRef Name,
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -27,10 +27,7 @@ static bool isMessageExpressionInsideMacro(const ObjCMessageExpr *Expr) {
return true;

SourceLocation SelectorLocation = Expr->getSelectorStartLoc();
if (SelectorLocation.isMacroID())
return true;

return false;
return SelectorLocation.isMacroID();
}

// Walk up the class hierarchy looking for an -init method, returning true
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -39,7 +39,7 @@ void ImplementationInNamespaceCheck::check(
}

// Enforce that the namespace is the result of macro expansion
if (Result.SourceManager->isMacroBodyExpansion(NS->getLocation()) == false) {
if (!Result.SourceManager->isMacroBodyExpansion(NS->getLocation())) {
diag(NS->getLocation(), "the outermost namespace should be the '%0' macro")
<< RequiredNamespaceDeclMacroName;
return;
Expand All @@ -55,7 +55,7 @@ void ImplementationInNamespaceCheck::check(
}

// Lastly, make sure the namespace name actually has the __llvm_libc prefix
if (NS->getName().starts_with(RequiredNamespaceRefStart) == false) {
if (!NS->getName().starts_with(RequiredNamespaceRefStart)) {
diag(NS->getLocation(), "the '%0' macro expansion should start with '%1'")
<< RequiredNamespaceDeclMacroName << RequiredNamespaceRefStart;
return;
Expand Down
3 changes: 1 addition & 2 deletions clang-tools-extra/clang-tidy/misc/ConstCorrectnessCheck.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -54,8 +54,7 @@ ConstCorrectnessCheck::ConstCorrectnessCheck(StringRef Name,

AllowedTypes(
utils::options::parseStringList(Options.get("AllowedTypes", ""))) {
if (AnalyzeValues == false && AnalyzeReferences == false &&
AnalyzePointers == false)
if (!AnalyzeValues && !AnalyzeReferences && !AnalyzePointers)
this->configurationDiag(
"The check 'misc-const-correctness' will not "
"perform any analysis because 'AnalyzeValues', "
Expand Down
2 changes: 1 addition & 1 deletion clang-tools-extra/clang-tidy/misc/IncludeCleanerCheck.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -70,7 +70,7 @@ IncludeCleanerCheck::IncludeCleanerCheck(StringRef Name,
IgnoreHeadersRegex.emplace_back(HeaderSuffix);
}

if (UnusedIncludes == false && MissingIncludes == false)
if (!UnusedIncludes && !MissingIncludes)
this->configurationDiag("The check 'misc-include-cleaner' will not "
"perform any analysis because 'UnusedIncludes' and "
"'MissingIncludes' are both false.");
Expand Down
7 changes: 2 additions & 5 deletions clang-tools-extra/clang-tidy/misc/NewDeleteOverloadsCheck.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -52,11 +52,8 @@ AST_MATCHER(FunctionDecl, isPlacementOverload) {

const auto *FPT = Node.getType()->castAs<FunctionProtoType>();
ASTContext &Ctx = Node.getASTContext();
if (Ctx.getLangOpts().SizedDeallocation &&
Ctx.hasSameType(FPT->getParamType(1), Ctx.getSizeType()))
return false;

return true;
return !(Ctx.getLangOpts().SizedDeallocation &&
Ctx.hasSameType(FPT->getParamType(1), Ctx.getSizeType()));
}

} // namespace
Expand Down
25 changes: 9 additions & 16 deletions clang-tools-extra/clang-tidy/misc/RedundantExpressionCheck.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -196,12 +196,9 @@ static bool areExclusiveRanges(BinaryOperatorKind OpcodeLHS,

// Handle the case where constants are off by one: x > 5 && x < 6.
APSInt ValueLhsPlus1;
if (OpcodeLHS == BO_GT && OpcodeRHS == BO_LT &&
incrementWithoutOverflow(ValueLHS, ValueLhsPlus1) &&
APSInt::compareValues(ValueLhsPlus1, ValueRHS) == 0)
return true;

return false;
return OpcodeLHS == BO_GT && OpcodeRHS == BO_LT &&
incrementWithoutOverflow(ValueLHS, ValueLhsPlus1) &&
APSInt::compareValues(ValueLhsPlus1, ValueRHS) == 0;
}

// Returns whether the ranges covered by the union of both relational
Expand Down Expand Up @@ -726,12 +723,10 @@ static bool areSidesBinaryConstExpressions(const BinaryOperator *&BinOp,
return !E->isValueDependent() && E->isIntegerConstantExpr(*AstCtx);
};

if ((IsIntegerConstantExpr(LhsBinOp->getLHS()) ||
IsIntegerConstantExpr(LhsBinOp->getRHS())) &&
(IsIntegerConstantExpr(RhsBinOp->getLHS()) ||
IsIntegerConstantExpr(RhsBinOp->getRHS())))
return true;
return false;
return (IsIntegerConstantExpr(LhsBinOp->getLHS()) ||
IsIntegerConstantExpr(LhsBinOp->getRHS())) &&
(IsIntegerConstantExpr(RhsBinOp->getLHS()) ||
IsIntegerConstantExpr(RhsBinOp->getRHS()));
}

static bool areSidesBinaryConstExpressionsOrDefinesOrIntegerConstant(
Expand All @@ -747,10 +742,8 @@ static bool areSidesBinaryConstExpressionsOrDefinesOrIntegerConstant(

auto IsDefineExpr = [AstCtx](const Expr *E) {
const SourceRange Lsr = E->getSourceRange();
if (!Lsr.getBegin().isMacroID() || E->isValueDependent() ||
!E->isIntegerConstantExpr(*AstCtx))
return false;
return true;
return !(!Lsr.getBegin().isMacroID() || E->isValueDependent() ||
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

For example, this transformation made this code a lot harder to understand

Copy link
Contributor Author

@capitan-davide capitan-davide Sep 16, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Agree, IMO this more readable if we apply De Morgan here. We can even apply it manually and the check won't complain any longer (so, we can keep the default configuration in this case).

Original:

    if (!Lsr.getBegin().isMacroID() || E->isValueDependent() ||
        !E->isIntegerConstantExpr(*AstCtx))
      return false;
    return true;

Before:

    return !(!Lsr.getBegin().isMacroID() || E->isValueDependent() ||
             !E->isIntegerConstantExpr(*AstCtx));

After:

    return Lsr.getBegin().isMacroID() && !E->isValueDependent() &&
             E->isIntegerConstantExpr(*AstCtx));

What do you think?

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Sorry for late reply, IMO DE-morgan looks better here. We could try it (I see there are places that it could be applied)

!E->isIntegerConstantExpr(*AstCtx));
};

return IsDefineExpr(Lhs) || IsDefineExpr(Rhs);
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -459,7 +459,7 @@ UseTrailingReturnTypeCheck::UseTrailingReturnTypeCheck(
TransformFunctions(Options.get("TransformFunctions", true)),
TransformLambdas(Options.get("TransformLambdas", TransformLambda::All)) {

if (TransformFunctions == false && TransformLambdas == TransformLambda::None)
if (!TransformFunctions && TransformLambdas == TransformLambda::None)
this->configurationDiag(
"The check 'modernize-use-trailing-return-type' will not perform any "
"analysis because 'TransformFunctions' and 'TransformLambdas' are "
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -209,7 +209,7 @@ getEquivalentForBoolLiteral(const CXXBoolLiteralExpr *BoolLiteral,
// Prior to C++11, false literal could be implicitly converted to pointer.
if (!Context.getLangOpts().CPlusPlus11 &&
(DestType->isPointerType() || DestType->isMemberPointerType()) &&
BoolLiteral->getValue() == false) {
!BoolLiteral->getValue()) {
return "0";
}

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -413,14 +413,12 @@ static bool areTypesCompatible(QualType ArgType, QualType ParamType,

// Arithmetic types are interconvertible, except scoped enums.
if (ParamType->isArithmeticType() && ArgType->isArithmeticType()) {
if ((ParamType->isEnumeralType() && ParamType->castAsCanonical<EnumType>()
return !(
(ParamType->isEnumeralType() && ParamType->castAsCanonical<EnumType>()
->getOriginalDecl()
->isScoped()) ||
(ArgType->isEnumeralType() &&
ArgType->castAsCanonical<EnumType>()->getOriginalDecl()->isScoped()))
return false;

return true;
ArgType->castAsCanonical<EnumType>()->getOriginalDecl()->isScoped()));
}

// Check if the argument and the param are both function types (the parameter
Expand Down