[clang-tidy] checker to get rid of raw function pointer usages

[denzor200]

Motivation

Many C++ codebases still use C-style function pointers (return_type (*)(parameters)) for callbacks and strategy patterns:

// BAD - Legacy C-style function pointer
void register_callback(void (*callback)(int, void*), void* user_data);

// BAD - Function taking raw function pointer
void process_data(int* array, size_t size, int (*transform)(int));

This approach is inflexible and cannot work with:

• Lambda expressions with captures;
• Function objects (functors);
• std::bind expressions;
• std::mem_fn expressions.

register_callback([&](int value, void*) { /* lambda with capture */ }, d); // won't be compiled
process_data(arr, n, [](int x) { return x * 2; }); // won't be compiled

Proposed Solution

Add a clang-tidy check that identifies raw function pointer parameters and suggests converting them to more flexible alternatives:

// GOD - converted to std::function
void register_callback(const std::function<void(int)>& callback);

// GOD - converted to template version
template<typename F>
void process_data(int* array, size_t size, F&& transform);

Since the first option can negatively impact performance, it's better to use the second one whenever possible—in static functions and functions declared in A-files. However, the second option can lead to binary bloat, so it shouldn't be used for very large functions.

Special Cases

1. C API Compatibility

// Should NOT warn when extern "C" or __cdecl specified
extern "C" {
    void take_c_api_callback(void (*callback)(int));  // No warning
}

[llvmbot]

@llvm/issue-subscribers-clang-tidy

Author: Denis Mikhailov (denzor200)

Details

**Motivation**

Developers often write code like:

// BAD - Legacy C-style function pointer
void register_callback(void (*callback)(int, void*), void* user_data);

// BAD - Function taking raw function pointer
void process_data(int* array, size_t size, int (*transform)(int));

This approach is inflexible and cannot work with:

• Lambda expressions with captures;
• Function objects (functors);
• std::bind expressions;
• std::mem_fn expressions.

register_callback([&](int value) { /* lambda with capture */ }); // won't be compiled
process_data(arr, n, [](int x) { return x * 2; }); // won't be compiled

Proposed Solution

Add a clang-tidy check that identifies raw function pointer parameters and suggests converting them to more flexible alternatives:

// GOD - converted to std::function
void register_callback(std::function<void(int)> callback);

// GOD - converted to template version
template<typename F>
void process_data(int* array, size_t size, F&& transform);

Since the first option can negatively impact performance, it's better to use the second one whenever possible—in static functions and functions declared in A-files. However, the second option can lead to binary bloat, so it shouldn't be used for very large functions.

Special Cases

1. C API Compatibility

// Should NOT warn when extern "C" or __cdecl specified
extern "C" {
    void take_c_api_callback(void (*callback)(int));  // No warning
}

[firewave]

Switching to std::function might have a performance impact if the object needs to be created (i.e. function is not a template). So maybe that should be considered in the default behavior and/or tunables of such a check.

[LegalizeAdulthood]

Switching to std::function might have a performance impact if the object needs to be created (i.e. function is not a template). So maybe that should be considered in the default behavior and/or tunables of such a check.

Yes, there are checks that issue warnings when std::function<> is passed by value instead of by const reference.

[denzor200]

changed description to use const reference

[firewave]

Switching to std::function might have a performance impact if the object needs to be created (i.e. function is not a template). So maybe that should be considered in the default behavior and/or tunables of such a check.

Yes, there are checks that issue warnings when std::function<> is passed by value instead of by const reference.

I was referring to the overhead of invoking via an actual std::function object. That could be avoided by having a templated function but depending on the implementation that might not be possible and would lead to a regression in performance (and a much noisier callstack - see danmar/cppcheck#8031 (comment)).