Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
5 changes: 3 additions & 2 deletions clang-tools-extra/clang-tidy/cert/CERTTidyModule.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -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"
Expand All @@ -41,7 +42,6 @@
#include "SetLongJmpCheck.h"
#include "StaticObjectExceptionCheck.h"
#include "ThrownExceptionTypeCheck.h"
#include "VariadicFunctionDefCheck.h"

namespace {

Expand Down Expand Up @@ -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>(
Expand Down
2 changes: 1 addition & 1 deletion clang-tools-extra/clang-tidy/cert/CMakeLists.txt
Original file line number Diff line number Diff line change
Expand Up @@ -16,14 +16,14 @@ add_clang_library(clangTidyCERTModule STATIC
SetLongJmpCheck.cpp
StaticObjectExceptionCheck.cpp
ThrownExceptionTypeCheck.cpp
VariadicFunctionDefCheck.cpp

LINK_LIBS
clangTidy
clangTidyBugproneModule
clangTidyConcurrencyModule
clangTidyGoogleModule
clangTidyMiscModule
clangTidyModernizeModule
clangTidyPerformanceModule
clangTidyReadabilityModule
clangTidyUtils
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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(
Expand All @@ -22,12 +22,13 @@ 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(),
"do not define a C-style variadic function; consider using a function "
"parameter pack or currying instead");
}

} // namespace clang::tidy::cert
} // namespace clang::tidy::modernize
Original file line number Diff line number Diff line change
Expand Up @@ -6,20 +6,20 @@
//
//===----------------------------------------------------------------------===//

#ifndef LLVM_CLANG_TOOLS_EXTRA_CLANG_TIDY_CERT_VARIADICFUNCTIONDEF_H
#define LLVM_CLANG_TOOLS_EXTRA_CLANG_TIDY_CERT_VARIADICFUNCTIONDEF_H
#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::cert {
namespace clang::tidy::modernize {

/// Guards against any C-style variadic function definitions (not declarations).
/// Find all function definitions of C-style variadic functions.
///
/// For the user-facing documentation see:
/// http://clang.llvm.org/extra/clang-tidy/checks/cert/dcl50-cpp.html
class VariadicFunctionDefCheck : public ClangTidyCheck {
/// http://clang.llvm.org/extra/clang-tidy/checks/modernize/avoid-variadic-functions.html
class AvoidVariadicFunctionsCheck : public ClangTidyCheck {
public:
VariadicFunctionDefCheck(StringRef Name, ClangTidyContext *Context)
AvoidVariadicFunctionsCheck(StringRef Name, ClangTidyContext *Context)
: ClangTidyCheck(Name, Context) {}
bool isLanguageVersionSupported(const LangOptions &LangOpts) const override {
return LangOpts.CPlusPlus;
Expand All @@ -28,6 +28,6 @@ class VariadicFunctionDefCheck : public ClangTidyCheck {
void check(const ast_matchers::MatchFinder::MatchResult &Result) override;
};

} // namespace clang::tidy::cert
} // namespace clang::tidy::modernize

#endif // LLVM_CLANG_TOOLS_EXTRA_CLANG_TIDY_CERT_VARIADICFUNCTIONDEF_H
#endif // LLVM_CLANG_TOOLS_EXTRA_CLANG_TIDY_MODERNIZE_AVOIDVARIADICFUNCTIONSCHECK_H
1 change: 1 addition & 0 deletions clang-tools-extra/clang-tidy/modernize/CMakeLists.txt
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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"
Expand Down Expand Up @@ -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>(
Expand Down
5 changes: 5 additions & 0 deletions clang-tools-extra/docs/ReleaseNotes.rst
Original file line number Diff line number Diff line change
Expand Up @@ -206,6 +206,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>`
Expand Down
7 changes: 5 additions & 2 deletions clang-tools-extra/docs/clang-tidy/checks/cert/dcl50-cpp.rst
Original file line number Diff line number Diff line change
@@ -1,10 +1,13 @@
.. 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.
The `cert-dcl50-cpp` check is an alias, please see
`modernize-avoid-variadic-functions <../modernize/avoid-variadic-functions.html>`_
for more information.

This check corresponds to the CERT C++ Coding Standard rule
Copy link
Contributor

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.

Copy link
Contributor Author

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.

`DCL50-CPP. Do not define a C-style variadic function
Expand Down
3 changes: 2 additions & 1 deletion clang-tools-extra/docs/clang-tidy/checks/list.rst
Original file line number Diff line number Diff line change
Expand Up @@ -171,7 +171,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>`,
Expand Down Expand Up @@ -288,6 +287,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"
Expand Down Expand Up @@ -435,6 +435,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>`,
Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,18 @@
.. 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 should be
used.


References
----------

This check corresponds to the CERT C++ Coding Standard rule
Copy link
Contributor

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.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Copy link
Contributor Author

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.

Copy link
Contributor Author

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)

`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>`_.
Original file line number Diff line number Diff line change
@@ -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
Expand Down
Loading