================ @@ -0,0 +1,147 @@ +//===--- ChainedComparisonCheck.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 "ChainedComparisonCheck.h" +#include "clang/AST/ASTContext.h" +#include "clang/ASTMatchers/ASTMatchFinder.h" +#include "llvm/ADT/SmallString.h" +#include "llvm/ADT/SmallVector.h" +#include <algorithm> +#include <array> + +using namespace clang::ast_matchers; + +namespace clang::tidy::bugprone { +static bool isExprAComparisonOperator(const Expr *E) { + if (const auto *Op = dyn_cast_or_null<BinaryOperator>(E->IgnoreImplicit())) + return Op->isComparisonOp(); + if (const auto *Op = + dyn_cast_or_null<CXXOperatorCallExpr>(E->IgnoreImplicit())) + return Op->isComparisonOp(); + return false; +} + +namespace { +AST_MATCHER(BinaryOperator, + hasBinaryOperatorAChildComparisonOperatorWithoutParen) { + return isExprAComparisonOperator(Node.getLHS()) || + isExprAComparisonOperator(Node.getRHS()); +} + +AST_MATCHER(CXXOperatorCallExpr, + hasCppOperatorAChildComparisonOperatorWithoutParen) { + return std::any_of(Node.arg_begin(), Node.arg_end(), + isExprAComparisonOperator); +} + +struct ChainedComparisonData { + llvm::SmallString<256U> Name; + llvm::SmallVector<const Expr *, 32U> Operands; + + explicit ChainedComparisonData(const Expr *Op) { extract(Op); } + +private: + void add(const Expr *Operand); + void add(llvm::StringRef Opcode); + void extract(const Expr *Op); + bool extract(const BinaryOperator *Op); + bool extract(const CXXOperatorCallExpr *Op); +}; + +void ChainedComparisonData::add(const Expr *Operand) { + if (!Name.empty()) + Name += ' '; + Name += 'v'; + Name += std::to_string(Operands.size()); + Operands.push_back(Operand); +} + +void ChainedComparisonData::add(llvm::StringRef Opcode) { + Name += ' '; + Name += Opcode.str(); +} + +bool ChainedComparisonData::extract(const BinaryOperator *Op) { + if (!Op) + return false; + + if (isExprAComparisonOperator(Op->getLHS())) + extract(Op->getLHS()->IgnoreImplicit()); + else + add(Op->getLHS()->IgnoreUnlessSpelledInSource()); + + add(Op->getOpcodeStr()); + + if (isExprAComparisonOperator(Op->getRHS())) + extract(Op->getRHS()->IgnoreImplicit()); + else + add(Op->getRHS()->IgnoreUnlessSpelledInSource()); + return true; +} + +bool ChainedComparisonData::extract(const CXXOperatorCallExpr *Op) { + if (!Op || Op->getNumArgs() != 2U) + return false; + + const Expr *FirstArg = Op->getArg(0U)->IgnoreImplicit(); + if (isExprAComparisonOperator(FirstArg)) + extract(FirstArg); + else + add(FirstArg->IgnoreUnlessSpelledInSource()); + + add(getOperatorSpelling(Op->getOperator())); + + const Expr *SecondArg = Op->getArg(1U)->IgnoreImplicit(); + if (isExprAComparisonOperator(SecondArg)) + extract(SecondArg); + else + add(SecondArg->IgnoreUnlessSpelledInSource()); + return true; +} + +void ChainedComparisonData::extract(const Expr *Op) { + extract(dyn_cast_or_null<BinaryOperator>(Op)) || + extract(dyn_cast_or_null<CXXOperatorCallExpr>(Op)); +} + +} // namespace + +void ChainedComparisonCheck::registerMatchers(MatchFinder *Finder) { + const auto OperatorMatcher = expr(anyOf( + binaryOperator(isComparisonOperator(), + hasBinaryOperatorAChildComparisonOperatorWithoutParen()), + cxxOperatorCallExpr( + isComparisonOperator(), + hasCppOperatorAChildComparisonOperatorWithoutParen()))); + + Finder->addMatcher( + expr(OperatorMatcher, unless(hasParent(OperatorMatcher))).bind("op"), + this); +} + +void ChainedComparisonCheck::check(const MatchFinder::MatchResult &Result) { + const auto *MatchedOperator = Result.Nodes.getNodeAs<Expr>("op"); + + ChainedComparisonData Data(MatchedOperator); + if (Data.Operands.empty()) + return; + + diag(MatchedOperator->getBeginLoc(), + "chained comparison '%0' may generate unintended results, use " + "parentheses to specify order of evaluation or a logical operator to " + "separate comparison expressions") + << llvm::StringRef(Data.Name).trim(); + + for (std::size_t Index = 0U; Index < Data.Operands.size(); ++Index) { + diag(Data.Operands[Index]->getBeginLoc(), "operand 'v%0' is here", + DiagnosticIDs::Note) + << Index; + } ---------------- 5chmidti wrote:
What do you think about streaming the source range of the operand as well, to get the whole underline instead of just the `^` (for warning and notes)? Doing this would also allow multi-line matches to be printed as a whole, not just the first line. https://github.com/llvm/llvm-project/pull/76365 _______________________________________________ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
