llvmbot wrote:
<!--LLVM PR SUMMARY COMMENT--> @llvm/pr-subscribers-clang-tidy Author: None (isuckatcs) <details> <summary>Changes</summary> This check is inspired by a recent [issue](https://bugs.ruby-lang.org/issues/20908) that happened with Ruby. Ruby supports native extensions written in C, and the compilation of those extensions happens on the user side. The source code of the Ruby VM takes advantage of the fact that in C, `void foo()` is a function that can take any number of parameters, however this is no longer true as of C23. As a result users started facing issues when they tried compiling native extensions with C23 flags. I think the check might be useful for libraries or projects that have an exposed API to their users to give a heads-up that working with unprototyped functions is not fully portable. The check detects if unprototyped function types are used in the source code. For example: ```c void foo(); // Bad: unprototyped function declaration void foo(void); // Good: a function declaration that takes no parameters void (*ptr)(); // Bad: pointer to an unprototyped function void (*ptr)(void); // Good: pointer to a function that takes no parameters ``` Before C23 `void foo()` means a function that takes any number of parameters, so the following snippet is valid. ```c // -std=c17 void foo(); int main() { foo(1, 2, 3); return 0; } ``` Starting from C23 however, ``void foo()`` means a function that takes no parameters, so the same snippet is invalid. ```c // -std=c23 void foo(); int main() { foo(1, 2, 3); // ^ error: too many arguments to function call, expected 0, have 3 return 0; } ``` Similarly a pointer to an unprototyped function binds to any function before C23, so the following snippet is considered valid. ```c // -std=c17 void foo(int x, int y); int main() { void (*ptr)() = &foo; return 0; } ``` >From C23 however, the smae pointer will only bind to functions that take no >parameters. ```c // -std=c23 void foo(int x, int y); int main() { void (*ptr)() = &foo; // ^ error: incompatible function pointer types initializing 'void (*)(void)' with an expression of type 'void (*)(int, int)' return 0; } ``` Some codebases might explicitly take advantage of the pre-C23 behavior, but these codebases should also be aware that their solution might not be fully portable between compilers. --- Full diff: https://github.com/llvm/llvm-project/pull/161023.diff 8 Files Affected: - (added) clang-tools-extra/clang-tidy/portability/AvoidUnprototypedFunctionsCheck.cpp (+58) - (added) clang-tools-extra/clang-tidy/portability/AvoidUnprototypedFunctionsCheck.h (+33) - (modified) clang-tools-extra/clang-tidy/portability/CMakeLists.txt (+1) - (modified) clang-tools-extra/clang-tidy/portability/PortabilityTidyModule.cpp (+3) - (modified) clang-tools-extra/docs/ReleaseNotes.rst (+5) - (modified) clang-tools-extra/docs/clang-tidy/checks/list.rst (+1) - (added) clang-tools-extra/docs/clang-tidy/checks/portability/avoid-unprototyped-functions.rst (+73) - (added) clang-tools-extra/test/clang-tidy/checkers/portability/avoid-unprototyped-functions.c (+32) ``````````diff diff --git a/clang-tools-extra/clang-tidy/portability/AvoidUnprototypedFunctionsCheck.cpp b/clang-tools-extra/clang-tidy/portability/AvoidUnprototypedFunctionsCheck.cpp new file mode 100644 index 0000000000000..032986471e3e1 --- /dev/null +++ b/clang-tools-extra/clang-tidy/portability/AvoidUnprototypedFunctionsCheck.cpp @@ -0,0 +1,58 @@ +//===--- AvoidUnprototypedFunctionsCheck.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 "AvoidUnprototypedFunctionsCheck.h" +#include "clang/ASTMatchers/ASTMatchFinder.h" + +using namespace clang::ast_matchers; + +namespace clang::tidy::portability { + +void AvoidUnprototypedFunctionsCheck::registerMatchers(MatchFinder *Finder) { + auto FunctionTypeMatcher = + forEachDescendant(functionType(unless(functionProtoType()))); + Finder->addMatcher(declaratorDecl(FunctionTypeMatcher).bind("declaratorDecl"), + this); + Finder->addMatcher(typedefDecl(FunctionTypeMatcher).bind("typedefDecl"), + this); +} + +void AvoidUnprototypedFunctionsCheck::check( + const MatchFinder::MatchResult &Result) { + if (const auto *MatchedTypedefDecl = + Result.Nodes.getNodeAs<TypedefDecl>("typedefDecl")) { + diag(MatchedTypedefDecl->getLocation(), + "avoid unprototyped functions in typedef; explicitly add a 'void' " + "parameter if the function takes no arguments"); + return; + } + + const auto *MatchedDeclaratorDecl = + Result.Nodes.getNodeAs<Decl>("declaratorDecl"); + if (!MatchedDeclaratorDecl) { + return; + } + + if (const auto *MatchedFunctionDecl = + llvm::dyn_cast<FunctionDecl>(MatchedDeclaratorDecl)) { + if (MatchedFunctionDecl->isMain() || + MatchedFunctionDecl->hasWrittenPrototype()) + return; + + diag(MatchedFunctionDecl->getLocation(), + "avoid unprototyped function declarations; explicitly spell out a " + "single 'void' parameter if the function takes no argument"); + return; + } + + diag(MatchedDeclaratorDecl->getLocation(), + "avoid unprototyped functions in type specifiers; explicitly add a " + "'void' parameter if the function takes no arguments"); +} + +} // namespace clang::tidy::portability diff --git a/clang-tools-extra/clang-tidy/portability/AvoidUnprototypedFunctionsCheck.h b/clang-tools-extra/clang-tidy/portability/AvoidUnprototypedFunctionsCheck.h new file mode 100644 index 0000000000000..fb89dc6526a24 --- /dev/null +++ b/clang-tools-extra/clang-tidy/portability/AvoidUnprototypedFunctionsCheck.h @@ -0,0 +1,33 @@ +//===--- AvoidUnprototypedFunctionsCheck.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_PORTABILITY_AVOIDUNPROTOTYPEDFUNCTIONSCHECK_H +#define LLVM_CLANG_TOOLS_EXTRA_CLANG_TIDY_PORTABILITY_AVOIDUNPROTOTYPEDFUNCTIONSCHECK_H + +#include "../ClangTidyCheck.h" + +namespace clang::tidy::portability { + +/// Checks if unprototyped function types are used in the source code. +/// +/// For the user-facing documentation see: +/// http://clang.llvm.org/extra/clang-tidy/checks/portability/avoid-unprototyped-functions.html +class AvoidUnprototypedFunctionsCheck : public ClangTidyCheck { +public: + AvoidUnprototypedFunctionsCheck(StringRef Name, ClangTidyContext *Context) + : ClangTidyCheck(Name, Context) {} + void registerMatchers(ast_matchers::MatchFinder *Finder) override; + void check(const ast_matchers::MatchFinder::MatchResult &Result) override; + bool isLanguageVersionSupported(const LangOptions &LangOpts) const override { + return !LangOpts.CPlusPlus; + } +}; + +} // namespace clang::tidy::portability + +#endif // LLVM_CLANG_TOOLS_EXTRA_CLANG_TIDY_PORTABILITY_AVOIDUNPROTOTYPEDFUNCTIONSCHECK_H diff --git a/clang-tools-extra/clang-tidy/portability/CMakeLists.txt b/clang-tools-extra/clang-tidy/portability/CMakeLists.txt index 5a38722a61481..451d5685f7df0 100644 --- a/clang-tools-extra/clang-tidy/portability/CMakeLists.txt +++ b/clang-tools-extra/clang-tidy/portability/CMakeLists.txt @@ -5,6 +5,7 @@ set(LLVM_LINK_COMPONENTS ) add_clang_library(clangTidyPortabilityModule STATIC + AvoidUnprototypedFunctionsCheck.cpp PortabilityTidyModule.cpp RestrictSystemIncludesCheck.cpp SIMDIntrinsicsCheck.cpp diff --git a/clang-tools-extra/clang-tidy/portability/PortabilityTidyModule.cpp b/clang-tools-extra/clang-tidy/portability/PortabilityTidyModule.cpp index 316b98b46cf3f..b692b84056afe 100644 --- a/clang-tools-extra/clang-tidy/portability/PortabilityTidyModule.cpp +++ b/clang-tools-extra/clang-tidy/portability/PortabilityTidyModule.cpp @@ -9,6 +9,7 @@ #include "../ClangTidy.h" #include "../ClangTidyModule.h" #include "../ClangTidyModuleRegistry.h" +#include "AvoidUnprototypedFunctionsCheck.h" #include "RestrictSystemIncludesCheck.h" #include "SIMDIntrinsicsCheck.h" #include "StdAllocatorConstCheck.h" @@ -20,6 +21,8 @@ namespace portability { class PortabilityModule : public ClangTidyModule { public: void addCheckFactories(ClangTidyCheckFactories &CheckFactories) override { + CheckFactories.registerCheck<AvoidUnprototypedFunctionsCheck>( + "portability-avoid-unprototyped-functions"); CheckFactories.registerCheck<RestrictSystemIncludesCheck>( "portability-restrict-system-includes"); CheckFactories.registerCheck<SIMDIntrinsicsCheck>( diff --git a/clang-tools-extra/docs/ReleaseNotes.rst b/clang-tools-extra/docs/ReleaseNotes.rst index 8032f0d23f494..d20e1b13b9742 100644 --- a/clang-tools-extra/docs/ReleaseNotes.rst +++ b/clang-tools-extra/docs/ReleaseNotes.rst @@ -136,6 +136,11 @@ New checks Finds unintended character output from ``unsigned char`` and ``signed char`` to an ``ostream``. +- New :doc:`portability-avoid-unprototyped-functions + <clang-tidy/checks/portability/avoid-unprototyped-functions>` check. + + Checks if unprototyped function types are used in the source code. + - New :doc:`readability-ambiguous-smartptr-reset-call <clang-tidy/checks/readability/ambiguous-smartptr-reset-call>` check. diff --git a/clang-tools-extra/docs/clang-tidy/checks/list.rst b/clang-tools-extra/docs/clang-tidy/checks/list.rst index 1ec476eef3420..9e45c06a58a3f 100644 --- a/clang-tools-extra/docs/clang-tidy/checks/list.rst +++ b/clang-tools-extra/docs/clang-tidy/checks/list.rst @@ -351,6 +351,7 @@ Clang-Tidy Checks :doc:`performance-type-promotion-in-math-fn <performance/type-promotion-in-math-fn>`, "Yes" :doc:`performance-unnecessary-copy-initialization <performance/unnecessary-copy-initialization>`, "Yes" :doc:`performance-unnecessary-value-param <performance/unnecessary-value-param>`, "Yes" + :doc:`portability-avoid-unprototyped-functions <portability/avoid-unprototyped-functions>`, :doc:`portability-restrict-system-includes <portability/restrict-system-includes>`, "Yes" :doc:`portability-simd-intrinsics <portability/simd-intrinsics>`, :doc:`portability-std-allocator-const <portability/std-allocator-const>`, diff --git a/clang-tools-extra/docs/clang-tidy/checks/portability/avoid-unprototyped-functions.rst b/clang-tools-extra/docs/clang-tidy/checks/portability/avoid-unprototyped-functions.rst new file mode 100644 index 0000000000000..3d7ed212b61d9 --- /dev/null +++ b/clang-tools-extra/docs/clang-tidy/checks/portability/avoid-unprototyped-functions.rst @@ -0,0 +1,73 @@ +.. title:: clang-tidy - portability-avoid-unprototyped-functions + +portability-avoid-unprototyped-functions +======================================== + +Checks if unprototyped function types are used in the source code. + +For example: + +.. code-block:: c + + void foo(); // Bad: unprototyped function declaration + void foo(void); // Good: a function declaration that takes no parameters + + void (*ptr)(); // Bad: pointer to an unprototyped function + void (*ptr)(void); // Good: pointer to a function that takes no parameters + +Before C23 ``void foo()`` means a function that takes any number of parameters, so the following snippet is valid. + +.. code-block:: c + + // -std=c17 + void foo(); + + int main() { + foo(1, 2, 3); + + return 0; + } + +Starting from C23 however, ``void foo()`` means a function that takes no parameters, so the same snippet is invalid. + +.. code-block:: c + + // -std=c23 + void foo(); + + int main() { + foo(1, 2, 3); + // ^ error: too many arguments to function call, expected 0, have 3 + + return 0; + } + +Similarly a pointer to an unprototyped function binds to any function before C23, so the following snippet is considered valid. + +.. code-block:: c + + // -std=c17 + void foo(int x, int y); + + int main() { + void (*ptr)() = &foo; + + return 0; + } + +From C23 however, the smae pointer will only bind to functions that take no parameters. + +.. code-block:: c + + // -std=c23 + void foo(int x, int y); + + int main() { + void (*ptr)() = &foo; + // ^ error: incompatible function pointer types initializing 'void (*)(void)' with an expression of type 'void (*)(int, int)' + + return 0; + } + +Some codebases might explicitly take advantage of the pre-C23 behavior, but these codebases should also be aware that +their solution might not be fully portable between compilers. diff --git a/clang-tools-extra/test/clang-tidy/checkers/portability/avoid-unprototyped-functions.c b/clang-tools-extra/test/clang-tidy/checkers/portability/avoid-unprototyped-functions.c new file mode 100644 index 0000000000000..0efc029f5c63f --- /dev/null +++ b/clang-tools-extra/test/clang-tidy/checkers/portability/avoid-unprototyped-functions.c @@ -0,0 +1,32 @@ +// RUN: %check_clang_tidy %s portability-avoid-unprototyped-functions %t +struct S { + int (*UnprototypedFunctionPtrField)(); + // CHECK-MESSAGES: :[[@LINE-1]]:9: warning: avoid unprototyped functions in type specifiers; explicitly add a 'void' parameter if the function takes no arguments +}; + +void unprototypedFunction(); +// CHECK-MESSAGES: :[[@LINE-1]]:6: warning: avoid unprototyped function declarations; explicitly spell out a single 'void' parameter if the function takes no argument + +void unprototypedParamter(int (*UnprototypedFunctionPtrParameter)()); +// CHECK-MESSAGES: :[[@LINE-1]]:33: warning: avoid unprototyped functions in type specifiers; explicitly add a 'void' parameter if the function takes no arguments + +void unprototypedVariable() { +// CHECK-MESSAGES: :[[@LINE-1]]:6: warning: avoid unprototyped function declarations; explicitly spell out a single 'void' parameter if the function takes no argument + + int (*UnprototypedFunctionPtrVariable)(); + // CHECK-MESSAGES: :[[@LINE-1]]:9: warning: avoid unprototyped functions in type specifiers; explicitly add a 'void' parameter if the function takes no arguments +} + +typedef int (*UnprototypedFunctionPtrArray[13])(); +// CHECK-MESSAGES: :[[@LINE-1]]:15: warning: avoid unprototyped functions in typedef; explicitly add a 'void' parameter if the function takes no arguments + +void unprototypedTypeAliasParameter(UnprototypedFunctionPtrArray); + +#define ANYARG + +void unprototypedMacroParameter(ANYARG); +// CHECK-MESSAGES: :[[@LINE-1]]:6: warning: avoid unprototyped function declarations; explicitly spell out a single 'void' parameter if the function takes no argument + +void functionWithNoArguments(void); + +int main(); `````````` </details> https://github.com/llvm/llvm-project/pull/161023 _______________________________________________ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
