-
Notifications
You must be signed in to change notification settings - Fork 15.2k
[clang-tidy] Rename 'cert-dcl50-cpp' to 'modernize-avoid-variadic-functions' #157737
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Conversation
@llvm/pr-subscribers-clang-tidy @llvm/pr-subscribers-clang-tools-extra Author: Baranov Victor (vbvictor) ChangesChose Closes #157301. Full diff: https://github.com/llvm/llvm-project/pull/157737.diff 12 Files Affected:
diff --git a/clang-tools-extra/clang-tidy/cert/CERTTidyModule.cpp b/clang-tools-extra/clang-tidy/cert/CERTTidyModule.cpp
index a0d0ac1007c3e..8bb645f46e38d 100644
--- a/clang-tools-extra/clang-tidy/cert/CERTTidyModule.cpp
+++ b/clang-tools-extra/clang-tidy/cert/CERTTidyModule.cpp
@@ -27,6 +27,7 @@
#include "../misc/NonCopyableObjects.h"
#include "../misc/StaticAssertCheck.h"
#include "../misc/ThrowByValueCatchByReferenceCheck.h"
+#include "../modernize/AvoidVariadicFunctionsCheck.h"
#include "../performance/MoveConstructorInitCheck.h"
#include "../readability/EnumInitialValueCheck.h"
#include "../readability/UppercaseLiteralSuffixCheck.h"
@@ -41,7 +42,6 @@
#include "SetLongJmpCheck.h"
#include "StaticObjectExceptionCheck.h"
#include "ThrownExceptionTypeCheck.h"
-#include "VariadicFunctionDefCheck.h"
namespace {
@@ -245,7 +245,8 @@ class CERTModule : public ClangTidyModule {
.registerCheck<bugprone::PointerArithmeticOnPolymorphicObjectCheck>(
"cert-ctr56-cpp");
// DCL
- CheckFactories.registerCheck<VariadicFunctionDefCheck>("cert-dcl50-cpp");
+ CheckFactories.registerCheck<modernize::AvoidVariadicFunctionsCheck>(
+ "cert-dcl50-cpp");
CheckFactories.registerCheck<bugprone::ReservedIdentifierCheck>(
"cert-dcl51-cpp");
CheckFactories.registerCheck<misc::NewDeleteOverloadsCheck>(
diff --git a/clang-tools-extra/clang-tidy/cert/CMakeLists.txt b/clang-tools-extra/clang-tidy/cert/CMakeLists.txt
index eebbf907cc94e..6bbd6856a09bb 100644
--- a/clang-tools-extra/clang-tidy/cert/CMakeLists.txt
+++ b/clang-tools-extra/clang-tidy/cert/CMakeLists.txt
@@ -16,7 +16,6 @@ add_clang_library(clangTidyCERTModule STATIC
SetLongJmpCheck.cpp
StaticObjectExceptionCheck.cpp
ThrownExceptionTypeCheck.cpp
- VariadicFunctionDefCheck.cpp
LINK_LIBS
clangTidy
@@ -24,6 +23,7 @@ add_clang_library(clangTidyCERTModule STATIC
clangTidyConcurrencyModule
clangTidyGoogleModule
clangTidyMiscModule
+ clangTidyModernizeModule
clangTidyPerformanceModule
clangTidyReadabilityModule
clangTidyUtils
diff --git a/clang-tools-extra/clang-tidy/cert/VariadicFunctionDefCheck.h b/clang-tools-extra/clang-tidy/cert/VariadicFunctionDefCheck.h
deleted file mode 100644
index a082e370c3228..0000000000000
--- a/clang-tools-extra/clang-tidy/cert/VariadicFunctionDefCheck.h
+++ /dev/null
@@ -1,33 +0,0 @@
-//===--- VariadicFunctionDefCheck.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_CERT_VARIADICFUNCTIONDEF_H
-#define LLVM_CLANG_TOOLS_EXTRA_CLANG_TIDY_CERT_VARIADICFUNCTIONDEF_H
-
-#include "../ClangTidyCheck.h"
-
-namespace clang::tidy::cert {
-
-/// Guards against any C-style variadic function definitions (not declarations).
-///
-/// For the user-facing documentation see:
-/// http://clang.llvm.org/extra/clang-tidy/checks/cert/dcl50-cpp.html
-class VariadicFunctionDefCheck : public ClangTidyCheck {
-public:
- VariadicFunctionDefCheck(StringRef Name, ClangTidyContext *Context)
- : ClangTidyCheck(Name, Context) {}
- bool isLanguageVersionSupported(const LangOptions &LangOpts) const override {
- return LangOpts.CPlusPlus;
- }
- void registerMatchers(ast_matchers::MatchFinder *Finder) override;
- void check(const ast_matchers::MatchFinder::MatchResult &Result) override;
-};
-
-} // namespace clang::tidy::cert
-
-#endif // LLVM_CLANG_TOOLS_EXTRA_CLANG_TIDY_CERT_VARIADICFUNCTIONDEF_H
diff --git a/clang-tools-extra/clang-tidy/cert/VariadicFunctionDefCheck.cpp b/clang-tools-extra/clang-tidy/modernize/AvoidVariadicFunctionsCheck.cpp
similarity index 71%
rename from clang-tools-extra/clang-tidy/cert/VariadicFunctionDefCheck.cpp
rename to clang-tools-extra/clang-tidy/modernize/AvoidVariadicFunctionsCheck.cpp
index 5fba32417db42..f8f34e13cfdb2 100644
--- a/clang-tools-extra/clang-tidy/cert/VariadicFunctionDefCheck.cpp
+++ b/clang-tools-extra/clang-tidy/modernize/AvoidVariadicFunctionsCheck.cpp
@@ -1,4 +1,4 @@
-//===-- VariadicFunctionDefCheck.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.
@@ -6,14 +6,14 @@
//
//===----------------------------------------------------------------------===//
-#include "VariadicFunctionDefCheck.h"
+#include "AvoidVariadicFunctionsCheck.h"
#include "clang/ASTMatchers/ASTMatchFinder.h"
using namespace clang::ast_matchers;
-namespace clang::tidy::cert {
+namespace clang::tidy::modernize {
-void VariadicFunctionDefCheck::registerMatchers(MatchFinder *Finder) {
+void AvoidVariadicFunctionsCheck::registerMatchers(MatchFinder *Finder) {
// We only care about function *definitions* that are variadic, and do not
// have extern "C" language linkage.
Finder->addMatcher(
@@ -22,7 +22,8 @@ void VariadicFunctionDefCheck::registerMatchers(MatchFinder *Finder) {
this);
}
-void VariadicFunctionDefCheck::check(const MatchFinder::MatchResult &Result) {
+void AvoidVariadicFunctionsCheck::check(
+ const MatchFinder::MatchResult &Result) {
const auto *FD = Result.Nodes.getNodeAs<FunctionDecl>("func");
diag(FD->getLocation(),
@@ -30,4 +31,4 @@ void VariadicFunctionDefCheck::check(const MatchFinder::MatchResult &Result) {
"parameter pack or currying instead");
}
-} // namespace clang::tidy::cert
+} // namespace clang::tidy::modernize
diff --git a/clang-tools-extra/clang-tidy/modernize/AvoidVariadicFunctionsCheck.h b/clang-tools-extra/clang-tidy/modernize/AvoidVariadicFunctionsCheck.h
new file mode 100644
index 0000000000000..d93e9d0c5d678
--- /dev/null
+++ b/clang-tools-extra/clang-tidy/modernize/AvoidVariadicFunctionsCheck.h
@@ -0,0 +1,33 @@
+//===----------------------------------------------------------------------===//
+//
+// 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_MODERNIZE_AVOIDVARIADICFUNCTIONSCHECK_H
+#define LLVM_CLANG_TOOLS_EXTRA_CLANG_TIDY_MODERNIZE_AVOIDVARIADICFUNCTIONSCHECK_H
+
+#include "../ClangTidyCheck.h"
+
+namespace clang::tidy::modernize {
+
+/// Find all function definitions of C-style variadic functions.
+///
+/// For the user-facing documentation see:
+/// http://clang.llvm.org/extra/clang-tidy/checks/modernize/avoid-variadic-functions.html
+class AvoidVariadicFunctionsCheck : public ClangTidyCheck {
+public:
+ AvoidVariadicFunctionsCheck(StringRef Name, ClangTidyContext *Context)
+ : ClangTidyCheck(Name, Context) {}
+ bool isLanguageVersionSupported(const LangOptions &LangOpts) const override {
+ return LangOpts.CPlusPlus;
+ }
+ void registerMatchers(ast_matchers::MatchFinder *Finder) override;
+ void check(const ast_matchers::MatchFinder::MatchResult &Result) override;
+};
+
+} // namespace clang::tidy::modernize
+
+#endif // LLVM_CLANG_TOOLS_EXTRA_CLANG_TIDY_MODERNIZE_AVOIDVARIADICFUNCTIONSCHECK_H
diff --git a/clang-tools-extra/clang-tidy/modernize/CMakeLists.txt b/clang-tools-extra/clang-tidy/modernize/CMakeLists.txt
index 619a27b2f9bb6..bee6bb2b1d5df 100644
--- a/clang-tools-extra/clang-tidy/modernize/CMakeLists.txt
+++ b/clang-tools-extra/clang-tidy/modernize/CMakeLists.txt
@@ -6,6 +6,7 @@ set(LLVM_LINK_COMPONENTS
add_clang_library(clangTidyModernizeModule STATIC
AvoidBindCheck.cpp
AvoidCArraysCheck.cpp
+ AvoidVariadicFunctionsCheck.cpp
ConcatNestedNamespacesCheck.cpp
DeprecatedHeadersCheck.cpp
DeprecatedIosBaseAliasesCheck.cpp
diff --git a/clang-tools-extra/clang-tidy/modernize/ModernizeTidyModule.cpp b/clang-tools-extra/clang-tidy/modernize/ModernizeTidyModule.cpp
index fdf38bc4b6308..8811d52ce5448 100644
--- a/clang-tools-extra/clang-tidy/modernize/ModernizeTidyModule.cpp
+++ b/clang-tools-extra/clang-tidy/modernize/ModernizeTidyModule.cpp
@@ -11,6 +11,7 @@
#include "../ClangTidyModuleRegistry.h"
#include "AvoidBindCheck.h"
#include "AvoidCArraysCheck.h"
+#include "AvoidVariadicFunctionsCheck.h"
#include "ConcatNestedNamespacesCheck.h"
#include "DeprecatedHeadersCheck.h"
#include "DeprecatedIosBaseAliasesCheck.h"
@@ -63,6 +64,8 @@ class ModernizeModule : public ClangTidyModule {
void addCheckFactories(ClangTidyCheckFactories &CheckFactories) override {
CheckFactories.registerCheck<AvoidBindCheck>("modernize-avoid-bind");
CheckFactories.registerCheck<AvoidCArraysCheck>("modernize-avoid-c-arrays");
+ CheckFactories.registerCheck<AvoidVariadicFunctionsCheck>(
+ "modernize-avoid-variadic-functions");
CheckFactories.registerCheck<ConcatNestedNamespacesCheck>(
"modernize-concat-nested-namespaces");
CheckFactories.registerCheck<DeprecatedHeadersCheck>(
diff --git a/clang-tools-extra/docs/ReleaseNotes.rst b/clang-tools-extra/docs/ReleaseNotes.rst
index 28620a92e4205..b62697ce96c3e 100644
--- a/clang-tools-extra/docs/ReleaseNotes.rst
+++ b/clang-tools-extra/docs/ReleaseNotes.rst
@@ -176,6 +176,11 @@ New checks
New check aliases
^^^^^^^^^^^^^^^^^
+- Renamed :doc:`cert-dcl50-cpp <clang-tidy/checks/cert/dcl50-cpp>` to
+ :doc:`modernize-avoid-variadic-functions
+ <clang-tidy/checks/modernize/avoid-variadic-functions>`
+ keeping initial check as an alias to the new one.
+
- Renamed :doc:`cert-err34-c <clang-tidy/checks/cert/err34-c>` to
:doc:`bugprone-unchecked-string-to-number-conversion
<clang-tidy/checks/bugprone/unchecked-string-to-number-conversion>`
diff --git a/clang-tools-extra/docs/clang-tidy/checks/cert/dcl50-cpp.rst b/clang-tools-extra/docs/clang-tidy/checks/cert/dcl50-cpp.rst
index 5fc1fbfbb58e7..769e104527430 100644
--- a/clang-tools-extra/docs/clang-tidy/checks/cert/dcl50-cpp.rst
+++ b/clang-tools-extra/docs/clang-tidy/checks/cert/dcl50-cpp.rst
@@ -1,11 +1,10 @@
.. title:: clang-tidy - cert-dcl50-cpp
+.. meta::
+ :http-equiv=refresh: 5;URL=../modernize/avoid-variadic-functions.html
cert-dcl50-cpp
==============
-This check flags all function definitions (but not declarations) of C-style
-variadic functions.
-
-This check corresponds to the CERT C++ Coding Standard rule
-`DCL50-CPP. Do not define a C-style variadic function
-<https://www.securecoding.cert.org/confluence/display/cplusplus/DCL50-CPP.+Do+not+define+a+C-style+variadic+function>`_.
+The cert-dcl50-cpp check is an alias, please see
+`modernize-avoid-variadic-functions <../modernize/avoid-variadic-functions.html>`_
+for more information.
diff --git a/clang-tools-extra/docs/clang-tidy/checks/list.rst b/clang-tools-extra/docs/clang-tidy/checks/list.rst
index 89ad491935f7f..c99515ccbac6b 100644
--- a/clang-tools-extra/docs/clang-tidy/checks/list.rst
+++ b/clang-tools-extra/docs/clang-tidy/checks/list.rst
@@ -170,7 +170,6 @@ Clang-Tidy Checks
:doc:`bugprone-unused-return-value <bugprone/unused-return-value>`,
:doc:`bugprone-use-after-move <bugprone/use-after-move>`,
:doc:`bugprone-virtual-near-miss <bugprone/virtual-near-miss>`, "Yes"
- :doc:`cert-dcl50-cpp <cert/dcl50-cpp>`,
:doc:`cert-dcl58-cpp <cert/dcl58-cpp>`,
:doc:`cert-env33-c <cert/env33-c>`,
:doc:`cert-err33-c <cert/err33-c>`,
@@ -286,6 +285,7 @@ Clang-Tidy Checks
:doc:`misc-use-internal-linkage <misc/use-internal-linkage>`, "Yes"
:doc:`modernize-avoid-bind <modernize/avoid-bind>`, "Yes"
:doc:`modernize-avoid-c-arrays <modernize/avoid-c-arrays>`,
+ :doc:`modernize-avoid-variadic-functions <modernize/avoid-variadic-functions>`,
:doc:`modernize-concat-nested-namespaces <modernize/concat-nested-namespaces>`, "Yes"
:doc:`modernize-deprecated-headers <modernize/deprecated-headers>`, "Yes"
:doc:`modernize-deprecated-ios-base-aliases <modernize/deprecated-ios-base-aliases>`, "Yes"
@@ -433,6 +433,7 @@ Check aliases
:doc:`cert-dcl03-c <cert/dcl03-c>`, :doc:`misc-static-assert <misc/static-assert>`, "Yes"
:doc:`cert-dcl16-c <cert/dcl16-c>`, :doc:`readability-uppercase-literal-suffix <readability/uppercase-literal-suffix>`, "Yes"
:doc:`cert-dcl37-c <cert/dcl37-c>`, :doc:`bugprone-reserved-identifier <bugprone/reserved-identifier>`, "Yes"
+ :doc:`cert-dcl50-cpp <cert/dcl50-cpp>`, :doc:`modernize-avoid-variadic-functions <modernize/avoid-variadic-functions>`,
:doc:`cert-dcl51-cpp <cert/dcl51-cpp>`, :doc:`bugprone-reserved-identifier <bugprone/reserved-identifier>`, "Yes"
:doc:`cert-dcl54-cpp <cert/dcl54-cpp>`, :doc:`misc-new-delete-overloads <misc/new-delete-overloads>`,
:doc:`cert-dcl59-cpp <cert/dcl59-cpp>`, :doc:`google-build-namespaces <google/build-namespaces>`,
diff --git a/clang-tools-extra/docs/clang-tidy/checks/modernize/avoid-variadic-functions.rst b/clang-tools-extra/docs/clang-tidy/checks/modernize/avoid-variadic-functions.rst
new file mode 100644
index 0000000000000..c319f7975e039
--- /dev/null
+++ b/clang-tools-extra/docs/clang-tidy/checks/modernize/avoid-variadic-functions.rst
@@ -0,0 +1,17 @@
+.. title:: clang-tidy - modernize-avoid-variadic-functions
+
+modernize-avoid-variadic-functions
+==================================
+
+Find all function definitions (but not declarations) of C-style variadic
+functions.
+
+Instead of C-style variadic functions, C++ function parameter pack or currying
+should be used.
+
+References
+----------
+
+This check corresponds to the CERT C++ Coding Standard rule
+`DCL50-CPP. Do not define a C-style variadic function
+<https://www.securecoding.cert.org/confluence/display/cplusplus/DCL50-CPP.+Do+not+define+a+C-style+variadic+function>`_.
\ No newline at end of file
diff --git a/clang-tools-extra/test/clang-tidy/checkers/cert/variadic-function-def.cpp b/clang-tools-extra/test/clang-tidy/checkers/modernize/avoid-variadic-functions.cpp
similarity index 85%
rename from clang-tools-extra/test/clang-tidy/checkers/cert/variadic-function-def.cpp
rename to clang-tools-extra/test/clang-tidy/checkers/modernize/avoid-variadic-functions.cpp
index 6b2421b82fa4f..5bdc7c644c822 100644
--- a/clang-tools-extra/test/clang-tidy/checkers/cert/variadic-function-def.cpp
+++ b/clang-tools-extra/test/clang-tidy/checkers/modernize/avoid-variadic-functions.cpp
@@ -1,8 +1,8 @@
-// RUN: %check_clang_tidy %s cert-dcl50-cpp %t
+// RUN: %check_clang_tidy %s modernize-avoid-variadic-functions %t
// Variadic function definitions are diagnosed.
void f1(int, ...) {}
-// CHECK-MESSAGES: :[[@LINE-1]]:6: warning: do not define a C-style variadic function; consider using a function parameter pack or currying instead [cert-dcl50-cpp]
+// CHECK-MESSAGES: :[[@LINE-1]]:6: warning: do not define a C-style variadic function; consider using a function parameter pack or currying instead [modernize-avoid-variadic-functions]
// Variadic function *declarations* are not diagnosed.
void f2(int, ...); // ok
|
Find all function definitions (but not declarations) of C-style variadic | ||
functions. | ||
|
||
Instead of C-style variadic functions, C++ function parameter pack or currying |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Enhanced docs a little to give alternatives here and not it clang-tidy
warning.
This check flags all function definitions (but not declarations) of C-style | ||
variadic functions. | ||
|
||
This check corresponds to the CERT C++ Coding Standard rule |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I realize now that the link to the CERT guidelines is probably worth keeping. A bit unfortunate however with the automatic jump to the modernize page, but I think that's a separate issue tracked in another ticket.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I'd do this in current PR same as hicpp.
Find all function definitions (but not declarations) of C-style variadic | ||
functions. | ||
|
||
Instead of C-style variadic functions, C++ function parameter pack or currying |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
As a non-native speaker, I have no idea what currying
means :) and I cannot find that as an "official" C++ term in cppreference. I would vote for just removing this bit and keeping only parameter packs.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Currying means making a curry.. nothing more!
It's not widely used in C++ but its idea to transform f(a, b, c ..)
to f(a)(b)(c)..()
. https://en.wikipedia.org/wiki/Currying. It's more used in functional languages.
References | ||
---------- | ||
|
||
This check corresponds to the CERT C++ Coding Standard rule |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
It sounds strange to put CERT references here in a guideline-agnostic check, like I wrote above I think this link fits better in the CERT docs.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I've seen that such references are placed in many current checks
https://clang.llvm.org/extra/clang-tidy/checks/bugprone/spuriously-wake-up-functions.html
https://clang.llvm.org/extra/clang-tidy/checks/bugprone/reserved-identifier.html
https://clang.llvm.org/extra/clang-tidy/checks/bugprone/unhandled-self-assignment.html
https://clang.llvm.org/extra/clang-tidy/checks/bugprone/sizeof-expression.html
I assume almost every general check that has cert
equivalent has such link to cert guidelines. I think it's still useful to have this link because often CERT docs has more examples/justifications why it is bad.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I'd place link to CERT in both docs (hoping it won't become outdated)
@carlosgalvezp, Do you like new doc style? If so I'd rebase and merge |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM
18a429f
to
e9eaea7
Compare
code-lint failure would be fixed in #160014. |
Chose
modernize
section because we already havemodernize-avoid-bind
,modernize-avoid-c-arrays
and this check sound modernize-ish (but next place it could go ismisc
I guess).Closes #157301.