whisperity created this revision.
whisperity added reviewers: aaron.ballman, alexfh, hokein, JonasToth, zporky.
whisperity added projects: clang-tools-extra, clang.
Herald added subscribers: cfe-commits, martong, gamesh411, dkrupp, rnkovacs,
kbarton, nemanjai.
whisperity added a parent revision: D69560: [clang-tidy] Add
'experimental-cppcoreguidelines-avoid-adjacent-arguments-of-the-same-type'
check.
Herald added a subscriber: wuzish.
The basic version of the checker only considers the pseudo-equality between
types of parameters as the predicate on deciding whether or not the two can be
mixed up with each other at a potential call site. This, together with a low
minimum range length requirement, results in an incredibly verbose checker with
report count potentially in the multiple thousands.
This patch aims to mitigate that problem by silencing warnings about adjacent
parameter ranges in which the parameters are //related// to each other.
While this change definitely brings in false negatives (as mixing the arguments
to `max(a, b)` is still a potential issue, but we really can't do better in
that case...), the sheer amount of reduction in the report count should make up
for it. Across multiple projects (from both C and C++), the number of reports
has dropped by **at least 40%**.
This should help to make sure that the dodgiest interfaces are not hidden under
clumps of naively generated reports.
Currently, three relatedness heuristics are implemented, each in a way that it
is easy to, in the future, remove them or add new ones.
- Common expression: if both parameters are found in a common expression
somewhere in the function, they are considered related. This heuristic is an
absolute blast because it deals with arithmetics, comparisons, and forwarding
functions all in one.
- Return: if both parameters are returned on different execution paths of the
function, they are considered related. This deals with switch-like functions.
- Pass to same function: if both parameters are passed to the same function in
the same index, they are considered related. This is a special case of
forwarding. I.e. for `a` and `b` if there exists a call `f(foo, a);` and
`f(bar, b);`, they are deemed related. Thanks to @zporky, our C++ expert, the
idea.
Repository:
rG LLVM Github Monorepo
https://reviews.llvm.org/D78652
Files:
clang-tools-extra/clang-tidy/experimental/CppcoreguidelinesAvoidAdjacentParametersOfTheSameTypeCheck.cpp
clang-tools-extra/clang-tidy/experimental/CppcoreguidelinesAvoidAdjacentParametersOfTheSameTypeCheck.h
clang-tools-extra/test/clang-tidy/checkers/experimental-cppcoreguidelines-avoid-adjacent-parameters-of-the-same-type-cvr-on.cpp
clang-tools-extra/test/clang-tidy/checkers/experimental-cppcoreguidelines-avoid-adjacent-parameters-of-the-same-type-relatedness.cpp
clang-tools-extra/test/clang-tidy/checkers/experimental-cppcoreguidelines-avoid-adjacent-parameters-of-the-same-type.c
Index: clang-tools-extra/test/clang-tidy/checkers/experimental-cppcoreguidelines-avoid-adjacent-parameters-of-the-same-type.c
===================================================================
--- clang-tools-extra/test/clang-tidy/checkers/experimental-cppcoreguidelines-avoid-adjacent-parameters-of-the-same-type.c
+++ clang-tools-extra/test/clang-tidy/checkers/experimental-cppcoreguidelines-avoid-adjacent-parameters-of-the-same-type.c
@@ -1,7 +1,8 @@
// RUN: %check_clang_tidy %s \
// RUN: experimental-cppcoreguidelines-avoid-adjacent-parameters-of-the-same-type %t \
// RUN: -config='{CheckOptions: [ \
-// RUN: {key: experimental-cppcoreguidelines-avoid-adjacent-parameters-of-the-same-type.CVRMixPossible, value: 1} \
+// RUN: {key: experimental-cppcoreguidelines-avoid-adjacent-parameters-of-the-same-type.CVRMixPossible, value: 1}, \
+// RUN: {key: experimental-cppcoreguidelines-avoid-adjacent-parameters-of-the-same-type.CheckRelatedness, value: 0} \
// RUN: ]}' --
struct T {};
Index: clang-tools-extra/test/clang-tidy/checkers/experimental-cppcoreguidelines-avoid-adjacent-parameters-of-the-same-type-relatedness.cpp
===================================================================
--- /dev/null
+++ clang-tools-extra/test/clang-tidy/checkers/experimental-cppcoreguidelines-avoid-adjacent-parameters-of-the-same-type-relatedness.cpp
@@ -0,0 +1,187 @@
+// RUN: %check_clang_tidy %s \
+// RUN: experimental-cppcoreguidelines-avoid-adjacent-parameters-of-the-same-type %t \
+// RUN: -config='{CheckOptions: [ \
+// RUN: {key: experimental-cppcoreguidelines-avoid-adjacent-parameters-of-the-same-type.CheckRelatedness, value: 1} \
+// RUN: ]}' --
+
+namespace std {
+// NO-WARN: Not a definition, the user has no chance to fix this!
+int max(int x, int y);
+int abs(int x);
+
+template <typename T>
+bool less(const T &t1, const T &t2);
+} // namespace std
+
+class C {
+public:
+ void foo(int a);
+ void bar(int a, int b);
+};
+
+bool coin();
+int f(int a);
+int g(int b);
+int h(int a, int b);
+
+void myFunTakingTwoInt(int a, int b) {}
+// CHECK-MESSAGES: :[[@LINE-1]]:24: warning: 2 adjacent parameters for 'myFunTakingTwoInt' of similar type ('int') are
+// CHECK-MESSAGES: :[[@LINE-2]]:28: note: the first parameter in this range is 'a'
+// CHECK-MESSAGES: :[[@LINE-3]]:35: note: the last parameter in this range is 'b'
+
+int MyMax(int a, int b) { // NO-WARN: Same expression.
+ return a < b ? a : b;
+}
+
+int MyMax_2(int a, int b) { // NO-WARN: Same expression.
+ if (a < b)
+ return a;
+ return b;
+}
+
+int MyMax_3(int a, int b) { // NO-WARN: Same expression.
+ return std::max(a, b);
+}
+
+int BinaryToUnary(int a, int b) {
+ return a;
+}
+// CHECK-MESSAGES: :[[@LINE-3]]:19: warning: 2 adjacent parameters for 'BinaryToUnary' of similar type ('int') are
+// CHECK-MESSAGES: :[[@LINE-4]]:23: note: the first parameter in this range is 'a'
+// CHECK-MESSAGES: :[[@LINE-5]]:30: note: the last parameter in this range is 'b'
+
+int RandomReturn_1(int a, int b) { // NO-WARN: Same expression.
+ return coin() ? a : b;
+}
+
+int RandomReturn_2(int a, int b) { // NO-WARN: Both vars can be returned.
+ if (coin())
+ return a;
+ else
+ return b;
+}
+
+int RandomReturn_3(int a, int b) { // NO-WARN: Both vars can be returned.
+ bool Flip = coin();
+ if (Flip)
+ return a;
+ Flip = coin();
+ if (Flip)
+ return b;
+ Flip = coin();
+ if (!Flip)
+ return 0;
+ return -1;
+}
+
+void Passthrough_1(int a, int b) {
+ f(a);
+ g(b);
+}
+// CHECK-MESSAGES: :[[@LINE-4]]:20: warning: 2 adjacent parameters for 'Passthrough_1' of similar type ('int') are
+// CHECK-MESSAGES: :[[@LINE-5]]:24: note: the first parameter in this range is 'a'
+// CHECK-MESSAGES: :[[@LINE-6]]:31: note: the last parameter in this range is 'b'
+
+void Passthrough_2(int a, int b) { // NO-WARN: Both passed to same func on same index.
+ f(a);
+ f(b);
+}
+
+void Passthrough_2b(int a, int b) { // NO-WARN: Both passed to same func on same index.
+ g(b);
+ g(a);
+}
+
+void Passthrough_Multifun(int a, int b) { // NO-WARN: Both passed to same func on same index.
+ h(1, a);
+ h(1, b);
+}
+
+void Passthrough_Multifun_b(int a, int b) {
+ h(1, a);
+ h(b, 1);
+}
+// CHECK-MESSAGES: :[[@LINE-4]]:29: warning: 2 adjacent parameters for 'Passthrough_Multifun_b' of similar type ('int') are
+// CHECK-MESSAGES: :[[@LINE-5]]:33: note: the first parameter in this range is 'a'
+// CHECK-MESSAGES: :[[@LINE-6]]:40: note: the last parameter in this range is 'b'
+
+void Passthrough_2_Member(int a, int b) { // NO-WARN: Both passed to same func on same index.
+ C{}.foo(a);
+ C{}.foo(b);
+}
+
+void Passthrough_2b_Member(int a, int b) { // NO-WARN: Both passed to same func on same index.
+ C c;
+ c.foo(b);
+ c.foo(a);
+}
+
+void Passthrough_Multifun_Member(int a, int b) { // NO-WARN: Both passed to same func on same index.
+ C c;
+ C c2;
+
+ c.bar(1, a);
+ c2.bar(1, b);
+}
+
+void Passthrough_Multifun_b_Member(int a, int b) {
+ C{}.bar(1, a);
+ C{}.bar(b, 1);
+}
+// CHECK-MESSAGES: :[[@LINE-4]]:36: warning: 2 adjacent parameters for 'Passthrough_Multifun_b_Member' of similar type ('int') are
+// CHECK-MESSAGES: :[[@LINE-5]]:40: note: the first parameter in this range is 'a'
+// CHECK-MESSAGES: :[[@LINE-6]]:47: note: the last parameter in this range is 'b'
+
+int Assign(int a, int b) {
+ int x = a + b;
+ int y = a * x;
+ return y;
+}
+
+int ManyArgs(int x, int y,
+ int Ret1, int Ret2, // NO-WARN: Ret1 and Ret2 both returned at some point.
+ int Comp1, int Comp2, // NO-WARN: Comp1 and Comp2 compared with each other.
+ int Pass1, int Pass2) {
+ if (Comp1 >= Comp2)
+ return Ret1;
+
+ if (Comp1 < -1337 - y) {
+ int One = f(Pass1);
+ int Two = g(Pass2);
+
+ return One * Two;
+ }
+
+ if (Comp1 * 42 == x)
+ return Ret2;
+}
+// CHECK-MESSAGES: :[[@LINE-17]]:14: warning: 3 adjacent parameters for 'ManyArgs' of similar type ('int') are
+// CHECK-MESSAGES: :[[@LINE-18]]:18: note: the first parameter in this range is 'x'
+// CHECK-MESSAGES: :[[@LINE-18]]:18: note: the last parameter in this range is 'Ret1'
+// CHECK-MESSAGES: :[[@LINE-19]]:24: warning: 2 adjacent parameters for 'ManyArgs' of similar type ('int') are
+// CHECK-MESSAGES: :[[@LINE-20]]:28: note: the first parameter in this range is 'Ret2'
+// CHECK-MESSAGES: :[[@LINE-20]]:18: note: the last parameter in this range is 'Comp1'
+// CHECK-MESSAGES: :[[@LINE-21]]:25: warning: 3 adjacent parameters for 'ManyArgs' of similar type ('int') are
+// CHECK-MESSAGES: :[[@LINE-22]]:29: note: the first parameter in this range is 'Comp2'
+// CHECK-MESSAGES: :[[@LINE-22]]:29: note: the last parameter in this range is 'Pass2'
+
+struct TotallyNotAnEEtherathor {
+ TotallyNotAnEEtherathor &operator++();
+ bool operator!=(const TotallyNotAnEEtherathor &RHS) const;
+ const int &operator*();
+};
+
+bool Within(int Element, TotallyNotAnEEtherathor OneAfterBeforeTheZerothIndex, TotallyNotAnEEtherathor PrePastTheNextOfLast) {
+ // NO-WARN: Same expression (==).
+ while (OneAfterBeforeTheZerothIndex != PrePastTheNextOfLast) {
+ if (*OneAfterBeforeTheZerothIndex == Element)
+ return true;
+ ++OneAfterBeforeTheZerothIndex;
+ }
+ return false;
+}
+
+bool isValidRange(TotallyNotAnEEtherathor East, TotallyNotAnEEtherathor West) {
+ // NO-WARN: Same expression (call).
+ return std::less(East, West);
+}
Index: clang-tools-extra/test/clang-tidy/checkers/experimental-cppcoreguidelines-avoid-adjacent-parameters-of-the-same-type-cvr-on.cpp
===================================================================
--- clang-tools-extra/test/clang-tidy/checkers/experimental-cppcoreguidelines-avoid-adjacent-parameters-of-the-same-type-cvr-on.cpp
+++ clang-tools-extra/test/clang-tidy/checkers/experimental-cppcoreguidelines-avoid-adjacent-parameters-of-the-same-type-cvr-on.cpp
@@ -1,7 +1,8 @@
// RUN: %check_clang_tidy %s \
// RUN: experimental-cppcoreguidelines-avoid-adjacent-parameters-of-the-same-type %t \
// RUN: -config='{CheckOptions: [ \
-// RUN: {key: experimental-cppcoreguidelines-avoid-adjacent-parameters-of-the-same-type.CVRMixPossible, value: 1} \
+// RUN: {key: experimental-cppcoreguidelines-avoid-adjacent-parameters-of-the-same-type.CVRMixPossible, value: 1}, \
+// RUN: {key: experimental-cppcoreguidelines-avoid-adjacent-parameters-of-the-same-type.CheckRelatedness, value: 0} \
// RUN: ]}' --
void library(void *vp, void *vp2, void *vp3, int n, int m);
Index: clang-tools-extra/clang-tidy/experimental/CppcoreguidelinesAvoidAdjacentParametersOfTheSameTypeCheck.h
===================================================================
--- clang-tools-extra/clang-tidy/experimental/CppcoreguidelinesAvoidAdjacentParametersOfTheSameTypeCheck.h
+++ clang-tools-extra/clang-tidy/experimental/CppcoreguidelinesAvoidAdjacentParametersOfTheSameTypeCheck.h
@@ -48,6 +48,10 @@
/// Whether to consider 'T' and 'const T'/'volatile T'/etc. arguments to be
/// possible mixup.
const bool CVRMixPossible;
+
+ /// Whether to check for two parameters to be "related" when deciding the
+ /// mixup.
+ const bool CheckRelatedness;
};
} // namespace experimental
Index: clang-tools-extra/clang-tidy/experimental/CppcoreguidelinesAvoidAdjacentParametersOfTheSameTypeCheck.cpp
===================================================================
--- clang-tools-extra/clang-tidy/experimental/CppcoreguidelinesAvoidAdjacentParametersOfTheSameTypeCheck.cpp
+++ clang-tools-extra/clang-tidy/experimental/CppcoreguidelinesAvoidAdjacentParametersOfTheSameTypeCheck.cpp
@@ -9,8 +9,11 @@
#include "CppcoreguidelinesAvoidAdjacentParametersOfTheSameTypeCheck.h"
#include "../utils/OptionsUtils.h"
#include "clang/AST/ASTContext.h"
+#include "clang/AST/RecursiveASTVisitor.h"
#include "clang/ASTMatchers/ASTMatchFinder.h"
#include "llvm/ADT/BitmaskEnum.h"
+#include "llvm/ADT/DenseMap.h"
+#include "llvm/ADT/SmallPtrSet.h"
#include "llvm/ADT/SmallVector.h"
#include <string>
@@ -86,11 +89,13 @@
static_assert(std::is_trivially_copyable<Mixup>::value,
"keep Mixup trivially copyable!");
+using MixupVec = SmallVector<Mixup, 8>;
+
/// Represents a (closed) range of adjacent parameters that can be mixed up at
/// a call site.
struct MixableAdjacentParamRange {
std::size_t NumParamsChecked = 0;
- llvm::SmallVector<Mixup, 8> Mixups;
+ MixupVec Mixups;
const ParmVarDecl *getFirstParm() const {
// The first element added to the mixup vector has the left hand
@@ -110,6 +115,211 @@
}
};
+AST_MATCHER_FUNCTION(ast_matchers::internal::Matcher<Stmt>, ParamRefExpr) {
+ return expr(ignoringParenImpCasts(ignoringElidableConstructorCall(
+ declRefExpr(to(parmVarDecl().bind("param"))))));
+}
+
+/// Helper class that can be used to decide if two parameters (of the same
+/// function) are "related" in some sense. Related parameters should not be
+/// diagnosed.
+class ParamRelatedness {
+public:
+ ParamRelatedness(const FunctionDecl *F, bool ShouldEnable)
+ : Enabled(ShouldEnable) {
+ if (!Enabled)
+ return;
+
+ SameExpr.emplace(const_cast<FunctionDecl *>(F));
+ Returns.emplace(F);
+ PassToSameFunction.emplace(const_cast<FunctionDecl *>(F));
+ }
+
+ bool operator()(const ParmVarDecl *Param1, const ParmVarDecl *Param2) const {
+ if (!Enabled)
+ // Consider every parameter unrelated.
+ return false;
+
+ if (SameExpr && SameExpr->operator()(Param1, Param2))
+ return true;
+
+ if (Returns && Returns->operator()(Param1, Param2))
+ return true;
+
+ if (PassToSameFunction && PassToSameFunction->operator()(Param1, Param2))
+ return true;
+
+ return false;
+ }
+
+private:
+ const bool Enabled;
+
+ /// Implements a heuristic that can be used to mark two parameters related if
+ /// their usage shares a "reasonably close" parent in the syntax tree - in
+ /// this case, that two parameters are referred in the same expression's tree.
+ class PassedInSameExpr : public RecursiveASTVisitor<PassedInSameExpr> {
+ private:
+ using Base = RecursiveASTVisitor<PassedInSameExpr>;
+
+ public:
+ explicit PassedInSameExpr(FunctionDecl *FD) : Function(FD) {
+ TraverseFunctionDecl(FD);
+ }
+
+ bool operator()(const ParmVarDecl *Param1,
+ const ParmVarDecl *Param2) const {
+ auto P1It = ParentExprsOfParamRefs.find(Param1);
+ auto P2It = ParentExprsOfParamRefs.find(Param2);
+ if (P1It == ParentExprsOfParamRefs.end() ||
+ P2It == ParentExprsOfParamRefs.end())
+ return false;
+
+ for (const auto *P1SetE : P1It->second)
+ if (P2It->second.find(P1SetE) != P2It->second.end())
+ return true;
+
+ return false;
+ }
+
+ bool TraverseDecl(Decl *D) {
+ CurrentRootOfExprOnlyTree = nullptr;
+ return Base::TraverseDecl(D);
+ }
+
+ bool TraverseStmt(Stmt *S, DataRecursionQueue *Queue = nullptr) {
+ if (auto *E = dyn_cast_or_null<Expr>(S)) {
+ bool RootSetByCurrent = false;
+
+ if (!CurrentRootOfExprOnlyTree) {
+ CurrentRootOfExprOnlyTree = E;
+ RootSetByCurrent = true;
+ }
+
+ bool Ret = Base::TraverseStmt(S);
+
+ if (RootSetByCurrent)
+ CurrentRootOfExprOnlyTree = nullptr;
+
+ return Ret;
+ }
+
+ CurrentRootOfExprOnlyTree = nullptr;
+
+ return Base::TraverseStmt(S);
+ }
+
+ bool VisitDeclRefExpr(DeclRefExpr *DRE) {
+ if (!CurrentRootOfExprOnlyTree)
+ return true;
+
+ if (auto *PVD = dyn_cast<ParmVarDecl>(DRE->getDecl()))
+ if (llvm::find(Function->parameters(), PVD))
+ ParentExprsOfParamRefs[PVD].insert(CurrentRootOfExprOnlyTree);
+
+ return true;
+ }
+
+ bool shouldWalkTypesOfTypeLocs() const { return false; }
+
+ private:
+ const FunctionDecl *const Function;
+ const Expr *CurrentRootOfExprOnlyTree = nullptr;
+ llvm::DenseMap<const ParmVarDecl *, llvm::SmallPtrSet<const Expr *, 4>>
+ ParentExprsOfParamRefs;
+ };
+
+ /// Implements a heuristic that considers two parameters related if both of
+ /// them are returned at some point in the function (i.e. the function is a
+ /// "switch" between N params based on a condition).
+ class ReturnStmts {
+ public:
+ explicit ReturnStmts(const FunctionDecl *FD) {
+ auto Matches = match(functionDecl(forEachDescendant(
+ returnStmt(hasReturnValue(ParamRefExpr())))),
+ *FD, FD->getASTContext());
+ for (const auto &Match : Matches) {
+ const auto *ReturnedParam = Match.getNodeAs<ParmVarDecl>("param");
+ assert(ReturnedParam && "Bogus matcher.");
+
+ if (find(FD->parameters(), ReturnedParam) == FD->param_end())
+ // Returning an expr to a ParmVarDecl that isn't parameter of the
+ // function should not be possible, but it's not our responsibility if
+ // Sema mishandled something.
+ 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();
+ }
+
+ private:
+ SmallVector<const ParmVarDecl *, 8> ReturnedParams;
+ };
+
+ /// Implements a heuristic that considers two parameters related if they are
+ /// passed at some point to the same function in a call, at the same index,
+ /// i.e. f(a, b) and f(a, c) passes 'b' and 'c' at the same index (2) to f().
+ class PassToSameFunction {
+ public:
+ explicit PassToSameFunction(FunctionDecl *FD) {
+ auto Matches = match(
+ functionDecl(forEachDescendant(
+ callExpr(forEachArgumentWithParam(
+ ParamRefExpr(), parmVarDecl().bind("passed-to")))
+ .bind("call-expr"))),
+ *FD, FD->getASTContext());
+ for (const auto &Match : Matches) {
+ const auto *CallE = Match.getNodeAs<CallExpr>("call-expr");
+ const auto *PassedTo = Match.getNodeAs<ParmVarDecl>("passed-to");
+ const auto *PassedParam = Match.getNodeAs<ParmVarDecl>("param");
+ assert(CallE && PassedTo && PassedParam && "Bogus matcher.");
+
+ const FunctionDecl *CalledFun = CallE->getDirectCallee();
+ if (!CalledFun)
+ continue;
+
+ int TargetIdx = -1;
+ for (unsigned Idx = 0; Idx < CalledFun->getNumParams(); ++Idx)
+ if (CalledFun->getParamDecl(Idx) == PassedTo)
+ TargetIdx = static_cast<int>(Idx);
+ assert(TargetIdx >= 0 && "Matched passing but didn't find the param?");
+
+ TargetParamsOfFunParams[PassedParam].insert(
+ std::make_pair(CalledFun, TargetIdx));
+ }
+ }
+
+ bool operator()(const ParmVarDecl *Param1,
+ const ParmVarDecl *Param2) const {
+ auto P1It = TargetParamsOfFunParams.find(Param1);
+ auto P2It = TargetParamsOfFunParams.find(Param2);
+ if (P1It == TargetParamsOfFunParams.end() ||
+ P2It == TargetParamsOfFunParams.end())
+ return false;
+
+ for (const auto &P1SetE : P1It->second)
+ if (find(P2It->second, P1SetE) != P2It->second.end())
+ return true;
+
+ return false;
+ }
+
+ private:
+ llvm::DenseMap<const ParmVarDecl *,
+ llvm::SmallSet<std::pair<const FunctionDecl *, unsigned>, 4>>
+ TargetParamsOfFunParams;
+ };
+
+ llvm::Optional<PassedInSameExpr> SameExpr;
+ llvm::Optional<ReturnStmts> Returns;
+ llvm::Optional<PassToSameFunction> PassToSameFunction;
+};
+
} // namespace
/// Returns whether an lvalue reference refers to the same type as T.
@@ -232,7 +442,8 @@
/// index StartIdx.
static MixableAdjacentParamRange BuildMixableParameterRange(
const CppcoreguidelinesAvoidAdjacentParametersOfTheSameTypeCheck &Checker,
- const FunctionDecl *F, unsigned int StartIdx) {
+ const FunctionDecl *F, unsigned int StartIdx,
+ const ParamRelatedness &Relatedness) {
MixableAdjacentParamRange MixRange;
const unsigned int ParamCount = F->getNumParams();
assert(StartIdx < ParamCount && "invalid start index given!");
@@ -251,27 +462,38 @@
// If the next parameter in the range is ignored, break the range.
break;
- bool AnyMixupStored = false;
+ MixupVec MixupsOfIth;
for (unsigned int J = StartIdx; J < I; ++J) {
const ParmVarDecl *Jth = F->getParamDecl(J);
Mixup M{Jth, Ith,
HowPossibleToMixUpAtCallSite(Jth->getType(), Ith->getType(), Ctx,
Checker.CVRMixPossible)};
+
+ if (Relatedness(Ith, Jth)) {
+ // Consider two related parameters to not be possible to mix up
+ // (at the user's request).
+ // In addition, if an argument is related to any of the previous, the
+ // mixing range should be broken.
+ MixupsOfIth.clear();
+ break;
+ }
+
M.sanitise();
assert(M.Flags != MIXUP_Invalid &&
"Bits fell off, result is sentinel value.");
- if (M.Flags != MIXUP_None) {
- MixRange.Mixups.emplace_back(M);
- AnyMixupStored = true;
- }
+ if (M.Flags != MIXUP_None)
+ MixupsOfIth.emplace_back(M);
}
- if (!AnyMixupStored)
+ if (MixupsOfIth.empty())
// If there is no "new" mixup possibility for the Ith param, it signifies
// the end of range.
break;
+ MixRange.Mixups.insert(MixRange.Mixups.end(), MixupsOfIth.begin(),
+ MixupsOfIth.end());
+
// If the loop did not break earlier, another parameter was found to be
// mixable.
++MixRange.NumParamsChecked;
@@ -456,7 +678,8 @@
Options.get("IgnoredNames", DefaultIgnoredParamNames))),
IgnoredParamTypes(utils::options::parseStringList(
Options.get("IgnoredTypes", DefaultIgnoredParamTypes))),
- CVRMixPossible(Options.get("CVRMixPossible", false)) {}
+ CVRMixPossible(Options.get("CVRMixPossible", false)),
+ CheckRelatedness(Options.get("CheckRelatedness", true)) {}
void CppcoreguidelinesAvoidAdjacentParametersOfTheSameTypeCheck::storeOptions(
ClangTidyOptions::OptionMap &Opts) {
@@ -466,6 +689,7 @@
Options.store(Opts, "IgnoredTypes",
utils::options::serializeStringList(IgnoredParamTypes));
Options.store(Opts, "CVRMixPossible", CVRMixPossible);
+ Options.store(Opts, "CheckRelatedness", CheckRelatedness);
}
void CppcoreguidelinesAvoidAdjacentParametersOfTheSameTypeCheck::
@@ -502,6 +726,7 @@
const MatchFinder::MatchResult &Result) {
const auto *Fun = Result.Nodes.getNodeAs<FunctionDecl>("fun");
+ ParamRelatedness RelatednessChecker{Fun, CheckRelatedness};
unsigned int ParamMixRangeStartIdx = 0;
const unsigned int NumArgs = Fun->getNumParams();
@@ -514,7 +739,8 @@
}
MixableAdjacentParamRange MixingRange =
- BuildMixableParameterRange(*this, Fun, ParamMixRangeStartIdx);
+ BuildMixableParameterRange(
+ *this, Fun, ParamMixRangeStartIdx, RelatednessChecker);
assert(MixingRange.NumParamsChecked > 0 && "Ensure continuity!");
ParamMixRangeStartIdx += MixingRange.NumParamsChecked;
if (MixingRange.NumParamsChecked < MinimumLength)
_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
