vingeldal created this revision. Herald added subscribers: cfe-commits, kbarton, mgorny, nemanjai. Herald added a project: clang.
This check is part of the C++ Core Guidelines, rule I.24 https://github.com/vingeldal/CppCoreGuidelines/blob/master/CppCoreGuidelines.md#Ri-unrelated Repository: rG LLVM Github Monorepo https://reviews.llvm.org/D74463 Files: clang-tools-extra/clang-tidy/cppcoreguidelines/AvoidAdjacentParametersOfTheSameTypeCheck.cpp clang-tools-extra/clang-tidy/cppcoreguidelines/AvoidAdjacentParametersOfTheSameTypeCheck.h clang-tools-extra/clang-tidy/cppcoreguidelines/CMakeLists.txt clang-tools-extra/clang-tidy/cppcoreguidelines/CppCoreGuidelinesTidyModule.cpp clang-tools-extra/docs/ReleaseNotes.rst clang-tools-extra/docs/clang-tidy/checks/cppcoreguidelines-avoid-adjacent-parameters-of-the-same-type.rst clang-tools-extra/docs/clang-tidy/checks/list.rst clang-tools-extra/test/clang-tidy/checkers/cppcoreguidelines-avoid-adjacent-parameters-of-the-same-type.cpp
Index: clang-tools-extra/test/clang-tidy/checkers/cppcoreguidelines-avoid-adjacent-parameters-of-the-same-type.cpp =================================================================== --- /dev/null +++ clang-tools-extra/test/clang-tidy/checkers/cppcoreguidelines-avoid-adjacent-parameters-of-the-same-type.cpp @@ -0,0 +1,57 @@ +// RUN: %check_clang_tidy %s cppcoreguidelines-avoid-adjacent-parameters-of-the-same-type %t + +void func_no_parameter(); + +void func_parameter_argument(int arg1); + +void func_int_int_array(int arg1, int arg2[]); + +void func_int_int_reference(int arg1, int &arg2); + +void func_int_int_pointer(int arg1, int *arg2); + +void func_int_const_int(int arg1, const int arg2); + +void func_int_unsigned_int(int arg1, unsigned int arg2); + +void func_int_long_int(int arg1, long int arg2); + +void func_int_float(int arg1, float arg2); + +void func_more_than_two_different(char arg1, int arg2, float arg3); + +enum DummyEnum { first, + second }; +void func_int_and_enum(int arg1, DummyEnum arg2); + +template <typename T> +void func_template(T arg1, int arg2); + +template <typename T> +void func_template_adjacent_int(T arg1, int arg2, int arg3); +// CHECK-MESSAGES: :[[@LINE-1]]:6: warning: function 'func_template_adjacent_int' has adjacent parameters of the same type. If the order of these parameters matter consider rewriting this to avoid a mixup of parameters. [cppcoreguidelines-avoid-adjacent-parameters-of-the-same-type] + +template <typename T> +void func_template_adjacent_template_type(T arg1, T arg2); +// CHECK-MESSAGES: :[[@LINE-1]]:6: warning: function 'func_template_adjacent_template_type' has adjacent parameters of the same type. If the order of these parameters matter consider rewriting this to avoid a mixup of parameters. [cppcoreguidelines-avoid-adjacent-parameters-of-the-same-type] + +void func_two_adjacent_int(int arg1, int arg2); +// CHECK-MESSAGES: :[[@LINE-1]]:6: warning: function 'func_two_adjacent_int' has adjacent parameters of the same type. If the order of these parameters matter consider rewriting this to avoid a mixup of parameters. [cppcoreguidelines-avoid-adjacent-parameters-of-the-same-type] + +void func_adjacent_int_more_than_two(int arg1, int arg2, int int_3); +// CHECK-MESSAGES: :[[@LINE-1]]:6: warning: function 'func_adjacent_int_more_than_two' has adjacent parameters of the same type. If the order of these parameters matter consider rewriting this to avoid a mixup of parameters. [cppcoreguidelines-avoid-adjacent-parameters-of-the-same-type] + +void func_adjacent_int_not_all_same(float arg1, int arg2, int int_3); +// CHECK-MESSAGES: :[[@LINE-1]]:6: warning: function 'func_adjacent_int_not_all_same' has adjacent parameters of the same type. If the order of these parameters matter consider rewriting this to avoid a mixup of parameters. [cppcoreguidelines-avoid-adjacent-parameters-of-the-same-type] + +void func_adjacent_float(float arg1, float arg2); +// CHECK-MESSAGES: :[[@LINE-1]]:6: warning: function 'func_adjacent_float' has adjacent parameters of the same type. If the order of these parameters matter consider rewriting this to avoid a mixup of parameters. [cppcoreguidelines-avoid-adjacent-parameters-of-the-same-type] + +class DummyClass { +public: + void func_member1(); + void func_member2(int arg1, int arg2, float arg3); + static void func_member3(int arg1, int arg2); +}; +// CHECK-MESSAGES: :[[@LINE-3]]:10: warning: function 'func_member2' has adjacent parameters of the same type. If the order of these parameters matter consider rewriting this to avoid a mixup of parameters. [cppcoreguidelines-avoid-adjacent-parameters-of-the-same-type] +// CHECK-MESSAGES: :[[@LINE-3]]:17: warning: function 'func_member3' has adjacent parameters of the same type. If the order of these parameters matter consider rewriting this to avoid a mixup of parameters. [cppcoreguidelines-avoid-adjacent-parameters-of-the-same-type] \ No newline at end of file Index: clang-tools-extra/docs/clang-tidy/checks/list.rst =================================================================== --- clang-tools-extra/docs/clang-tidy/checks/list.rst +++ clang-tools-extra/docs/clang-tidy/checks/list.rst @@ -372,6 +372,7 @@ `clang-analyzer-unix.Vfork <clang-analyzer-unix.Vfork.html>`_, `Clang Static Analyzer <https://clang.llvm.org/docs/analyzer/checkers.html>`_, `clang-analyzer-unix.cstring.BadSizeArg <clang-analyzer-unix.cstring.BadSizeArg.html>`_, `Clang Static Analyzer <https://clang.llvm.org/docs/analyzer/checkers.html>`_, `clang-analyzer-unix.cstring.NullArg <clang-analyzer-unix.cstring.NullArg.html>`_, `Clang Static Analyzer <https://clang.llvm.org/docs/analyzer/checkers.html>`_, + `cppcoreguidelines-avoid-adjacent-parameters-of-the-same-type <cppcoreguidelines-avoid-adjacent-parameters-of-the-same-type.html>`_, `cppcoreguidelines-avoid-c-arrays <cppcoreguidelines-avoid-c-arrays.html>`_, `modernize-avoid-c-arrays <modernize-avoid-c-arrays.html>`_, `cppcoreguidelines-avoid-magic-numbers <cppcoreguidelines-avoid-magic-numbers.html>`_, `readability-magic-numbers <readability-magic-numbers.html>`_, `cppcoreguidelines-c-copy-assignment-signature <cppcoreguidelines-c-copy-assignment-signature.html>`_, `misc-unconventional-assign-operator <misc-unconventional-assign-operator.html>`_, @@ -407,3 +408,4 @@ `hicpp-use-override <hicpp-use-override.html>`_, `modernize-use-override <modernize-use-override.html>`_, "Yes" `hicpp-vararg <hicpp-vararg.html>`_, `cppcoreguidelines-pro-type-vararg <cppcoreguidelines-pro-type-vararg.html>`_, `llvm-qualified-auto <llvm-qualified-auto.html>`_, `readability-qualified-auto <readability-qualified-auto.html>`_, "Yes" + Index: clang-tools-extra/docs/clang-tidy/checks/cppcoreguidelines-avoid-adjacent-parameters-of-the-same-type.rst =================================================================== --- /dev/null +++ clang-tools-extra/docs/clang-tidy/checks/cppcoreguidelines-avoid-adjacent-parameters-of-the-same-type.rst @@ -0,0 +1,28 @@ +.. title:: clang-tidy - cppcoreguidelines-avoid-adjacent-parameters-of-the-same-type + +cppcoreguidelines-avoid-adjacent-parameters-of-the-same-type +============================================================ + +This check implements the C++ Core Guideline `I.24 Avoid adjacent unrelated parameters of the same type <https://github.com/vingeldal/CppCoreGuidelines/blob/master/CppCoreGuidelines.md#i24-avoid-adjacent-unrelated-parameters-of-the-same-type>`. +Adjacent function parameters of the same type, where the order of the parameters is important, are risky. +The user of such a function could easily confuse the order of parameters, causing errors which the compiler can't +possibly detect. + +Consider the following example: + +.. code-block:: c++ + +void copy_n(T* p, T* q, int n); // copy from [p:p + n) to [q:q + n) + +In this case one could make the source a const parameter: + +.. code-block:: c++ + +void copy_n(const T* p, T* q, int n); // copy from [p:p + n) to [q:q + n) + +This check has no way of knowing if the order of parameters matter, so it will give some false positives. +The following example demonstrates a case where there is no risk associated with swapping parameters, because the order doesn't matter. + +.. code-block:: c++ + +int max(int a, int b); \ No newline at end of file Index: clang-tools-extra/docs/ReleaseNotes.rst =================================================================== --- clang-tools-extra/docs/ReleaseNotes.rst +++ clang-tools-extra/docs/ReleaseNotes.rst @@ -69,6 +69,10 @@ New checks ^^^^^^^^^^ +- New :doc:`cppcoreguidelines-avoid-adjacent-parameters-of-the-same-type + <clang-tidy/checks/cppcoreguidelines-avoid-adjacent-parameters-of-the-same-type>` check. + + Finds declarations of functions with adjacent parameters of the same type. - New :doc:`bugprone-misplaced-pointer-arithmetic-in-alloc <clang-tidy/checks/bugprone-misplaced-pointer-arithmetic-in-alloc>` check. Index: clang-tools-extra/clang-tidy/cppcoreguidelines/CppCoreGuidelinesTidyModule.cpp =================================================================== --- clang-tools-extra/clang-tidy/cppcoreguidelines/CppCoreGuidelinesTidyModule.cpp +++ clang-tools-extra/clang-tidy/cppcoreguidelines/CppCoreGuidelinesTidyModule.cpp @@ -14,6 +14,7 @@ #include "../modernize/AvoidCArraysCheck.h" #include "../modernize/UseOverrideCheck.h" #include "../readability/MagicNumbersCheck.h" +#include "AvoidAdjacentParametersOfTheSameTypeCheck.h" #include "AvoidGotoCheck.h" #include "InitVariablesCheck.h" #include "InterfacesGlobalInitCheck.h" @@ -42,6 +43,8 @@ class CppCoreGuidelinesModule : public ClangTidyModule { public: void addCheckFactories(ClangTidyCheckFactories &CheckFactories) override { + CheckFactories.registerCheck<AvoidAdjacentParametersOfTheSameTypeCheck>( + "cppcoreguidelines-avoid-adjacent-parameters-of-the-same-type"); CheckFactories.registerCheck<modernize::AvoidCArraysCheck>( "cppcoreguidelines-avoid-c-arrays"); CheckFactories.registerCheck<AvoidGotoCheck>( Index: clang-tools-extra/clang-tidy/cppcoreguidelines/CMakeLists.txt =================================================================== --- clang-tools-extra/clang-tidy/cppcoreguidelines/CMakeLists.txt +++ clang-tools-extra/clang-tidy/cppcoreguidelines/CMakeLists.txt @@ -1,6 +1,7 @@ set(LLVM_LINK_COMPONENTS support) add_clang_library(clangTidyCppCoreGuidelinesModule + AvoidAdjacentParametersOfTheSameTypeCheck.cpp AvoidGotoCheck.cpp CppCoreGuidelinesTidyModule.cpp InitVariablesCheck.cpp Index: clang-tools-extra/clang-tidy/cppcoreguidelines/AvoidAdjacentParametersOfTheSameTypeCheck.h =================================================================== --- /dev/null +++ clang-tools-extra/clang-tidy/cppcoreguidelines/AvoidAdjacentParametersOfTheSameTypeCheck.h @@ -0,0 +1,40 @@ +//===--- AvoidAdjacentParametersOfTheSameTypeCheck.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_CPPCOREGUIDELINES_AVOIDADJACENTPARAMETERSOFTHESAMETYPECHECK_H +#define LLVM_CLANG_TOOLS_EXTRA_CLANG_TIDY_CPPCOREGUIDELINES_AVOIDADJACENTPARAMETERSOFTHESAMETYPECHECK_H + +#include "../ClangTidyCheck.h" + +namespace clang { +namespace tidy { +namespace cppcoreguidelines { +// FIXME: Change the name of this check to "Avoid adjacent unrelated parameters +// of the same type" +/// Adjacent function parameters of the same type, where the order of the +/// parameters is important, are risky. The user of such a function could easily +/// confuse the order of parameters, causing errors which the compiler can't +/// possibly detect. +/// +/// For the user-facing documentation see: +/// http://clang.llvm.org/extra/clang-tidy/checks/cppcoreguidelines-avoid-adjacent-parameters-of-the-same-type.html +class AvoidAdjacentParametersOfTheSameTypeCheck : public ClangTidyCheck { +public: + AvoidAdjacentParametersOfTheSameTypeCheck(StringRef Name, + ClangTidyContext *Context) + : ClangTidyCheck(Name, Context) {} + void registerMatchers(ast_matchers::MatchFinder *Finder) override; + void check(const ast_matchers::MatchFinder::MatchResult &Result) override; +}; + +} // namespace cppcoreguidelines +} // namespace tidy +} // namespace clang + +#endif // LLVM_CLANG_TOOLS_EXTRA_CLANG_TIDY_CPPCOREGUIDELINES_AVOIDADJACENTPARAMETERSOFTHESAMETYPECHECK_H Index: clang-tools-extra/clang-tidy/cppcoreguidelines/AvoidAdjacentParametersOfTheSameTypeCheck.cpp =================================================================== --- /dev/null +++ clang-tools-extra/clang-tidy/cppcoreguidelines/AvoidAdjacentParametersOfTheSameTypeCheck.cpp @@ -0,0 +1,53 @@ +//===--- AvoidAdjacentParametersOfTheSameTypeCheck.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 "AvoidAdjacentParametersOfTheSameTypeCheck.h" +#include "clang/AST/ASTContext.h" +#include "clang/ASTMatchers/ASTMatchFinder.h" + +using namespace clang::ast_matchers; + +namespace clang { +namespace tidy { +namespace cppcoreguidelines { +// FIXME: Make sure this matcher only accepts functions with minimum 2 +// parameters +void AvoidAdjacentParametersOfTheSameTypeCheck::registerMatchers( + MatchFinder *Finder) { + Finder->addMatcher(functionDecl().bind("multi-parameter-function"), this); +} + +void AvoidAdjacentParametersOfTheSameTypeCheck::check( + const MatchFinder::MatchResult &Result) { + const auto *MatchedDecl = + Result.Nodes.getNodeAs<FunctionDecl>("multi-parameter-function"); + bool adjacentParametersOfTheSameTypeFound = false; + auto parameters = MatchedDecl->parameters(); + auto parameter = parameters.begin(); + auto nextParameter = parameters.begin(); + ++nextParameter; + + for (; parameter < parameters.end(); ++parameter, ++nextParameter) { + if ((*parameter)->getType() == (*nextParameter)->getType()) { + adjacentParametersOfTheSameTypeFound = true; + break; + } + } + + if (adjacentParametersOfTheSameTypeFound) { + diag(MatchedDecl->getLocation(), + "function %0 has adjacent parameters of the same type. If the order " + "of these parameters matter consider rewriting this to avoid a mixup " + "of parameters.") + << MatchedDecl; + } +} + +} // namespace cppcoreguidelines +} // namespace tidy +} // namespace clang
_______________________________________________ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits