5

Consider this example:

#include <vector>
#include <string>
#include <functional>
#include <iostream>

using closure_type = std::function<void(void)>;
using closure_vec = std::vector<closure_type>;

class callbacks {
    static closure_type common(std::string name, uint32_t number) {
        return [number, name]() { std::cout << name << number << std::endl; };
    }
public:
    static closure_type foo(uint32_t number) { return common("foo ", number); }
    static closure_type print(std::string msg) {
        return [msg]() { std::cout << "print " << msg << std::endl; };
    }
};

template <typename... calls_t> closure_vec wrap(uint32_t number, calls_t &&... calls) {
    return closure_vec {
        callbacks::foo(number),
        std::forward<calls_t>(calls)...,
    };
}

int main() {
    auto vec = wrap(42,
        callbacks::print("hello, "),
        callbacks::print("world"));

    for(auto &e: vec)
        e();
    return 0;
}

Demo (On the right most tab there is a full message)

When this code is checked with clang-tidy, I get the following warning:

warning: Potential memory leak [clang-analyzer-cplusplus.NewDeleteLeaks]

The line number points at the scope exit of the wrap function.

As I understand the message, the tool is concerned that the results form callbacks::foo might be lost. But I don not understand how is it possible: std::function is a safe class and should destroy everything nicely in its destructor. Also its lifetime is controlled by the vector which is safe too.

What is going on here? How do I fix this or workaround?

Unfortunately I cannot just suppress the warning, as this code is scattered everywhere in the codebase.

ivaigult
  • 6,198
  • 5
  • 38
  • 66
  • 2
    I see no dynamic allocations here, hence no memory leaks are possible. The possible explanations are: 1) the leak is from code that's not shown, 2) the C++ library is leaking, 3) The leak detection tool is defective. – Sam Varshavchik Jul 21 '21 at 17:03
  • "the leak is from code that's not shown" - I posted minimal example that reproduces the issue, so it's not the case. – ivaigult Jul 21 '21 at 17:06
  • This doesn't address the question, but a class with nothing but static members generally ought to be a namespace. – Pete Becker Jul 21 '21 at 17:08
  • 1
    Seems like clang-tidy doesn't like something about libstdc++'s implemention of `std::function`. Adding `-stdlib=libc++` makes the warning go away. So I guess Sam was kinda right. The problem is in code you don't show... but it isn't your code at all. – StoryTeller - Unslander Monica Jul 21 '21 at 17:08
  • Does it happen if you don't use an initializer list? Replace `wrap` with a body that either moves from a (std) array or reserves & comma fold executes emplace back moves? – Yakk - Adam Nevraumont Jul 21 '21 at 17:12
  • I shown all the code I have, unless you mean my standard library :-) It's literally a self contained example that reproduces the problem in Godbolt. – ivaigult Jul 21 '21 at 17:12

1 Answers1

2

Try

closure_vec retval;
retval.reserve(sizeof...(calls)+1);
retval.push_back(callbacks::foo(number));
( retval.push_back(std::forward<calls_t>(calls)), ... );
return retval;

this avoids the const initializer_list contained copies of std function your code created, so should be more efficient as well.

Live example.

I tried using a C style array here, but I got the warning as well despite not using a std::initializer_list.

This also works:

std::array<closure_type, sizeof...(calls)+1> tmp ={
    nullptr,
    std::forward<calls_t>(calls)...
};
tmp[0] = callbacks::foo(number);
return {std::make_move_iterator(std::begin(tmp)), std::make_move_iterator(std::end(tmp))};

the problem is callbacks::foo(number) within the initalization.

Yakk - Adam Nevraumont
  • 262,606
  • 27
  • 330
  • 524