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);
}
}
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.