25

I'm learning C++, and I'm trying to implement a binary search function that finds the first element for which a predicate holds. The function's first argument is a vector and the second argument is a function that evaluates the predicate for a given element. The binary search function looks like this:

template <typename T> int binsearch(const std::vector<T> &ts, bool (*predicate)(T)) {
    ...
}

This works as expected if used like this:

bool gte(int x) {
    return x >= 5;
}

int main(int argc, char** argv) {
    std::vector<int> a = {1, 2, 3};
    binsearch(a, gte);
    return 0;
}

But if I use a lambda function as a predicate, I get a compiler error:

search-for-a-range.cpp:20:5: error: no matching function for call to 'binsearch'
    binsearch(a, [](int e) -> bool { return e >= 5; });
    ^~~~~~~~~
search-for-a-range.cpp:6:27: note: candidate template ignored: could not match 'bool (*)(T)' against '(lambda at
      search-for-a-range.cpp:20:18)'
template <typename T> int binsearch(const std::vector<T> &ts,
                          ^
1 error generated.

The above error is generated by

binsearch(a, [](int e) -> bool { return e >= 5; });

What's wrong? Why is the compiler not convinced that my lambda has the right type?

songyuanyao
  • 169,198
  • 16
  • 310
  • 405
rodion
  • 6,087
  • 4
  • 24
  • 29
  • 1
    change bool (*predicate)(T) to std::function – Evgeniy Oct 25 '16 at 08:31
  • 3
    Just a small note on the function arguments - you'll observe that `std::lower_bound` (which you are reimplementing) takes *a pair of iterators* rather than a container as its argument. This allows it to work with any kind of container, or a subset of a container, or even ranges the Standard Library designers haven't yet thought of. When you have your code working over `std::vector` I strongly advise looking to make it more general in this way; I promise you'll learn something! – Toby Speight Oct 25 '16 at 08:49
  • @TobySpeight he would need more than just a predicate. The predicate must be an order relationship and he needs a target value on top of that – Rerito Oct 25 '16 at 09:14
  • Er, I meant `std::find_if()`, not `std::lower_bound()`. Both are instructive, and implementing your own is good exercise, so the rest still stands. – Toby Speight Oct 25 '16 at 09:18
  • 3
    @Evgeniy definitely don't. This is already a template, there's nothing to gain with type erasure here. – Quentin Oct 25 '16 at 12:12

5 Answers5

20

Your function binsearch takes a function pointer as argument. A lambda and a function pointer are different types: a lambda may be considered as an instance of a struct implementing operator().

Note that stateless lambdas (lambdas that don't capture any variable) are implicitly convertible to function pointer. Here the implicit conversion doesn't work because of template substitution:

#include <iostream>

template <typename T>
void call_predicate(const T& v, void (*predicate)(T)) {
    std::cout << "template" << std::endl;
    predicate(v);
}

void call_predicate(const int& v, void (*predicate)(int)) {
    std::cout << "overload" << std::endl;
    predicate(v);
}

void foo(double v) {
    std::cout << v << std::endl;
}

int main() {
    // compiles and calls template function
    call_predicate(42.0, foo);

    // compiles and calls overload with implicit conversion
    call_predicate(42, [](int v){std::cout << v << std::endl;});

    // doesn't compile because template substitution fails
    //call_predicate(42.0, [](double v){std::cout << v << std::endl;});

    // compiles and calls template function through explicit instantiation
    call_predicate<double>(42.0, [](double v){std::cout << v << std::endl;});
}

You should make your function binsearch more generic, something like:

template <typename T, typename Predicate>
T binsearch(const std::vector<T> &ts, Predicate p) {

    // usage

    for(auto& t : ts)
    {
        if(p(t)) return t;
    }

    // default value if p always returned false

    return T{};
}

Take inspiration from standard algorithms library.

rocambille
  • 15,398
  • 12
  • 50
  • 68
  • 1
    Thanks! I think this is slightly less strict than what I intended. For example, your approach would allow to pass a vector of ints and a predicate that maps a char (not an int) to a bool. Probably doesn't matter for my use case, though. – rodion Oct 25 '16 at 09:23
  • 1
    If you're doing such things as passing an int to a predicate that takes a char, your compiler should complain about precision loss. If it is not, change your compiler ;) – rocambille Oct 25 '16 at 09:31
  • Stateless lambdas will convert to function pointers though. The example from the question would compile if it weren't for the template parameter T. The answer is misleading in this regard, imho. – ComicSansMS Oct 25 '16 at 12:31
  • @ComicSansMS I edited my answer. Is it less misleading? – rocambille Oct 25 '16 at 12:51
  • May I suggest adding a 4th example: `call_predicate(42.0, ... `. It's not that templates don't work, it's Template Argument Deduction which fails here. – MSalters Oct 25 '16 at 12:57
  • @MSalters Added ;) – rocambille Oct 25 '16 at 13:00
  • @wasthishelpful Thanks. It's much better now. – ComicSansMS Oct 25 '16 at 13:04
14

The lambda expression with empty capture list could be implicitly converted to a function pointer. But the function pointer predicate is taking T as its parameter, that need to be deduced. Type conversion won't be considered in template type deduction, T can't be deduced; just as the error message said, candidate template (i.e. binsearch) is ignored.

You can use operator+ to achieve this, it'll convert lambda to function pointer, which will be passed to binsearch later and then T will be successfully deduced[1].

binsearch(a, +[](int e) -> bool { return e >= 5; });
//           ~

Of course you could use static_cast explicitly:

binsearch(a, static_cast<bool(*)(int)>([](int e) -> bool { return e >= 5; }));

Note if you change predicate's type to be independent of T, i.e. bool (*predicate)(int), passing lambda with empty capture list would work too; the lambda expression will be converted to function pointer implicitly.


Another solution is changing the parameter type from function pointer to std::function, which is more general for functors:

template <typename T> int binsearch(const std::vector<T> &ts, std::function<bool (typename std::vector<T>::value_type)> predicate) {
    ...
}

then

binsearch(a, [](int e) -> bool { return e >= 5; });

[1] A positive lambda: '+[]{}' - What sorcery is this?

Community
  • 1
  • 1
songyuanyao
  • 169,198
  • 16
  • 310
  • 405
  • 3
    @david how is static cast more readable? There is *more to read*, but all of it is repeating what is said right next to it. Magic that works reliably need not spell itself out. – Yakk - Adam Nevraumont Oct 25 '16 at 08:44
  • 4
    more to read != less readable. magical features that only C++ gurus can understand (such as + and lambda functions) == not readable. – David Haim Oct 25 '16 at 08:45
  • I really like the "+" magic, but I just realized that lambdas cannot be cast to function pointers if they capture local variables. Nonetheless, I learned a lot from your answer. – rodion Oct 25 '16 at 10:15
  • Or change `binsearch`'s signature slightly: `template int binsearch(const std::vector &ts, bool (*predicate)(std::enable_if_t))`. Still the same function-pointer, but now induces conversion to it. – Deduplicator Oct 25 '16 at 13:19
  • 1
    @DavidHaim Lambdas are not "magical features that only C++ gurus can understand". Anyone who programs in C++ should be familiar with lambdas at this point. `+` decaying a lambda is admittedly magical, but *if you don't know what it does, it doesn't really matter*. If you look at the line, it actually does what it seems to do *if you don't understand the magic*. How it does it is magical, but the non-guru *has no need to know the magic* to understand the line. You no more need to know how `+` works than you need to understand vectorization to write a fast `for()` loop. – Yakk - Adam Nevraumont Oct 25 '16 at 14:12
  • 1
    Lambdas are indeed fundamental and there is no argue about it. the use of `+` as a lambda to function converter is very specific and non common. I simply see no reason use something that most of the C++ developers don't really know rather the very known and common `static_cast` . but I guess I won't convince you – David Haim Oct 25 '16 at 14:23
  • "`+` decaying a lambda is admittedly magical, but if you don't know what it does, it doesn't really matter. " That sounds like Cargo Cult Programming at its best - just keep adding `+` operators everywhere till the error messages go away! – alephzero Oct 26 '16 at 03:35
8

Why is the compiler not convinced that my lambda has the right type?

Template functions being told to deduce their template parameters do no conversion. A lambda is not a function pointer, so there is no way to deduce the T in that argument. As all function arguments independently deduce their template parameters (unless deduction is blocked), this results in an error.

There are a number of fixes you can do.

You can fix the template function.

template <class T>
int binsearch(const std::vector<T> &ts, bool (*predicate)(T))

Replace the function pointer with a Predicate predicate or Predicate&& predicate and leave the body unchanged.

template <class T, class Predicate>
int binsearch(const std::vector<T> &ts, Predicate&& predicate)

Use deduction blocking:

template<class T>struct tag_t{using type=T;};
template<class T>using block_deduction=typename tag_t<T>::type;
template <class T>
int binsearch(const std::vector<T> &ts, block_deduction<bool (*)(T)> predicate)

optionally while replacing the function pointer with a std::function<bool(T)>.

You can fix it at the call site.

You can pass T manually binsearch<T>(vec, [](int x){return x<0;}).

You can decay the lambda to a function pointer with putting a + in front of it +[](int x) ... or a static_cast<bool(*)(int)>( ... ).

The best option is Predicate one. This is also what standard library code does.

We can also go a step further and make your code even more generic:

template <class Range, class Predicate>
auto binsearch(const Range &ts, Predicate&& predicate)
-> typename std::decay< decltype(*std::begin(ts)) >::type

The -> typename std::decay ... trailing return type part can be eliminated in C++14.

A benefit to this is that if the body also uses std::begin and std::end to find begin/end iterators, binsearch now supports deques, flat C-style arrays, std::arrays, std::strings, std::vectors, and even some custom types.

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

If you have any control over binsearch I suggest you refactor it:

template <typename T, typename Predicate>
int binsearch(std::vector<T> const& vec, Predicate&& pred) {
    // This is just to illustrate how to call pred
    for (auto const& el : vec) {
        if (pred(el)) {
            // Do something
        }
    }
    return 0; // Or anything meaningful
}

Another way for you is to perform type-erasure on your functor objects/function pointers/whatever... by embedding them into an std::function<bool(T const&)>. To do so, just rewrite the function above as:

template <typename T>
int binsearch(std::vector<T> const& vec, std::function<bool(T const&)> pred);

But since template argument deduction does not do any conversion, you need to explicitly feed your function like the following:

auto my_predicate = [](int x) { return true; }; // Replace with actual predicate
std::vector<int> my_vector = {1, 2, 3, 4};
binsearch(my_vector, std::function<bool (int const&)>(my_predicate));

However given the description of your function, it seems it does the same job as std::find_if.

std::vector<int> my_vector = {1, 12, 15, 13, 16};
auto it = std::find_if(std::begin(my_vector), std::end(my_vector),
                       [](int vec_el) { return !vec_el%5; });
// it holds the first element in my_vector that is a multiple of 5.
if (it != std::end(my_vector)) {
    std::cout << *it << std::endl; // prints 15 in this case
}

Note that to do a binary search, you need more than just a predicate: you need a predicate that defines an order on your range AND a target value.

Rerito
  • 5,886
  • 21
  • 47
  • Do not repeatedly forward a predicate when calling it. The first time you forward it you are making a promise not to call it again. Remove the forward to fix this; detecting the last iteration and conditionally forwarding (while correct) is going to make the code ugly for little yield. – Yakk - Adam Nevraumont Oct 25 '16 at 08:46
  • Switching to `std::function` will not solve the problem here. – ComicSansMS Oct 25 '16 at 12:50
  • @ComicSansMS Sorry, the sentence was misleading... Edited it for clarity's sake – Rerito Oct 25 '16 at 13:02
  • @Rerito Sorry, but I think it's still misleading. Switching to `std::function` allows me to pass stateful lambdas (just as the first solution with the `Predicate` template argument did). But it does not address the issue from the question at all: The fact that template argument deduction fails if I bury the `T` deep enough in the parameter type. This happens with both the function pointer and the `std::function` approach. – ComicSansMS Oct 25 '16 at 13:08
  • @ComicSansMS I don't think I fully get what you mean. Could you provide an example for my own instruction? – Rerito Oct 25 '16 at 13:32
  • @Rerito Does [this example](http://coliru.stacked-crooked.com/a/ba7910a94c85b1ef) clear it up? – ComicSansMS Oct 25 '16 at 13:46
  • @ComicSansMS Yes, you were refering to the template argument deduction not doing any conversion. I edited my answer to reflect that – Rerito Oct 25 '16 at 13:53
  • @Rerito Yeah, with your last edit it looks better. Thanks. – ComicSansMS Oct 25 '16 at 14:39
0

A function pointer and a lambda function is not the same thing.

An object t can not be assigned to predicate where:

 bool (*predicate)(int)

and

auto t = [](int e) -> bool { return e >= 5; });

Might as well use std::function<bool(int)>. Your signature will look like this:

template <typename T> 
int binsearch(const std::vector<T> &ts, std::function<bool(T)> predicate){
   // ...
}

Now that's not a function pointer, You'll need to bind your founction pointers if they are necessary, (I assume you're fine with just lambdas)

Akaedintov
  • 747
  • 1
  • 6
  • 17
  • 1
    This has an overhead that may not be desirable. Note that the standard library doesn't do this because there is no need. You can add a template parameter for the predicate instead. – juanchopanza Oct 25 '16 at 09:10
  • Of course you can assign a stateless lambda to a function pointer like this. [Proof on coliru](http://coliru.stacked-crooked.com/a/27c3bbeef42bd2d6). It's the unspecified template argument that ruins it. Switching to `std::function` also doesn't solve the problem here, as long as you leave the `T` unspecified. – ComicSansMS Oct 25 '16 at 12:40