Skip to content

Conversation

isuckatcs
Copy link
Member

This check is inspired by a recent issue 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:

  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.

  // -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.

  // -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.

  // -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.

  // -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.

@llvmbot
Copy link
Member

llvmbot commented Sep 27, 2025

@llvm/pr-subscribers-clang-tools-extra

@llvm/pr-subscribers-clang-tidy

Author: None (isuckatcs)

Changes

This check is inspired by a recent issue 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:

  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.

  // -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.

  // -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.

  // -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.

  // -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 --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();

@isuckatcs isuckatcs force-pushed the portability-avoid-unprototyped-functions branch from d8fbd2f to 51cf252 Compare September 27, 2025 21:17
@vbvictor
Copy link
Contributor

vbvictor commented Sep 27, 2025

In discussion #142629, @localspook suggested to use -Wstrict-prototypes for such checking, #142629 (comment). Would it work for your case?

@isuckatcs
Copy link
Member Author

isuckatcs commented Sep 27, 2025

Oh, I missed that thread completely.

My concern about -Wstrict-prototypes is that it's in Clang only. Also, IIUC even if it's enabled, we don't warn when the standard is C23, however for portability we should still emit a warning.

I like to think of Clang-Tidy as a standalone linter, which can be used even though the code is being compiled with MSVC or GCC. In GCC -Wstrict-prototypes is not particularly descriptive in it's warnings, as this example showcases it, and in MSVC I'm not aware of any similar warnings.

By having the check in Clang-Tidy, setups that use a different compiler, but also use Clang-Tidy as a linter could still take adavantage of the warning.

@vbvictor
Copy link
Contributor

vbvictor commented Sep 27, 2025

In GCC -Wstrict-prototypes is not particularly descriptive in it's warnings

If you have -Wstrict-prototypes enabled in GCC, clang-tidy can show clang's warning message if you enable check clang-diagnostic-strict-prototypes. https://godbolt.org/z/e9cfsG4EM. But it only works if -Wstrict-prototypes is already enabled in compile_commands.json or other place.

Now I wonder if we could make a flag for clang-tidy to force it to think as if all clang diagnostics are enabled (implicit -Wall or smth), so then we could just write --checks=clang-diagnostic-strict-prototypes and have this check enabled regardless of -Wstrict-prototypes presence. This could be a cool feature to have all clang diagnostics via clang-tidy even the project is build with GCC/MSVC.

@isuckatcs
Copy link
Member Author

isuckatcs commented Sep 27, 2025

Now I wonder if we could make a flag for clang-tidy to force it to think as if all clang diagnostics are enabled

I'd also prefer this, as right now we have no way to suggest fixit hints for a type, because IIUC we have no access to the token location in a type specifier, while -Wstrict-prototypes can do that.

Another issue is emitting the warning on types for C23, becuase in the AST a source spelled void(*)() is resolved as void(*)(void) in the AST, so we lose that information as well.

We already support passing additional flags, like clang-tidy main.c -- -Wstrict-prototypes, so implementing such a feature is probably not difficult.

@carlosgalvezp
Copy link
Contributor

You can probably also add the -Wstrict-prototypes to the ExtraArgs field in the .clang-tidy file.

@vbvictor
Copy link
Contributor

vbvictor commented Sep 28, 2025

You can probably also add the -Wstrict-prototypes to the ExtraArgs field in the .clang-tidy file.

This is a good workaround (maybe not very well-known)
So if you have config like this

# .clang-tidy
Checks: clang-diagnostic-strict-prototypes
ExtraArgs: ["-Wstrict-prototypes", "-W...", ...]

Then you can have clang's output in clang-tidy:

main.c:1:22: warning: a function declaration without a prototype is deprecated in all versions of C [clang-diagnostic-strict-prototypes]
    1 |  void foo(void (*ptr)(), int y);
      |                      ^
      |                       void
main.c:3:11: warning: a function declaration without a prototype is deprecated in all versions of C [clang-diagnostic-strict-prototypes]
    3 |   int main() {
      |           ^
      |            void
main.c:4:16: warning: a function declaration without a prototype is deprecated in all versions of C [clang-diagnostic-strict-prototypes]
    4 |     void (*ptr)() = &foo;
      |                ^
      |                 void

And this would work regardless of host-compiler

@localspook
Copy link
Contributor

Hold up — this wasn't obvious to me (I guess because I've never used ExtraArgs in the config file), but this is genuinely a pretty convenient solution! Expose a clang warning as a clang-tidy check, all without messing with the build system. I think we should document it!

@isuckatcs
Copy link
Member Author

You can probably also add the -Wstrict-prototypes to the ExtraArgs field in the .clang-tidy file.

This still leaves us with the other problem. -Wstrict-prototypes doesn't emit anything with -std=c23, but for portability reasons we should still require the user to write void foo(void) instead of void foo() regardless of the standard.

@isuckatcs
Copy link
Member Author

So if you have config like this

# .clang-tidy
Checks: clang-diagnostic-strict-prototypes
ExtraArgs: ["-Wstrict-prototypes", "-W...", ...]

IIUC the clang-diagnostic-strict-prototypes check on its own doesn't enable the warning, but what if it did?

I think we could tweak how we handle clang-diagnostic-<warning> checks, so --checks=clang-diagnostic-strict-prototypes is enough to enable the warning without having to create a .clang-tidy file with ExtraArgs.

@localspook
Copy link
Contributor

For code that compiles as C23 but doesn't want to use C23 features to stay compatible with earlier versions, clang provides the -Wpre-c23-compat warning. However, it seems that warning doesn't cover your case (it seems to only warn on language features that are invalid in previous versions, not change meaning in previous versions). I wonder if we should extend that warning?

Copy link

github-actions bot commented Sep 28, 2025

✅ With the latest revision this PR passed the C/C++ code linter.

@isuckatcs
Copy link
Member Author

isuckatcs commented Sep 28, 2025

IIUC the clang-diagnostic-strict-prototypes check on its own doesn't enable the warning, but what if it did?

I quickly implemented this idea. WDYT?

The implementation also allows setting multiple flags with only 1 check. E.g.: --checks="clang-additional-diagnostic-vla*" sets -Wvla, -Wvla-cxx-extension, -Wvla-extension, and -Wvla-extension-static-assert as well.

Apparently using only the clang-diagnostic- prefix interferes with the current behavior, hence clang-additional-diagnostic- is used.

-Wpre-c23-compat warning. [...] (it seems to only warn on language features that are invalid in previous versions, not change meaning in previous versions). I wonder if we should extend that warning?

Interesting question TBH. The warning warns on binary literals, but pre C23 code still compiles with binary literals even though it is invalid. I wonder if it happens because there is a GNU extension that allows them, but still the code compiles with -std=c11, not just -std=gnu11. Also, even MSVC seems to accept them.

@isuckatcs isuckatcs force-pushed the portability-avoid-unprototyped-functions branch 2 times, most recently from e81418c to c946c4c Compare September 28, 2025 13:12
AdjustedChecks = AdjustedChecks.drop_front(1);

CachedGlobList Filter(AdjustedChecks);
for (StringRef Flag : clang::DiagnosticIDs::getDiagnosticFlags()) {
Copy link
Contributor

Choose a reason for hiding this comment

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

Suggested change
for (StringRef Flag : clang::DiagnosticIDs::getDiagnosticFlags()) {
for (const StringRef Flag : clang::DiagnosticIDs::getDiagnosticFlags()) {

@@ -0,0 +1,58 @@
//===--- AvoidUnprototypedFunctionsCheck.cpp - clang-tidy -----------------===//
Copy link
Contributor

Choose a reason for hiding this comment

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

Suggested change
//===--- AvoidUnprototypedFunctionsCheck.cpp - clang-tidy -----------------===//
//===----------------------------------------------------------------------===//

Copy link
Member Author

Choose a reason for hiding this comment

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

Seems like I used an outdated add_new_check.py. 😅

@@ -0,0 +1,33 @@
//===--- AvoidUnprototypedFunctionsCheck.h - clang-tidy ---------*- C++ -*-===//
Copy link
Contributor

Choose a reason for hiding this comment

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

Suggested change
//===--- AvoidUnprototypedFunctionsCheck.h - clang-tidy ---------*- C++ -*-===//
//===----------------------------------------------------------------------===//

/// 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
Copy link
Contributor

Choose a reason for hiding this comment

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

Suggested change
/// http://clang.llvm.org/extra/clang-tidy/checks/portability/avoid-unprototyped-functions.html
/// https://clang.llvm.org/extra/clang-tidy/checks/portability/avoid-unprototyped-functions.html

@@ -208,6 +208,11 @@ New checks

Detect redundant parentheses.

- New :doc:`portability-avoid-unprototyped-functions
Copy link
Contributor

Choose a reason for hiding this comment

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

Please keep alphabetical order (by check name) in this section.

- 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.
Copy link
Contributor

Choose a reason for hiding this comment

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

I think statement should be more specific. Same in documentation.

return 0;
}

From C23 however, the smae pointer will only bind to functions that take no parameters.
Copy link
Contributor

Choose a reason for hiding this comment

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

Suggested change
From C23 however, the smae pointer will only bind to functions that take no parameters.
From C23 however, the same pointer will only bind to functions that take no parameters.

return 0;
}

Some codebases might explicitly take advantage of the pre-C23 behavior, but these codebases should also be aware that
Copy link
Contributor

Choose a reason for hiding this comment

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

Please follow 80 characters limit.

@@ -581,7 +582,38 @@ runClangTidy(clang::tidy::ClangTidyContext &Context,
return AdjustedArgs;
};

// Add extra arguments passed by clang-diagnostic-* checks.
Copy link
Contributor

Choose a reason for hiding this comment

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

Does it really relate to check?

@isuckatcs isuckatcs force-pushed the portability-avoid-unprototyped-functions branch from c946c4c to e29ef81 Compare September 28, 2025 15:21
…lang warnings

Prior to this change to enable additional clang diagnostics, one had to
specify extra arguments on either the command line, or in the config
file.

This meant invocations like `clang-tidy ... -- -Wall`, or `clang-tidy
--extra-arg=-Wall ...` as well as config files like the one below.

```
\# .clang-tidy
Checks: ...
ExtraArgs: ["-Wall"]
```

This patch merges this functionality with the commonly used checks
interface, so the invocation for enabling the `-Wall` flag will look
like `clang-tidy --checks="...,clang-additional-diagnostic-all" ...`.
@isuckatcs isuckatcs force-pushed the portability-avoid-unprototyped-functions branch from e29ef81 to d6116b4 Compare September 28, 2025 15:31
@carlosgalvezp
Copy link
Contributor

IIUC the clang-diagnostic-strict-prototypes check on its own doesn't enable the warning, but what if it did?

I think this topic should be discussed in an RFC outside of this thread. Personally I'm not a big fan of adding one more way to do the same thing. I'd rather improve the documentation to clearly describe the way to do it.

@carlosgalvezp
Copy link
Contributor

I wonder if we should extend that warning?

Sounds reasonable to me!

@isuckatcs
Copy link
Member Author

So extending the warning basically means incorporating the functionality of -Wstrict-prototypes into -Wpre-c23-compat, right? I also wonder what if we simply just preserve -Wstrict-prototypes for C23 as well?

@zygoloid what's your take on this, are we allowed to do any of it on the frontend?

@isuckatcs
Copy link
Member Author

I think this topic should be discussed in an RFC outside of this thread.

I created an RFC on discourse for gathering feedback.

Personally I'm not a big fan of adding one more way to do the same thing. I'd rather improve the documentation to clearly describe the way to do it.

Technically, there are already 2 ways to do it (3 if we count .clang-tidy and the CLI separately), all with their advantage and drawback. I assume we want to settle on the --extra-arg approach as the recommended way then.

@localspook
Copy link
Contributor

localspook commented Sep 29, 2025

The warning warns on binary literals, but pre C23 code still compiles with binary literals even though it is invalid.

Oh yeah, I think I picked a bad example; binary literals were only added to the standard in C23, but apparently everyone supports them as an extension in earlier versions too

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

6 participants