160

I want to pass an overloaded function to the std::for_each() algorithm. For example,

class A {
    void f(char c);
    void f(int i);

    void scan(const std::string& s) {
        std::for_each(s.begin(), s.end(), f);
    }
};

I'd expect the compiler to resolve f() by the iterator type. Apparently, it (GCC 4.1.2) doesn't do it. So, how can I specify which f() I want?

Peter Mortensen
  • 30,738
  • 21
  • 105
  • 131
davka
  • 13,974
  • 11
  • 61
  • 86

6 Answers6

157

You can use static_cast<>() to specify which f to use according to the function signature implied by the function pointer type:

// Uses the void f(char c); overload
std::for_each(s.begin(), s.end(), static_cast<void (*)(char)>(&f));
// Uses the void f(int i); overload
std::for_each(s.begin(), s.end(), static_cast<void (*)(int)>(&f)); 

Or, you can also do this:

// The compiler will figure out which f to use according to
// the function pointer declaration.
void (*fpc)(char) = &f;
std::for_each(s.begin(), s.end(), fpc); // Uses the void f(char c); overload
void (*fpi)(int) = &f;
std::for_each(s.begin(), s.end(), fpi); // Uses the void f(int i); overload

If f is a member function, then you need to use mem_fun, or for your case, use the solution presented in this Dr. Dobb's article.

milleniumbug
  • 15,379
  • 3
  • 47
  • 71
In silico
  • 51,091
  • 10
  • 150
  • 143
  • The second method looks very unsafe, is it not? – the_drow May 31 '10 at 09:08
  • 1
    thanks! I still have a problem, though, probably due to the fact that `f()` is a member of a class (see the edited example above) – davka May 31 '10 at 09:09
  • @the_drow: How so? The function pointer assignment only works if there is an overload that matches the function signature implied by the function pointer declaration. – In silico May 31 '10 at 09:10
  • 14
    @the_drow: The second method is actually much safer, if one of the overloads goes away the first method silently gives undefined behavior, while the second catches the problem at compile time. – Ben Voigt May 31 '11 at 04:36
  • 4
    @BenVoigt Hmm, I tested this on vs2010 and couldn't find a case where the static_cast wouldn't catch the problem at compile time. It gave a C2440 with "None of the functions with this name in scope match the target type". Can you clarify? – Nathan Monteleone May 23 '12 at 16:33
  • 7
    @Nathan: It's possible I was thinking of `reinterpret_cast`. Most often I see C-style casts used for this. My rule is just that casts on function pointers are dangerous and unnecessary (as the second code snippet shows, an implicit conversion exists). – Ben Voigt May 23 '12 at 21:51
  • 1
    I also recommend typedefs, they clarify function pointers significantly. – Mooing Duck Feb 17 '14 at 20:52
  • 4
    For member functions: `std::for_each(s.begin(), s.end(), static_cast(&A::f));` – sam-w May 07 '14 at 13:49
  • 1
    @sjwarner `std::for_each` won't accept pointer to member function. – Oktalist Aug 12 '16 at 13:10
  • `mem_fun` is deprecated and should be replaced by `mem_fn` – Azrael3000 May 27 '19 at 08:06
40

Lambdas to the rescue! (note: C++11 required)

std::for_each(s.begin(), s.end(), [&](char a){ return f(a); });

Or using decltype for the lambda parameter:

std::for_each(s.begin(), s.end(), [&](decltype(*s.begin()) a){ return f(a); });

With polymorphic lambdas (C++14):

std::for_each(s.begin(), s.end(), [&](auto a){ return f(a); });

Or disambiguate by removing overloading (only works for free functions):

void f_c(char i)
{
    return f(i);
}
    
void scan(const std::string& s)
{
    std::for_each(s.begin(), s.end(), f_c);
}
Remy Lebeau
  • 555,201
  • 31
  • 458
  • 770
milleniumbug
  • 15,379
  • 3
  • 47
  • 71
  • 1
    Hurray for lambdas! Indeed, an excellent solution to the overload resolution problem. (I thought of this also, but decided to leave it out of my answer so as not to muddy the waters.) – aldo Feb 17 '14 at 21:20
  • 2
    More code for the same result. I think that's not what the lambdas were made for. – Tomáš Zato Jan 27 '16 at 17:15
  • 2
    @TomášZato The difference is that this answer works, and the accepted one doesn't (for the example posted by OP - you also need to use `mem_fn` and `bind`, which, BTW. are also C++11). Also, if we want to get really pedantic `[&](char a){ return f(a); }` is 28 characters, and `static_cast(&f)` is 35 characters. – milleniumbug Jan 27 '16 at 17:42
  • 1
    @TomášZato There you go http://coliru.stacked-crooked.com/a/1faad53c4de6c233 not sure how to make this more clear – milleniumbug Jan 27 '16 at 18:01
24

Why doesn't it work

I'd expect the compiler to resolve f() by the iterator type. Apparently, it (gcc 4.1.2) doesn't do it.

It'd be great if that were the case! However, for_each is a function template, declared as:

template <class InputIterator, class UnaryFunction>
UnaryFunction for_each(InputIterator, InputIterator, UnaryFunction );

Template deduction needs to select a type for UnaryFunction at the point of the call. But f doesn't have a specific type - it's an overloaded function, there are many fs each with different types. There is no current way for for_each to aid the template deduction process by stating which f it wants, so template deduction simply fails. In order to have the template deduction succeed, you need to do more work on the call site.

Generic solution to fixing it

Hopping in here a few years and C++14 later. Rather than use a static_cast (which would allow template deduction to succeed by "fixing" which f we want to use, but requires you to manually do overload resolution to "fix" the correct one), we want to make the compiler work for us. We want to call f on some args. In the most generic way possible, that's:

[&](auto&&... args) -> decltype(auto) { return f(std::forward<decltype(args)>(args)...); }

That's a lot to type, but this sort of problem comes up annoyingly frequently, so we can just wrap that in a macro (sigh):

#define AS_LAMBDA(func) [&](auto&&... args) -> decltype(func(std::forward<decltype(args)>(args)...)) { return func(std::forward<decltype(args)>(args)...); }

and then just use it:

void scan(const std::string& s) {
    std::for_each(s.begin(), s.end(), AS_LAMBDA(f));
}

This will do exactly what you wish the compiler did - perform overload resolution on the name f itself and just do the right thing. This will work regardless of whether f is a free function or a member function.

Barry
  • 286,269
  • 29
  • 621
  • 977
6

The problem here seems to be not overload resolution but in fact template parameter deduction. While the excellent answer from @In silico will solve an ambiguous overloading problem in general, it seems the best fix when dealing with std::for_each (or similar) is to explicitly specify its template parameters:

// Simplified to use free functions instead of class members.

#include <algorithm>
#include <iostream>
#include <string>

void f( char c )
{
  std::cout << c << std::endl;
}

void f( int i )
{
  std::cout << i << std::endl;
}

void scan( std::string const& s )
{
  // The problem:
  //   error C2914: 'std::for_each' : cannot deduce template argument as function argument is ambiguous
  // std::for_each( s.begin(), s.end(), f );

  // Excellent solution from @In silico (see other answer):
  //   Declare a pointer of the desired type; overload resolution occurs at time of assignment
  void (*fpc)(char) = f;
  std::for_each( s.begin(), s.end(), fpc );
  void (*fpi)(int)  = f;
  std::for_each( s.begin(), s.end(), fpi );

  // Explicit specification (first attempt):
  //   Specify template parameters to std::for_each
  std::for_each< std::string::const_iterator, void(*)(char) >( s.begin(), s.end(), f );
  std::for_each< std::string::const_iterator, void(*)(int)  >( s.begin(), s.end(), f );

  // Explicit specification (improved):
  //   Let the first template parameter be derived; specify only the function type
  std::for_each< decltype( s.begin() ), void(*)(char) >( s.begin(), s.end(), f );
  std::for_each< decltype( s.begin() ), void(*)(int)  >( s.begin(), s.end(), f );
}

void main()
{
  scan( "Test" );
}
Community
  • 1
  • 1
aldo
  • 2,927
  • 21
  • 36
5

Not to answer your question, but am I the only one that finds

for ( int i = 0; i < s.size(); i++ ) {
   f( s[i] );
}

both simpler and shorter than the for_each alternative suggested by in silico in this case?

  • 2
    probably, but it's boring :) also, if I want to use iterator to avoid the [] operator, this becomes longer... – davka May 31 '10 at 09:25
  • 3
    @Davka Boring is what we want. Also, iterators are typically no faster (may be slower) than using op[, if that is your concern. –  May 31 '10 at 09:28
  • 9
    Algorithms should be preferred to for loops, since they're less error prone and can have better opportunities for optimisation. There's an article on that somewhere... here it is: http://www.drdobbs.com/184401446 – AshleysBrain May 31 '10 at 09:43
  • If you don't have lambdas and auto, iterators can have some serious semantic and syntactic overhead that is just not worth it. If the OP had used a numeric loop, he'd have saved himself time. – Puppy May 31 '10 at 09:51
  • 5
    @Ashley Until I see some objective stats on the "less error prone" I see no need to believe it. And Meyers in the article appears to talking about loops that use iterators - I'm talking about the efficiency of loops that DON'T use iterators - my own benchmarks tend to suggest that these are marginally faster when optimised - certainly no slower. –  May 31 '10 at 09:52
  • 1
    Here am I, I also find your solution much better. – peterh Jun 28 '16 at 06:04
  • 1
    -1: At least write the damn thing correctly ! Use the correct type (size_t) and cache the size: `for(size_t i=0,n=s.size();i – Arnaud Nov 24 '18 at 14:27
  • 1
    This works only for iteration over a collection with an `operator[]` implementation which itself works with integers in the range [0, size) for some known size. There are countless uses for lambdas which are not this. In fact, countless uses where iterators and range-based `for` loops won't cut it (e.g. traversal of trees, traversal of heterogeneous collections with a bounded set of contained types (the "visitor pattern"), etc.) – hegel5000 Mar 03 '20 at 22:29
4

If you don't mind using C++11, here's a clever helper that is similar to (but less ugly than) the static cast:

template<class... Args, class T, class R>
auto resolve(R (T::*m)(Args...)) -> decltype(m)
{ return m; }

template<class T, class R>
auto resolve(R (T::*m)(void)) -> decltype(m)
{ return m; }

(Works for member functions; should be obvious how to modify it to work for freestanding functions, and you should be able to provide both versions and the compiler will select the right one for you.)

With thanks to Miro Knejp for suggesting: see also https://groups.google.com/a/isocpp.org/d/msg/std-discussion/rLVGeGUXsK0/IGj9dKmSyx4J.

Matthew
  • 2,593
  • 22
  • 25
  • OP's problem is not being able to pass an overloaded name into a function template, and your solution involves passing an overloaded name into a function template? This is just exactly the same problem. – Barry Apr 22 '16 at 13:03
  • 1
    @Barry Not the same problem. Template argument deduction succeeds in this case. [It works](http://ideone.com/8Dil5x) (with a few minor tweaks). – Oktalist Aug 12 '16 at 13:12
  • @Oktalist Because you're providing `R`, it's not deduced. There's also no mention of that in this answer. – Barry Aug 12 '16 at 13:29
  • 1
    @Barry I'm not providing `R`, I'm providing `Args`. `R` and `T` are deduced. It's true the answer could be improved. (There is no `T` in my example though, because it's not a pointer-to-member, because that wouldn't work with `std::for_each`.) – Oktalist Aug 12 '16 at 13:32