Search code examples
c++std-function

Why invoking operator() as a std::function does not work


Consider this small class aimed to collect a sequence of strings:

class issues_t final
{
 private:
    std::vector<std::string> m_issues;

 public:
    constexpr void operator()(std::string&& txt)
       {
        m_issues.push_back( std::move(txt) );
       }
};

I plan to use this as:

issues_t issues;
do_something(issues);

This works:

void do_something(issues_t& notify_issue)
{
    notify_issue("something");
}

But I'd rather like to consume this as:

using fnotify_t = std::function<void(std::string&&)>;
void do_something(fnotify_t const& notify_issue)
{
    notify_issue("something");
}

This compiles but doesn't work: in the debugger the push_back() is executed but the underlining vector is not populated (godbolt), so what I'm missing here?

minimum reproducible example:

#include <string>
#include <vector>
#include <functional>
#include <print>

class issues_t final
{
 private:
    std::vector<std::string> m_issues;

 public:
    constexpr void operator()(std::string&& txt)
       {
        m_issues.push_back( std::move(txt) );
       }

    [[nodiscard]] constexpr auto begin() const noexcept { return m_issues.cbegin(); }
    [[nodiscard]] constexpr auto end() const noexcept { return m_issues.cend(); }
};

using fnotify_t = std::function<void(std::string&&)>;
void do_first(fnotify_t const& notify_issue)
{
    notify_issue("do_first");
}

void do_second(issues_t& notify_issue)
{
    notify_issue("do_second");
}

int main()
{
    issues_t issues;
    issues("main");
    do_first(issues);
    do_second(issues);

    for( const auto& issue : issues )
       {
        std::print("    {}\n", issue);
       }
}

Solution

  • do_something accepts a const& which can bind to temporary objects. When calling do_something, a temporary std::function object is constructed, and the "something" string is stored within the temporary issues_t it contains, not within the original issues_t issues.

    Your design is prone to such mistakes, and working with templates and lambdas is likely more convenient:

    template <typename F>
    void do_something(F f) {
        f("something");
    }
    
    int main() {
        std::vector<std::string> issues;
    
        do_something([&issues](std::string&& s) {
            issues.push_back(std::move(s));
        });
    }
    

    More idiomatically, do_something should look like:

    // 1. constrain F to make sure f(...) actually works
    template <std::invocable<const char*> F>
    void do_something(F&& f) {
        // 2. use std::invoke to provide support for member functions pointers and
        //    other things where f() doesn't work
        // 3. perfectly forward f
        std::invoke(std::forward<F>(f), "something");
    }
    

    Any one of these "improvements" (1, 2, 3) aren't strictly necessary, but more idiomatic.