[clang-tools-extra] b9ece03 - [clang-tidy] Suppress reports to similarly used parameters in 'bugprone-easily-swappable-parameters'
via cfe-commits
cfe-commits at lists.llvm.org
Mon Jun 28 01:50:47 PDT 2021
Author: Whisperity
Date: 2021-06-28T10:49:37+02:00
New Revision: b9ece034611239d008ac11d8bb9b3af91313c41f
URL: https://github.com/llvm/llvm-project/commit/b9ece034611239d008ac11d8bb9b3af91313c41f
DIFF: https://github.com/llvm/llvm-project/commit/b9ece034611239d008ac11d8bb9b3af91313c41f.diff
LOG: [clang-tidy] Suppress reports to similarly used parameters in 'bugprone-easily-swappable-parameters'
There are several types of functions and various reasons why some
"swappable parameters" cannot be fixed with changing the parameters' types, etc.
The most common example might be int `min(int a, int b)`... no matter what you
do, the two parameters must remain the same type.
The **filtering heuristic** implemented in this patch deals with trying to find
such functions during the modelling and building of the swappable parameter
range.
If the parameter currently scrutinised matches either of the predicates below,
it will be regarded as **not swappable** even if the type of the parameter
matches.
Reviewed By: aaron.ballman
Differential Revision: http://reviews.llvm.org/D78652
Added:
clang-tools-extra/test/clang-tidy/checkers/bugprone-easily-swappable-parameters-relatedness.c
clang-tools-extra/test/clang-tidy/checkers/bugprone-easily-swappable-parameters-relatedness.cpp
Modified:
clang-tools-extra/clang-tidy/bugprone/EasilySwappableParametersCheck.cpp
clang-tools-extra/clang-tidy/bugprone/EasilySwappableParametersCheck.h
clang-tools-extra/docs/clang-tidy/checks/bugprone-easily-swappable-parameters.rst
clang-tools-extra/test/clang-tidy/checkers/bugprone-easily-swappable-parameters-ignore.cpp
clang-tools-extra/test/clang-tidy/checkers/bugprone-easily-swappable-parameters-implicit-qualifiers.cpp
clang-tools-extra/test/clang-tidy/checkers/bugprone-easily-swappable-parameters-implicits.c
clang-tools-extra/test/clang-tidy/checkers/bugprone-easily-swappable-parameters-implicits.cpp
clang-tools-extra/test/clang-tidy/checkers/bugprone-easily-swappable-parameters-len2.cpp
clang-tools-extra/test/clang-tidy/checkers/bugprone-easily-swappable-parameters-len3.cpp
clang-tools-extra/test/clang-tidy/checkers/bugprone-easily-swappable-parameters-qualifiermixing.cpp
clang-tools-extra/test/clang-tidy/checkers/bugprone-easily-swappable-parameters.c
Removed:
################################################################################
diff --git a/clang-tools-extra/clang-tidy/bugprone/EasilySwappableParametersCheck.cpp b/clang-tools-extra/clang-tidy/bugprone/EasilySwappableParametersCheck.cpp
index c4896979d2e99..247953c25a3b8 100644
--- a/clang-tools-extra/clang-tidy/bugprone/EasilySwappableParametersCheck.cpp
+++ b/clang-tools-extra/clang-tidy/bugprone/EasilySwappableParametersCheck.cpp
@@ -9,6 +9,7 @@
#include "EasilySwappableParametersCheck.h"
#include "../utils/OptionsUtils.h"
#include "clang/AST/ASTContext.h"
+#include "clang/AST/RecursiveASTVisitor.h"
#include "clang/ASTMatchers/ASTMatchFinder.h"
#include "clang/Lex/Lexer.h"
#include "llvm/ADT/SmallSet.h"
@@ -65,6 +66,10 @@ static constexpr bool DefaultQualifiersMix = false;
/// The default value for the ModelImplicitConversions check option.
static constexpr bool DefaultModelImplicitConversions = true;
+/// The default value for suppressing diagnostics about parameters that are
+/// used together.
+static constexpr bool DefaultSuppressParametersUsedTogether = true;
+
using namespace clang::ast_matchers;
namespace clang {
@@ -74,7 +79,12 @@ namespace bugprone {
using TheCheck = EasilySwappableParametersCheck;
namespace filter {
+class SimilarlyUsedParameterPairSuppressor;
+
static bool isIgnoredParameter(const TheCheck &Check, const ParmVarDecl *Node);
+static inline bool
+isSimilarlyUsedParameter(const SimilarlyUsedParameterPairSuppressor &Suppressor,
+ const ParmVarDecl *Param1, const ParmVarDecl *Param2);
} // namespace filter
namespace model {
@@ -1269,9 +1279,9 @@ approximateImplicitConversion(const TheCheck &Check, QualType LType,
return {MixFlags::None};
}
-static MixableParameterRange modelMixingRange(const TheCheck &Check,
- const FunctionDecl *FD,
- std::size_t StartIndex) {
+static MixableParameterRange modelMixingRange(
+ const TheCheck &Check, const FunctionDecl *FD, std::size_t StartIndex,
+ const filter::SimilarlyUsedParameterPairSuppressor &UsageBasedSuppressor) {
std::size_t NumParams = FD->getNumParams();
assert(StartIndex < NumParams && "out of bounds for start");
const ASTContext &Ctx = FD->getASTContext();
@@ -1297,6 +1307,19 @@ static MixableParameterRange modelMixingRange(const TheCheck &Check,
LLVM_DEBUG(llvm::dbgs()
<< "Check mix of #" << J << " against #" << I << "...\n");
+ if (isSimilarlyUsedParameter(UsageBasedSuppressor, Ith, Jth)) {
+ // Consider the two similarly used parameters to not be possible in a
+ // mix-up at the user's request, if they enabled this heuristic.
+ LLVM_DEBUG(llvm::dbgs() << "Parameters #" << I << " and #" << J
+ << " deemed related, ignoring...\n");
+
+ // If the parameter #I and #J mixes, then I is mixable with something
+ // in the current range, so the range has to be broken and I not
+ // included.
+ MixesOfIth.clear();
+ break;
+ }
+
Mix M{Jth, Ith,
calculateMixability(Check, Jth->getType(), Ith->getType(), Ctx,
Check.ModelImplicitConversions ? ICMM_All
@@ -1331,6 +1354,12 @@ static MixableParameterRange modelMixingRange(const TheCheck &Check,
} // namespace model
+/// Matches DeclRefExprs and their ignorable wrappers to ParmVarDecls.
+AST_MATCHER_FUNCTION(ast_matchers::internal::Matcher<Stmt>, paramRefExpr) {
+ return expr(ignoringParenImpCasts(ignoringElidableConstructorCall(
+ declRefExpr(to(parmVarDecl().bind("param"))))));
+}
+
namespace filter {
/// Returns whether the parameter's name or the parameter's type's name is
@@ -1391,6 +1420,261 @@ static bool isIgnoredParameter(const TheCheck &Check, const ParmVarDecl *Node) {
return false;
}
+/// This namespace contains the implementations for the suppression of
+/// diagnostics from similaly used ("related") parameters.
+namespace relatedness_heuristic {
+
+static constexpr std::size_t SmallDataStructureSize = 4;
+
+template <typename T, std::size_t N = SmallDataStructureSize>
+using ParamToSmallSetMap =
+ llvm::DenseMap<const ParmVarDecl *, llvm::SmallSet<T, N>>;
+
+/// Returns whether the sets mapped to the two elements in the map have at
+/// least one element in common.
+template <typename MapTy, typename ElemTy>
+bool lazyMapOfSetsIntersectionExists(const MapTy &Map, const ElemTy &E1,
+ const ElemTy &E2) {
+ auto E1Iterator = Map.find(E1);
+ auto E2Iterator = Map.find(E2);
+ if (E1Iterator == Map.end() || E2Iterator == Map.end())
+ return false;
+
+ for (const auto &E1SetElem : E1Iterator->second)
+ if (llvm::find(E2Iterator->second, E1SetElem) != E2Iterator->second.end())
+ return true;
+
+ return false;
+}
+
+/// Implements the heuristic that marks two parameters related if there is
+/// a usage for both in the same strict expression subtree. A strict
+/// expression subtree is a tree which only includes Expr nodes, i.e. no
+/// Stmts and no Decls.
+class AppearsInSameExpr : public RecursiveASTVisitor<AppearsInSameExpr> {
+ using Base = RecursiveASTVisitor<AppearsInSameExpr>;
+
+ const FunctionDecl *FD;
+ const Expr *CurrentExprOnlyTreeRoot = nullptr;
+ llvm::DenseMap<const ParmVarDecl *,
+ llvm::SmallPtrSet<const Expr *, SmallDataStructureSize>>
+ ParentExprsForParamRefs;
+
+public:
+ void setup(const FunctionDecl *FD) {
+ this->FD = FD;
+ TraverseFunctionDecl(const_cast<FunctionDecl *>(FD));
+ }
+
+ bool operator()(const ParmVarDecl *Param1, const ParmVarDecl *Param2) const {
+ return lazyMapOfSetsIntersectionExists(ParentExprsForParamRefs, Param1,
+ Param2);
+ }
+
+ bool TraverseDecl(Decl *D) {
+ CurrentExprOnlyTreeRoot = nullptr;
+ return Base::TraverseDecl(D);
+ }
+
+ bool TraverseStmt(Stmt *S, DataRecursionQueue *Queue = nullptr) {
+ if (auto *E = dyn_cast_or_null<Expr>(S)) {
+ bool RootSetInCurrentStackFrame = false;
+ if (!CurrentExprOnlyTreeRoot) {
+ CurrentExprOnlyTreeRoot = E;
+ RootSetInCurrentStackFrame = true;
+ }
+
+ bool Ret = Base::TraverseStmt(S);
+
+ if (RootSetInCurrentStackFrame)
+ CurrentExprOnlyTreeRoot = nullptr;
+
+ return Ret;
+ }
+
+ // A Stmt breaks the strictly Expr subtree.
+ CurrentExprOnlyTreeRoot = nullptr;
+ return Base::TraverseStmt(S);
+ }
+
+ bool VisitDeclRefExpr(DeclRefExpr *DRE) {
+ if (!CurrentExprOnlyTreeRoot)
+ return true;
+
+ if (auto *PVD = dyn_cast<ParmVarDecl>(DRE->getDecl()))
+ if (llvm::find(FD->parameters(), PVD))
+ ParentExprsForParamRefs[PVD].insert(CurrentExprOnlyTreeRoot);
+
+ return true;
+ }
+};
+
+/// Implements the heuristic that marks two parameters related if there are
+/// two separate calls to the same function (overload) and the parameters are
+/// passed to the same index in both calls, i.e f(a, b) and f(a, c) passes
+/// b and c to the same index (2) of f(), marking them related.
+class PassedToSameFunction {
+ ParamToSmallSetMap<std::pair<const FunctionDecl *, unsigned>> TargetParams;
+
+public:
+ void setup(const FunctionDecl *FD) {
+ auto ParamsAsArgsInFnCalls =
+ match(functionDecl(forEachDescendant(
+ callExpr(forEachArgumentWithParam(
+ paramRefExpr(), parmVarDecl().bind("passed-to")))
+ .bind("call-expr"))),
+ *FD, FD->getASTContext());
+ for (const auto &Match : ParamsAsArgsInFnCalls) {
+ const auto *PassedParamOfThisFn = Match.getNodeAs<ParmVarDecl>("param");
+ const auto *CE = Match.getNodeAs<CallExpr>("call-expr");
+ const auto *PassedToParam = Match.getNodeAs<ParmVarDecl>("passed-to");
+ assert(PassedParamOfThisFn && CE && PassedToParam);
+
+ const FunctionDecl *CalledFn = CE->getDirectCallee();
+ if (!CalledFn)
+ continue;
+
+ llvm::Optional<unsigned> TargetIdx;
+ unsigned NumFnParams = CalledFn->getNumParams();
+ for (unsigned Idx = 0; Idx < NumFnParams; ++Idx)
+ if (CalledFn->getParamDecl(Idx) == PassedToParam)
+ TargetIdx.emplace(Idx);
+
+ assert(TargetIdx.hasValue() && "Matched, but didn't find index?");
+ TargetParams[PassedParamOfThisFn].insert(
+ {CalledFn->getCanonicalDecl(), *TargetIdx});
+ }
+ }
+
+ bool operator()(const ParmVarDecl *Param1, const ParmVarDecl *Param2) const {
+ return lazyMapOfSetsIntersectionExists(TargetParams, Param1, Param2);
+ }
+};
+
+/// Implements the heuristic that marks two parameters related if the same
+/// member is accessed (referred to) inside the current function's body.
+class AccessedSameMemberOf {
+ ParamToSmallSetMap<const Decl *> AccessedMembers;
+
+public:
+ void setup(const FunctionDecl *FD) {
+ auto MembersCalledOnParams = match(
+ functionDecl(forEachDescendant(
+ memberExpr(hasObjectExpression(paramRefExpr())).bind("mem-expr"))),
+ *FD, FD->getASTContext());
+
+ for (const auto &Match : MembersCalledOnParams) {
+ const auto *AccessedParam = Match.getNodeAs<ParmVarDecl>("param");
+ const auto *ME = Match.getNodeAs<MemberExpr>("mem-expr");
+ assert(AccessedParam && ME);
+ AccessedMembers[AccessedParam].insert(
+ ME->getMemberDecl()->getCanonicalDecl());
+ }
+ }
+
+ bool operator()(const ParmVarDecl *Param1, const ParmVarDecl *Param2) const {
+ return lazyMapOfSetsIntersectionExists(AccessedMembers, Param1, Param2);
+ }
+};
+
+/// Implements the heuristic that marks two parameters related if
diff erent
+/// ReturnStmts return them from the function.
+class Returned {
+ llvm::SmallVector<const ParmVarDecl *, SmallDataStructureSize> ReturnedParams;
+
+public:
+ void setup(const FunctionDecl *FD) {
+ // TODO: Handle co_return.
+ auto ParamReturns = match(functionDecl(forEachDescendant(
+ returnStmt(hasReturnValue(paramRefExpr())))),
+ *FD, FD->getASTContext());
+ for (const auto &Match : ParamReturns) {
+ const auto *ReturnedParam = Match.getNodeAs<ParmVarDecl>("param");
+ assert(ReturnedParam);
+
+ if (find(FD->parameters(), ReturnedParam) == FD->param_end())
+ // Inside the subtree of a FunctionDecl there might be ReturnStmts of
+ // a parameter that isn't the parameter of the function, e.g. in the
+ // case of lambdas.
+ continue;
+
+ ReturnedParams.emplace_back(ReturnedParam);
+ }
+ }
+
+ bool operator()(const ParmVarDecl *Param1, const ParmVarDecl *Param2) const {
+ return llvm::find(ReturnedParams, Param1) != ReturnedParams.end() &&
+ llvm::find(ReturnedParams, Param2) != ReturnedParams.end();
+ }
+};
+
+} // namespace relatedness_heuristic
+
+/// Helper class that is used to detect if two parameters of the same function
+/// are used in a similar fashion, to suppress the result.
+class SimilarlyUsedParameterPairSuppressor {
+ const bool Enabled;
+ relatedness_heuristic::AppearsInSameExpr SameExpr;
+ relatedness_heuristic::PassedToSameFunction PassToFun;
+ relatedness_heuristic::AccessedSameMemberOf SameMember;
+ relatedness_heuristic::Returned Returns;
+
+public:
+ SimilarlyUsedParameterPairSuppressor(const FunctionDecl *FD, bool Enable)
+ : Enabled(Enable) {
+ if (!Enable)
+ return;
+
+ SameExpr.setup(FD);
+ PassToFun.setup(FD);
+ SameMember.setup(FD);
+ Returns.setup(FD);
+ }
+
+ /// Returns whether the specified two parameters are deemed similarly used
+ /// or related by the heuristics.
+ bool operator()(const ParmVarDecl *Param1, const ParmVarDecl *Param2) const {
+ if (!Enabled)
+ return false;
+
+ LLVM_DEBUG(llvm::dbgs()
+ << "::: Matching similar usage / relatedness heuristic...\n");
+
+ if (SameExpr(Param1, Param2)) {
+ LLVM_DEBUG(llvm::dbgs() << "::: Used in the same expression.\n");
+ return true;
+ }
+
+ if (PassToFun(Param1, Param2)) {
+ LLVM_DEBUG(llvm::dbgs()
+ << "::: Passed to same function in
diff erent calls.\n");
+ return true;
+ }
+
+ if (SameMember(Param1, Param2)) {
+ LLVM_DEBUG(llvm::dbgs()
+ << "::: Same member field access or method called.\n");
+ return true;
+ }
+
+ if (Returns(Param1, Param2)) {
+ LLVM_DEBUG(llvm::dbgs() << "::: Both parameter returned.\n");
+ return true;
+ }
+
+ LLVM_DEBUG(llvm::dbgs() << "::: None.\n");
+ return false;
+ }
+};
+
+// (This function hoists the call to operator() of the wrapper, so we do not
+// need to define the previous class at the top of the file.)
+static inline bool
+isSimilarlyUsedParameter(const SimilarlyUsedParameterPairSuppressor &Suppressor,
+ const ParmVarDecl *Param1, const ParmVarDecl *Param2) {
+ return Suppressor(Param1, Param2);
+}
+
} // namespace filter
/// Matches functions that have at least the specified amount of parameters.
@@ -1604,7 +1888,10 @@ EasilySwappableParametersCheck::EasilySwappableParametersCheck(
DefaultIgnoredParameterTypeSuffixes))),
QualifiersMix(Options.get("QualifiersMix", DefaultQualifiersMix)),
ModelImplicitConversions(Options.get("ModelImplicitConversions",
- DefaultModelImplicitConversions)) {}
+ DefaultModelImplicitConversions)),
+ SuppressParametersUsedTogether(
+ Options.get("SuppressParametersUsedTogether",
+ DefaultSuppressParametersUsedTogether)) {}
void EasilySwappableParametersCheck::storeOptions(
ClangTidyOptions::OptionMap &Opts) {
@@ -1615,6 +1902,8 @@ void EasilySwappableParametersCheck::storeOptions(
optutils::serializeStringList(IgnoredParameterTypeSuffixes));
Options.store(Opts, "QualifiersMix", QualifiersMix);
Options.store(Opts, "ModelImplicitConversions", ModelImplicitConversions);
+ Options.store(Opts, "SuppressParametersUsedTogether",
+ SuppressParametersUsedTogether);
}
void EasilySwappableParametersCheck::registerMatchers(MatchFinder *Finder) {
@@ -1647,6 +1936,11 @@ void EasilySwappableParametersCheck::check(
std::size_t NumParams = FD->getNumParams();
std::size_t MixableRangeStartIndex = 0;
+ // Spawn one suppressor and if the user requested, gather information from
+ // the AST for the parameters' usages.
+ filter::SimilarlyUsedParameterPairSuppressor UsageBasedSuppressor{
+ FD, SuppressParametersUsedTogether};
+
LLVM_DEBUG(llvm::dbgs() << "Begin analysis of " << getName(FD) << " with "
<< NumParams << " parameters...\n");
while (MixableRangeStartIndex < NumParams) {
@@ -1657,8 +1951,8 @@ void EasilySwappableParametersCheck::check(
continue;
}
- MixableParameterRange R =
- modelMixingRange(*this, FD, MixableRangeStartIndex);
+ MixableParameterRange R = modelMixingRange(
+ *this, FD, MixableRangeStartIndex, UsageBasedSuppressor);
assert(R.NumParamsChecked > 0 && "Ensure forward progress!");
MixableRangeStartIndex += R.NumParamsChecked;
if (R.NumParamsChecked < MinimumLength) {
diff --git a/clang-tools-extra/clang-tidy/bugprone/EasilySwappableParametersCheck.h b/clang-tools-extra/clang-tidy/bugprone/EasilySwappableParametersCheck.h
index b072c46680452..22e36ffa91cc1 100644
--- a/clang-tools-extra/clang-tidy/bugprone/EasilySwappableParametersCheck.h
+++ b/clang-tools-extra/clang-tidy/bugprone/EasilySwappableParametersCheck.h
@@ -47,6 +47,10 @@ class EasilySwappableParametersCheck : public ClangTidyCheck {
/// during analysis and consider types that are implicitly convertible to
/// one another mixable.
const bool ModelImplicitConversions;
+
+ /// If enabled, diagnostics for parameters that are used together in a
+ /// similar way are not emitted.
+ const bool SuppressParametersUsedTogether;
};
} // namespace bugprone
diff --git a/clang-tools-extra/docs/clang-tidy/checks/bugprone-easily-swappable-parameters.rst b/clang-tools-extra/docs/clang-tidy/checks/bugprone-easily-swappable-parameters.rst
index 5158394e4e1fe..5a61de8f2d770 100644
--- a/clang-tools-extra/docs/clang-tidy/checks/bugprone-easily-swappable-parameters.rst
+++ b/clang-tools-extra/docs/clang-tidy/checks/bugprone-easily-swappable-parameters.rst
@@ -140,6 +140,34 @@ noisiness.
`ConstIterator`, `const_reverse_iterator`, `ConstReverseIterator`.
In addition, `_Bool` (but not `_bool`) is also part of the default value.
+.. option:: SuppressParametersUsedTogether
+
+ Suppresses diagnostics about parameters that are used together or in a
+ similar fashion inside the function's body.
+ Defaults to `true`.
+ Specifying `false` will turn off the heuristics.
+
+ Currently, the following heuristics are implemented which will suppress the
+ warning about the parameter pair involved:
+
+ * The parameters are used in the same expression, e.g. ``f(a, b)`` or
+ ``a < b``.
+ * The parameters are further passed to the same function to the same
+ parameter of that function, of the same overload.
+ E.g. ``f(a, 1)`` and ``f(b, 2)`` to some ``f(T, int)``.
+
+ .. note::
+
+ The check does not perform path-sensitive analysis, and as such,
+ "same function" in this context means the same function declaration.
+ If the same member function of a type on two distinct instances are
+ called with the parameters, it will still be regarded as
+ "same function".
+
+ * The same member field is accessed, or member method is called of the
+ two parameters, e.g. ``a.foo()`` and ``b.foo()``.
+ * Separate ``return`` statements return either of the parameters on
+
diff erent code paths.
Limitations
-----------
diff --git a/clang-tools-extra/test/clang-tidy/checkers/bugprone-easily-swappable-parameters-ignore.cpp b/clang-tools-extra/test/clang-tidy/checkers/bugprone-easily-swappable-parameters-ignore.cpp
index 528363bef815f..14d6c05a20ef7 100644
--- a/clang-tools-extra/test/clang-tidy/checkers/bugprone-easily-swappable-parameters-ignore.cpp
+++ b/clang-tools-extra/test/clang-tidy/checkers/bugprone-easily-swappable-parameters-ignore.cpp
@@ -4,7 +4,8 @@
// RUN: {key: bugprone-easily-swappable-parameters.IgnoredParameterNames, value: "\"\";Foo;Bar"}, \
// RUN: {key: bugprone-easily-swappable-parameters.IgnoredParameterTypeSuffixes, value: "T"}, \
// RUN: {key: bugprone-easily-swappable-parameters.QualifiersMix, value: 0}, \
-// RUN: {key: bugprone-easily-swappable-parameters.ModelImplicitConversions, value: 0} \
+// RUN: {key: bugprone-easily-swappable-parameters.ModelImplicitConversions, value: 0}, \
+// RUN: {key: bugprone-easily-swappable-parameters.SuppressParametersUsedTogether, value: 0} \
// RUN: ]}' --
void ignoredUnnamed(int I, int, int) {} // NO-WARN: No >= 2 length of non-unnamed.
diff --git a/clang-tools-extra/test/clang-tidy/checkers/bugprone-easily-swappable-parameters-implicit-qualifiers.cpp b/clang-tools-extra/test/clang-tidy/checkers/bugprone-easily-swappable-parameters-implicit-qualifiers.cpp
index 42dbe9101126f..93290e51ef689 100644
--- a/clang-tools-extra/test/clang-tidy/checkers/bugprone-easily-swappable-parameters-implicit-qualifiers.cpp
+++ b/clang-tools-extra/test/clang-tidy/checkers/bugprone-easily-swappable-parameters-implicit-qualifiers.cpp
@@ -4,7 +4,8 @@
// RUN: {key: bugprone-easily-swappable-parameters.IgnoredParameterNames, value: ""}, \
// RUN: {key: bugprone-easily-swappable-parameters.IgnoredParameterTypeSuffixes, value: ""}, \
// RUN: {key: bugprone-easily-swappable-parameters.QualifiersMix, value: 1}, \
-// RUN: {key: bugprone-easily-swappable-parameters.ModelImplicitConversions, value: 1} \
+// RUN: {key: bugprone-easily-swappable-parameters.ModelImplicitConversions, value: 1}, \
+// RUN: {key: bugprone-easily-swappable-parameters.SuppressParametersUsedTogether, value: 0} \
// RUN: ]}' --
void numericAndQualifierConversion(int I, const double CD) { numericAndQualifierConversion(CD, I); }
diff --git a/clang-tools-extra/test/clang-tidy/checkers/bugprone-easily-swappable-parameters-implicits.c b/clang-tools-extra/test/clang-tidy/checkers/bugprone-easily-swappable-parameters-implicits.c
index 48a3e10d3e316..92a70f44e912c 100644
--- a/clang-tools-extra/test/clang-tidy/checkers/bugprone-easily-swappable-parameters-implicits.c
+++ b/clang-tools-extra/test/clang-tidy/checkers/bugprone-easily-swappable-parameters-implicits.c
@@ -4,7 +4,8 @@
// RUN: {key: bugprone-easily-swappable-parameters.IgnoredParameterNames, value: ""}, \
// RUN: {key: bugprone-easily-swappable-parameters.IgnoredParameterTypeSuffixes, value: ""}, \
// RUN: {key: bugprone-easily-swappable-parameters.QualifiersMix, value: 0}, \
-// RUN: {key: bugprone-easily-swappable-parameters.ModelImplicitConversions, value: 1} \
+// RUN: {key: bugprone-easily-swappable-parameters.ModelImplicitConversions, value: 1}, \
+// RUN: {key: bugprone-easily-swappable-parameters.SuppressParametersUsedTogether, value: 0} \
// RUN: ]}' --
void implicitDoesntBreakOtherStuff(int A, int B) {}
diff --git a/clang-tools-extra/test/clang-tidy/checkers/bugprone-easily-swappable-parameters-implicits.cpp b/clang-tools-extra/test/clang-tidy/checkers/bugprone-easily-swappable-parameters-implicits.cpp
index 7205d87a41e78..4481b516b65b6 100644
--- a/clang-tools-extra/test/clang-tidy/checkers/bugprone-easily-swappable-parameters-implicits.cpp
+++ b/clang-tools-extra/test/clang-tidy/checkers/bugprone-easily-swappable-parameters-implicits.cpp
@@ -4,7 +4,8 @@
// RUN: {key: bugprone-easily-swappable-parameters.IgnoredParameterNames, value: ""}, \
// RUN: {key: bugprone-easily-swappable-parameters.IgnoredParameterTypeSuffixes, value: ""}, \
// RUN: {key: bugprone-easily-swappable-parameters.QualifiersMix, value: 0}, \
-// RUN: {key: bugprone-easily-swappable-parameters.ModelImplicitConversions, value: 1} \
+// RUN: {key: bugprone-easily-swappable-parameters.ModelImplicitConversions, value: 1}, \
+// RUN: {key: bugprone-easily-swappable-parameters.SuppressParametersUsedTogether, value: 0} \
// RUN: ]}' --
void implicitDoesntBreakOtherStuff(int A, int B) {}
diff --git a/clang-tools-extra/test/clang-tidy/checkers/bugprone-easily-swappable-parameters-len2.cpp b/clang-tools-extra/test/clang-tidy/checkers/bugprone-easily-swappable-parameters-len2.cpp
index 3bcdee2e49796..ceb870cda3aac 100644
--- a/clang-tools-extra/test/clang-tidy/checkers/bugprone-easily-swappable-parameters-len2.cpp
+++ b/clang-tools-extra/test/clang-tidy/checkers/bugprone-easily-swappable-parameters-len2.cpp
@@ -4,7 +4,8 @@
// RUN: {key: bugprone-easily-swappable-parameters.IgnoredParameterNames, value: ""}, \
// RUN: {key: bugprone-easily-swappable-parameters.IgnoredParameterTypeSuffixes, value: ""}, \
// RUN: {key: bugprone-easily-swappable-parameters.QualifiersMix, value: 0}, \
-// RUN: {key: bugprone-easily-swappable-parameters.ModelImplicitConversions, value: 0} \
+// RUN: {key: bugprone-easily-swappable-parameters.ModelImplicitConversions, value: 0}, \
+// RUN: {key: bugprone-easily-swappable-parameters.SuppressParametersUsedTogether, value: 0} \
// RUN: ]}' --
namespace std {
diff --git a/clang-tools-extra/test/clang-tidy/checkers/bugprone-easily-swappable-parameters-len3.cpp b/clang-tools-extra/test/clang-tidy/checkers/bugprone-easily-swappable-parameters-len3.cpp
index 4b1f086aba948..bae1cf883c3c6 100644
--- a/clang-tools-extra/test/clang-tidy/checkers/bugprone-easily-swappable-parameters-len3.cpp
+++ b/clang-tools-extra/test/clang-tidy/checkers/bugprone-easily-swappable-parameters-len3.cpp
@@ -4,7 +4,8 @@
// RUN: {key: bugprone-easily-swappable-parameters.IgnoredParameterNames, value: ""}, \
// RUN: {key: bugprone-easily-swappable-parameters.IgnoredParameterTypeSuffixes, value: ""}, \
// RUN: {key: bugprone-easily-swappable-parameters.QualifiersMix, value: 0}, \
-// RUN: {key: bugprone-easily-swappable-parameters.ModelImplicitConversions, value: 0} \
+// RUN: {key: bugprone-easily-swappable-parameters.ModelImplicitConversions, value: 0}, \
+// RUN: {key: bugprone-easily-swappable-parameters.SuppressParametersUsedTogether, value: 0} \
// RUN: ]}' --
int add(int Left, int Right) { return Left + Right; } // NO-WARN: Only 2 parameters.
diff --git a/clang-tools-extra/test/clang-tidy/checkers/bugprone-easily-swappable-parameters-qualifiermixing.cpp b/clang-tools-extra/test/clang-tidy/checkers/bugprone-easily-swappable-parameters-qualifiermixing.cpp
index 02d41661802d2..8b547850ae639 100644
--- a/clang-tools-extra/test/clang-tidy/checkers/bugprone-easily-swappable-parameters-qualifiermixing.cpp
+++ b/clang-tools-extra/test/clang-tidy/checkers/bugprone-easily-swappable-parameters-qualifiermixing.cpp
@@ -4,7 +4,8 @@
// RUN: {key: bugprone-easily-swappable-parameters.IgnoredParameterNames, value: ""}, \
// RUN: {key: bugprone-easily-swappable-parameters.IgnoredParameterTypeSuffixes, value: ""}, \
// RUN: {key: bugprone-easily-swappable-parameters.QualifiersMix, value: 1}, \
-// RUN: {key: bugprone-easily-swappable-parameters.ModelImplicitConversions, value: 0} \
+// RUN: {key: bugprone-easily-swappable-parameters.ModelImplicitConversions, value: 0}, \
+// RUN: {key: bugprone-easily-swappable-parameters.SuppressParametersUsedTogether, value: 0} \
// RUN: ]}' --
typedef int MyInt1;
diff --git a/clang-tools-extra/test/clang-tidy/checkers/bugprone-easily-swappable-parameters-relatedness.c b/clang-tools-extra/test/clang-tidy/checkers/bugprone-easily-swappable-parameters-relatedness.c
new file mode 100644
index 0000000000000..f1d4e15ff6c8c
--- /dev/null
+++ b/clang-tools-extra/test/clang-tidy/checkers/bugprone-easily-swappable-parameters-relatedness.c
@@ -0,0 +1,30 @@
+// RUN: %check_clang_tidy %s bugprone-easily-swappable-parameters %t \
+// RUN: -config='{CheckOptions: [ \
+// RUN: {key: bugprone-easily-swappable-parameters.MinimumLength, value: 2}, \
+// RUN: {key: bugprone-easily-swappable-parameters.IgnoredParameterNames, value: ""}, \
+// RUN: {key: bugprone-easily-swappable-parameters.IgnoredParameterTypeSuffixes, value: ""}, \
+// RUN: {key: bugprone-easily-swappable-parameters.QualifiersMix, value: 0}, \
+// RUN: {key: bugprone-easily-swappable-parameters.ModelImplicitConversions, value: 0}, \
+// RUN: {key: bugprone-easily-swappable-parameters.SuppressParametersUsedTogether, value: 1} \
+// RUN: ]}' -- -x c
+
+int myprint();
+int add(int X, int Y);
+
+void notRelated(int A, int B) {}
+// CHECK-MESSAGES: :[[@LINE-1]]:17: warning: 2 adjacent parameters of 'notRelated' of similar type ('int')
+// CHECK-MESSAGES: :[[@LINE-2]]:21: note: the first parameter in the range is 'A'
+// CHECK-MESSAGES: :[[@LINE-3]]:28: note: the last parameter in the range is 'B'
+
+int addedTogether(int A, int B) { return add(A, B); } // NO-WARN: Passed to same function.
+
+void passedToSameKNRFunction(int A, int B) {
+ myprint("foo", A);
+ myprint("bar", B);
+}
+// CHECK-MESSAGES: :[[@LINE-4]]:30: warning: 2 adjacent parameters of 'passedToSameKNRFunction' of similar type ('int')
+// CHECK-MESSAGES: :[[@LINE-5]]:34: note: the first parameter in the range is 'A'
+// CHECK-MESSAGES: :[[@LINE-6]]:41: note: the last parameter in the range is 'B'
+// This is actually a false positive: the "passed to same function" heuristic
+// can't map the parameter index 1 to A and B because myprint() has no
+// parameters.
diff --git a/clang-tools-extra/test/clang-tidy/checkers/bugprone-easily-swappable-parameters-relatedness.cpp b/clang-tools-extra/test/clang-tidy/checkers/bugprone-easily-swappable-parameters-relatedness.cpp
new file mode 100644
index 0000000000000..e6fae124aad3f
--- /dev/null
+++ b/clang-tools-extra/test/clang-tidy/checkers/bugprone-easily-swappable-parameters-relatedness.cpp
@@ -0,0 +1,231 @@
+// RUN: %check_clang_tidy %s bugprone-easily-swappable-parameters %t \
+// RUN: -config='{CheckOptions: [ \
+// RUN: {key: bugprone-easily-swappable-parameters.MinimumLength, value: 2}, \
+// RUN: {key: bugprone-easily-swappable-parameters.IgnoredParameterNames, value: ""}, \
+// RUN: {key: bugprone-easily-swappable-parameters.IgnoredParameterTypeSuffixes, value: ""}, \
+// RUN: {key: bugprone-easily-swappable-parameters.QualifiersMix, value: 0}, \
+// RUN: {key: bugprone-easily-swappable-parameters.ModelImplicitConversions, value: 0}, \
+// RUN: {key: bugprone-easily-swappable-parameters.SuppressParametersUsedTogether, value: 1} \
+// RUN: ]}' --
+
+namespace std {
+template <typename T>
+T max(const T &A, const T &B);
+} // namespace std
+
+bool coin();
+void f(int);
+void g(int);
+void h(int, int);
+void i(int, bool);
+void i(int, char);
+
+struct Tmp {
+ int f(int);
+ int g(int, int);
+};
+
+struct Int {
+ int I;
+};
+
+void compare(int Left, int Right) {}
+// CHECK-MESSAGES: :[[@LINE-1]]:14: warning: 2 adjacent parameters of 'compare' of similar type ('int')
+// CHECK-MESSAGES: :[[@LINE-2]]:18: note: the first parameter in the range is 'Left'
+// CHECK-MESSAGES: :[[@LINE-3]]:28: note: the last parameter in the range is 'Right'
+
+int decideSequence(int A, int B) {
+ if (A)
+ return 1;
+ if (B)
+ return 2;
+ return 3;
+}
+// CHECK-MESSAGES: :[[@LINE-7]]:20: warning: 2 adjacent parameters of 'decideSequence' of similar type ('int')
+// CHECK-MESSAGES: :[[@LINE-8]]:24: note: the first parameter in the range is 'A'
+// CHECK-MESSAGES: :[[@LINE-9]]:31: note: the last parameter in the range is 'B'
+
+int myMax(int A, int B) { // NO-WARN: Appears in same expression.
+ return A < B ? A : B;
+}
+
+int myMax2(int A, int B) { // NO-WARN: Appears in same expression.
+ if (A < B)
+ return A;
+ return B;
+}
+
+int myMax3(int A, int B) { // NO-WARN: Appears in same expression.
+ return std::max(A, B);
+}
+
+int binaryToUnary(int A, int) {
+ return A;
+}
+// CHECK-MESSAGES: :[[@LINE-3]]:19: warning: 2 adjacent parameters of 'binaryToUnary' of similar type ('int')
+// CHECK-MESSAGES: :[[@LINE-4]]:23: note: the first parameter in the range is 'A'
+// CHECK-MESSAGES: :[[@LINE-5]]:29: note: the last parameter in the range is '<unnamed>'
+
+int randomReturn1(int A, int B) { // NO-WARN: Appears in same expression.
+ return coin() ? A : B;
+}
+
+int randomReturn2(int A, int B) { // NO-WARN: Both parameters returned.
+ if (coin())
+ return A;
+ return B;
+}
+
+int randomReturn3(int A, int B) { // NO-WARN: Both parameters returned.
+ bool Flip = coin();
+ if (Flip)
+ return A;
+ Flip = coin();
+ if (Flip)
+ return B;
+ Flip = coin();
+ if (!Flip)
+ return 0;
+ return -1;
+}
+
+void passthrough1(int A, int B) { // WARN: Different functions,
diff erent params.
+ f(A);
+ g(B);
+}
+// CHECK-MESSAGES: :[[@LINE-4]]:19: warning: 2 adjacent parameters of 'passthrough1' of similar type ('int')
+// CHECK-MESSAGES: :[[@LINE-5]]:23: note: the first parameter in the range is 'A'
+// CHECK-MESSAGES: :[[@LINE-6]]:30: note: the last parameter in the range is 'B'
+
+void passthrough2(int A, int B) { // NO-WARN: Passed to same index of same function.
+ f(A);
+ f(B);
+}
+
+void passthrough3(int A, int B) { // NO-WARN: Passed to same index of same funtion.
+ h(1, A);
+ h(1, B);
+}
+
+void passthrough4(int A, int B) { // WARN: Different index used.
+ h(1, A);
+ h(B, 2);
+}
+// CHECK-MESSAGES: :[[@LINE-4]]:19: warning: 2 adjacent parameters of 'passthrough4' of similar type ('int')
+// CHECK-MESSAGES: :[[@LINE-5]]:23: note: the first parameter in the range is 'A'
+// CHECK-MESSAGES: :[[@LINE-6]]:30: note: the last parameter in the range is 'B'
+
+void passthrough5(int A, int B) { // WARN: Different function overload.
+ i(A, false);
+ i(A, '\0');
+}
+// CHECK-MESSAGES: :[[@LINE-4]]:19: warning: 2 adjacent parameters of 'passthrough5' of similar type ('int')
+// CHECK-MESSAGES: :[[@LINE-5]]:23: note: the first parameter in the range is 'A'
+// CHECK-MESSAGES: :[[@LINE-6]]:30: note: the last parameter in the range is 'B'
+
+void passthrough6(int A, int B) { // NO-WARN: Passed to same index of same function.
+ Tmp Temp;
+ Temp.f(A);
+ Temp.f(B);
+}
+
+void passthrough7(int A, int B) { // NO-WARN: Passed to same index of same function.
+ // Clang-Tidy isn't path sensitive, the fact that the two objects we call the
+ // function on is
diff erent is not modelled.
+ Tmp Temp1, Temp2;
+ Temp1.f(A);
+ Temp2.f(B);
+}
+
+void passthrough8(int A, int B) { // WARN: Different functions used.
+ f(A);
+ Tmp{}.f(B);
+}
+// CHECK-MESSAGES: :[[@LINE-4]]:19: warning: 2 adjacent parameters of 'passthrough8' of similar type ('int')
+// CHECK-MESSAGES: :[[@LINE-5]]:23: note: the first parameter in the range is 'A'
+// CHECK-MESSAGES: :[[@LINE-6]]:30: note: the last parameter in the range is 'B'
+
+// Test that the matching of "passed-to-function" is done to the proper node.
+// Put simply, this test should not crash here.
+void forwardDeclared(int X);
+
+void passthrough9(int A, int B) { // NO-WARN: Passed to same index of same function.
+ forwardDeclared(A);
+ forwardDeclared(B);
+}
+
+void forwardDeclared(int X) {}
+
+void passthrough10(int A, int B) { // NO-WARN: Passed to same index of same function.
+ forwardDeclared(A);
+ forwardDeclared(B);
+}
+
+bool compare1(Int I, Int J) { // NO-WARN: Same member accessed.
+ int Val1 = I.I;
+ int Val2 = J.I;
+ return Val1 < Val2;
+}
+
+bool compare2(Tmp T1, Tmp T2) { // NO-WARN: Same member accessed.
+ int Val1 = T1.g(0, 1);
+ int Val2 = T2.g(2, 3);
+ return Val1 < Val2;
+}
+
+bool compare3(Tmp T1, Tmp T2) { // WARN: Different member accessed.
+ int Val1 = T1.f(0);
+ int Val2 = T2.g(1, 2);
+ return Val1 < Val2;
+}
+// CHECK-MESSAGES: :[[@LINE-5]]:15: warning: 2 adjacent parameters of 'compare3' of similar type ('Tmp')
+// CHECK-MESSAGES: :[[@LINE-6]]:19: note: the first parameter in the range is 'T1'
+// CHECK-MESSAGES: :[[@LINE-7]]:27: note: the last parameter in the range is 'T2'
+
+int rangeBreaker(int I, int J, int K, int L, int M, int N) {
+ // (I, J) swappable.
+
+ if (J == K) // (J, K) related.
+ return -1;
+
+ if (K + 2 > Tmp{}.f(K))
+ return M;
+
+ // (K, L, M) swappable.
+
+ return N; // (M, N) related.
+}
+// CHECK-MESSAGES: :[[@LINE-13]]:18: warning: 2 adjacent parameters of 'rangeBreaker' of similar type ('int')
+// CHECK-MESSAGES: :[[@LINE-14]]:22: note: the first parameter in the range is 'I'
+// CHECK-MESSAGES: :[[@LINE-15]]:29: note: the last parameter in the range is 'J'
+// CHECK-MESSAGES: :[[@LINE-16]]:32: warning: 3 adjacent parameters of 'rangeBreaker' of similar type ('int')
+// CHECK-MESSAGES: :[[@LINE-17]]:36: note: the first parameter in the range is 'K'
+// CHECK-MESSAGES: :[[@LINE-18]]:50: note: the last parameter in the range is 'M'
+
+int returnsNotOwnParameter(int I, int J, int K) {
+ const auto &Lambda = [&K](int L, int M, int N) {
+ if (K)
+ return L;
+ return M; // (L, M) related.
+ };
+
+ if (Lambda(-1, 0, 1))
+ return I;
+ return J; // (I, J) related.
+}
+// CHECK-MESSAGES: :[[@LINE-11]]:35: warning: 2 adjacent parameters of 'returnsNotOwnParameter' of similar type ('int')
+// CHECK-MESSAGES: :[[@LINE-12]]:39: note: the first parameter in the range is 'J'
+// CHECK-MESSAGES: :[[@LINE-13]]:46: note: the last parameter in the range is 'K'
+// CHECK-MESSAGES: :[[@LINE-13]]:36: warning: 2 adjacent parameters of 'operator()' of similar type ('int')
+// CHECK-MESSAGES: :[[@LINE-14]]:40: note: the first parameter in the range is 'M'
+// CHECK-MESSAGES: :[[@LINE-15]]:47: note: the last parameter in the range is 'N'
+
+int usedTogetherInCapture(int I, int J, int K) { // NO-WARN: Used together.
+ const auto &Lambda = [I, J, K]() {
+ int A = I + 1;
+ int B = J - 2;
+ int C = K * 3;
+ return A + B + C;
+ };
+ return Lambda();
+}
diff --git a/clang-tools-extra/test/clang-tidy/checkers/bugprone-easily-swappable-parameters.c b/clang-tools-extra/test/clang-tidy/checkers/bugprone-easily-swappable-parameters.c
index 06a3993472ae6..a0b3b52188906 100644
--- a/clang-tools-extra/test/clang-tidy/checkers/bugprone-easily-swappable-parameters.c
+++ b/clang-tools-extra/test/clang-tidy/checkers/bugprone-easily-swappable-parameters.c
@@ -4,7 +4,8 @@
// RUN: {key: bugprone-easily-swappable-parameters.IgnoredParameterNames, value: ""}, \
// RUN: {key: bugprone-easily-swappable-parameters.IgnoredParameterTypeSuffixes, value: "bool;MyBool;struct U;MAKE_LOGICAL_TYPE(int)"}, \
// RUN: {key: bugprone-easily-swappable-parameters.QualifiersMix, value: 0}, \
-// RUN: {key: bugprone-easily-swappable-parameters.ModelImplicitConversions, value: 0} \
+// RUN: {key: bugprone-easily-swappable-parameters.ModelImplicitConversions, value: 0}, \
+// RUN: {key: bugprone-easily-swappable-parameters.SuppressParametersUsedTogether, value: 0} \
// RUN: ]}' -- -x c
#define bool _Bool
More information about the cfe-commits
mailing list