Reputation: 107
I'm currently working on a library for function abstraction. I want to abstract a multi-parameter-function to a one-parameter-function which is an index. Optionally I want to pass a predicate object which determines if it should run the code.
using FnType = std::function<void(const unsigned long)>;
template<typename Fn, typename Predicate, typename ... Args>
const FnType make_fn(const Fn &fn, const Predicate &predicate, Args ... args) {
return [&](const unsigned long idx) {
if(predicate(idx)) fn(idx, args ...);
};
}
template<typename Fn, typename ... Args>
const FnType make_fn(const Fn &fn, Args ... args) {
return [&](const unsigned long idx) {
fn(idx, args ...);
};
}
This code works perfectly when using a predicate.
std::vector<double> data(10);
auto fn = [&](unsigned idx, double d) { data[idx] *= d; };
auto pred = [](unsigned long idx) { return idx % 2 == 0; };
FnType new_fn = make_fn(fn, pred, 2.0);
But when I try to pass no predicate
FnType new_fn2 = make_fn(fn, 2.0);
it returns the following compile time error message:
called object type 'double' is not a function or function pointer
if(predicate(idx)) fn(idx, args ...);
^~~~~~~~~
Is there any possibility to say the compiler it should use the second function which does not use any predicate?
Upvotes: 2
Views: 858
Reputation: 302643
Let's take a look at your signatures:
template<typename Fn, typename Predicate, typename ... Args>
const FnType make_fn(const Fn&, const Predicate&, Args ...); // (1)
template<typename Fn, typename ... Args>
const FnType make_fn(const Fn&, Args ... ) // (2)
When we call with:
make_fn(fn, 2.0);
There isn't anything special about Predicate
- it's just another type. So when we do overload resolution, we have two exact matches. The way we resolve variadic parameters is that (1)
is more specialized than (2)
- since (1)
takes at least 2 arguments and (2)
takes at least 1 argument, the former is more specific. That's why we call the former. The compiler doesn't know from the signature what you mean by Predicate
.
The simplest solution would be to simply rename one function or the other - do you really need them to be overloaded? Have a make_fn()
and a make_predicate_fn()
.
Alternatively, we could change that with SFINAE, by enforcing that Predicate
is callable:
template <typename Fn, typename Predicate, typename... Args,
typename = decltype(std::declval<Predicate>()(0u))>
const FnType make_fn(const Fn&, const Predicate&, Args ...);
That would make (1)
ill-formed for your call, so (2)
would be preferred. Though this additionally has the downside of what if you don't want to use the predicate in this way, but instead simply have some function whose first argument is a predicate? The wrong thing would happen.
A better fix proposed by T.C. in comments is to just introduce an empty tag type:
struct predicate_tag_t { };
constexpr predicate_tag_t predicate_tag{};
And overload on the presence of that tag:
template <typename Fn, typename Predicate, typename... Args>
FnType make_fn(const Fn&, predicate_tag_t, const Predicate&, Args&&... );
template <typename Fn, typename... Args>
FnType make_fn(const Fn&, Args&&... );
That way your examples become:
FnType new_fn = make_fn(fn, predicate_tag, pred, 2.0);
FnType new_fn2 = make_fn(fn, 2.0);
You have dangling references in both make_fn
s when you have:
const FnType make_fn(const Fn &fn, Args ... args) {
return [&](const unsigned long idx) { ... }
}
You're copying all your args, and then holding references to all of them. But once make_fn
completes, all the args
go out of scope. You'll need to copy them - so [=]
.
Upvotes: 5
Reputation: 15468
Change your upper function to
template<typename Fn, typename Predicate, typename ... Args
, typename = std::enable_if_t<sizeof ... (Args) != 0> >
const FnType make_fn(const Fn &fn, const Predicate &predicate, Args ... args) {
return [=](const unsigned long idx) {
if(predicate(idx)) fn(idx, args ...);
};
}
This ensures there is at least one member of the parameter pack, and then the second function is selected during overload resolution.
Some more things to note:
Better make the return type auto
-- you can still asign it to a std::function
, but requires less overhead if you simply call it.
Probably you don't want to pass the general parameters in Args ...
by value, so better write Args const& ... args
.
Upvotes: 1