1

I was trying to implement the copy-and-swap idiom in my custom Matrix class, and I ran into some trouble with the implementation of swap() in the way suggested in the linked-to question:

(The compiler I used is the one from MS VS2010 IDE, dialect is good old-fashioned C++03.)

// matrix.h

namespace my_space 
{

template<typename T> class Matrix
{
public:
    /* ... */

    friend void swap(Matrix<T> &first, Matrix<T> &second)
    {
        using std::swap;
        swap(first.width_, second.width_);
        swap(first.height_, second.height_);
        swap(first.data_, second.data_);
    }
};

} // namespace

Now I have trouble reaching regular std::swap() in the code for functions residing in this namespace:

// some_code.cpp:
#include "matrix.h"
#include <algorithm>

using namespace my_space;
using namespace std;

// SomeClass is part of my_space!
void SomeClass::some_function()
{
    int a = 3, b = 7;
    swap(a,b);  // I wan't std::swap!
}

Unfortunately, for some reason, my_space::swap() for Matrix seems to alias all other calls to std::swap(), and I've no idea why since the arguments don't fit and ADL should favor std::swap:

1>f:\src\some_code.cpp(653): error C3767: 'swap': candidate function(s) not accessible
1>          could be the friend function at 'f:\src\matrix.h(63)' : 'swap'  [may be found via argument-dependent lookup]

(The error repeats 10 times for every line where I'm trying to use std::swap)

Does my_space::swap() always overrule std::swap() in my_space, even if the arguments don't fit? It's not as if std::swap() is not visible, and it worked OK before my_space::swap() was created.

Community
  • 1
  • 1
neuviemeporte
  • 6,310
  • 10
  • 49
  • 78
  • Where are you calling `swap` with `Matrix<>` arguments? Your example uses `int`s... The error makes it look like you're trying to swap two different types of `Matrix<>` objects (i.e., with two different `T`s in `Matrix`) -- why should _that_ work? – ildjarn Jul 19 '12 at 23:52
  • @ildjarn: I'm not calling swap with Matrix arguments (yet). I defined my own swap() for Matrix, but when I'm trying to swap ints elsewhere, the compiler doesn't want to use std::swap, and reverts to my Matrix swap() for some reason. – neuviemeporte Jul 20 '12 at 00:02

4 Answers4

2

The approach taken by STL containers uses a member function and then overload the static function. For example:

template<class T, class Alloc=std::allocator<T> >
class vector
{
   T *data;
   size_t n;
   size_t max_n;
public:
   void swap(vector<T, Alloc> &other)
   {
      swap(this->data, other.data);
      swap(this->n, other.n);
      swap(this->max_n, other.max_n);
   }
};

template<class T, class A>
void swap(vector<T, A> &lhs, vector<T, A> &rhs)
{
   lhs.swap(rhs);
}

In the suggested Matrix class, simply take the same approach...

namespace my_space
{
template<typename T>
class Matrix
{
   unsigned width_;
   unsigned height_;
   std::vector<T> data_;
public:
   void swap(Matrix<T> &other)
   {
      std::swap(this->width_, other.width_);
      std::swap(this->height_, other.height_);
      std::swap(this->data_, other.data_);  // calls this->data_.swap(other.data_);
   }
};
}

namespace std
{
   template<typename T>
   void swap(my_space::Matrix<T> &lhs, my_space::Matrix<T> &rhs)
   {
      lhs.swap(rhs);
   }
}
Andrew
  • 603
  • 1
  • 5
  • 13
1

Include the following line in Matrix:

template<typename U> friend void swap(Matrix<U> &first, Matrix<U> &second);

and define the swap outside of the class. The reason you are getting the error function template has already been defined, is because each instantiation of Matrix<unsigned short> and Matrix<char> will contain the same defintion of your swap function since you defined the friend function inside of the Matrix template.

Jesse Good
  • 50,901
  • 14
  • 124
  • 166
  • I didn't define it elsewhere, andd I wanted it inside Matrix for clarity and immediate visibility. Has to be inside the header anyway as a template. Anyways, keeping only the declaration in the class body, and moving the definition outside helped, the code compiles, not sure why though... – neuviemeporte Jul 19 '12 at 23:39
  • @neuviemeporte: Because you define the friend function inside of the `Matrix` template, each instantiation of the swap function will have a separate definition. That is why you need to define it outside of the class `Matrix`. – Jesse Good Jul 19 '12 at 23:54
  • Still confused, swap() is a template function so swap(Matrix, Matrix) is a different function from swap(Matrix, Matrix) - why then should Matrix and Matrix contain the same definition? Regardless, defining a friend function inside the class body is equivalent to defining a global function so it shouldn't be part of any Matrix anyway...? – neuviemeporte Jul 20 '12 at 00:06
  • 1
    @neuviemeporte, `swap` is a template function but it doesn't depend on the class template parameter in any way. `Matrix` and `Matrix` will both define identical `swap` functions. – Mark Ransom Jul 20 '12 at 00:12
  • @MarkRansom: Thanks! I got confused and didn't think about the two variants of Matrix polluting the global space with two identical copies of swap()... which makes me wonder again - does swap need to be a template function after all? – neuviemeporte Jul 20 '12 at 00:21
  • @neuviemeporte, yes it needs to be a template function, but outside of the class. I don't know why it isn't being considered a specialization of `std::swap`, probably because of the different namespaces. – Mark Ransom Jul 20 '12 at 00:25
  • @MarkRansom: Won't defining friend void swap(Matrix, Matrix) inside the class do the trick? That way, swap() is dependent on the original type and every variant of Matrix will create its own global swap. Or am I missing sommething? – neuviemeporte Jul 20 '12 at 00:29
  • @neuviemeporte, having it inside the class means it's a member, and you'll need to call it from an object of type `Matrix` that it isn't even going to use. Even if you declare it `static` it will still need the class name qualifier. – Mark Ransom Jul 20 '12 at 00:43
  • @MarkRansom: A friend function is not a member, even if defined inside a class, and can be called with no instance of that class. It's just a funny way to define an inline global function. See the code for swap() in ildjarn's answer. – neuviemeporte Jul 20 '12 at 00:56
  • @MarkRansom: I don't think so, why would it? `Matrix` inside the template is a shorthand for `Matrix`, which means that each instantiation will yield a different swap (different argument type). – David Rodríguez - dribeas Jul 20 '12 at 03:08
  • @DavidRodríguez-dribeas: The question has been edited several times. Originally, swap was a function template for `Matrix` and not `Matrix` which would produce the same definition for each instantiation. – Jesse Good Jul 20 '12 at 03:22
  • Sorry to all involved for the confusion and multiple edits. I was trying different things as my understanding of the issue evolved. I'm deeply grateful for all insight in spite of the unfavourable circumstances, especially to Mark, Jesse and David. Wish I could +1 you guys multiple times. – neuviemeporte Jul 20 '12 at 12:38
1

The following builds cleanly for me with VC++ 2010 SP1:

Matrix.h:

#pragma once

#include <algorithm>
#include <vector>

namespace my_space
{
    template<typename T>
    class Matrix
    {
    public:
        Matrix(unsigned const w, unsigned const h)
          : width_(w), height_(h), data_(w * h)
        { }

    private:
        unsigned width_;
        unsigned height_;
        std::vector<T> data_;

        friend void swap(Matrix& lhs, Matrix& rhs)
        {
            using std::swap;
            swap(lhs.width_,  rhs.width_);
            swap(lhs.height_, rhs.height_);
            swap(lhs.data_,   rhs.data_);
        }
    };
}

.cpp:

#include "Matrix.h"

int main()
{
    using namespace my_space;
    using std::swap;

    int a(0), b(1);
    swap(a, b);

    Matrix<int> c(2, 3), d(4, 5);
    swap(c, d);

    Matrix<short> e(6, 7), f(8, 9);
    swap(e, f);
}

Since you didn't post an SSCCE (hint, hint), it's very difficult to see exactly where you're going wrong, but you can use this as a starting point to narrow down your issue.

ildjarn
  • 62,044
  • 9
  • 127
  • 211
  • Hint taken, sorry, I went a little to far with the first S, the question is too long as it is I'm afraid. :( – neuviemeporte Jul 20 '12 at 00:22
  • You only used one variant of `Matrix`. Try also using a `Matrix` in the same `main` function. – Mark Ransom Jul 20 '12 at 00:23
  • @ildjarn: Thanks for taking the trouble; the important bit seems to be the "using std::swap" in main(). If I do that in my TUs requiring std::swap(), the errors go away. Still not sure why the compiler would try to apply the Matrix swap to plain ints though without it, especially since it worked OK before. – neuviemeporte Jul 20 '12 at 00:37
  • 1
    @neuviemeporte, I think if you swapped the two `using namespace` directives in your code it might start working. I suspect this example is using `std::swap` instead of `my_space::swap` when swapping the matrices. – Mark Ransom Jul 20 '12 at 00:40
  • @MarkRansom: Swapping the order of using namespace didn't help, and the problem is the opposite: my_space::swap is (attempted to be) used for swapping ints, but I think you're on to something, because the code swapping the ints resides inside my_space, so that's probably the reason for defaulting to my_space::swap first, and hence the need for an explicit "using std::swap". – neuviemeporte Jul 20 '12 at 00:49
  • @Mark : No, this example is definitely invoking `std::swap` for the `int`s and `my_space::swap` for the `Matrix<>`s -- if I put `int foo;` in the top of `my_space::swap` this code generates two warnings about unreferenced local variables (one for each instantiation of `Matrix<>`). – ildjarn Jul 20 '12 at 02:01
  • This answer does not explain why lookup is finding the friend function (hint: it should not be found, I believe, as a friend declaration is only available through ADL) – David Rodríguez - dribeas Jul 20 '12 at 03:06
  • @David : I don't know what the OP's problem is, given the lack of real code in the question, so I don't know how to explain said problem. – ildjarn Jul 20 '12 at 03:07
1

If the code is really like what you posted, that is an issue with the compiler. The code compiles fine in clang++, as it should.

Friend declarations are strange in that they declare a function that has namespace scope, but the declaration is only available though ADL, and even then, only if at least one of the arguments is of the type of the class that has the friend declaration. Unless there is a namespace level declaration, the function is not available in the namespace scope.


Test 1 (function not available at namespace level with out an explicit declaration):

namespace A {
   struct B {
      friend void f();  // [1]
   };
   // void f();         // [2]
}
void A::f() {}          // [3]

In [1] we add a friend declaration, that declares void A::f() as a friend of A::B. Without the additional declaration in [2] at namespace level, the definition in [3] will fail to compile, since being outside of the A namespace that definition is not also a self-declaration.


The implication here is that, because the function is not available for lookup at namespace level, but only through ADL on Matrix<T> (for some particular instantiating type T), the compiler cannot possibly find that as a match to a swap of two int values.

In his answer, Jesse Good states that each instantiation of Matrix and Matrix will contain the same defintion of your swap function since you defined the friend function inside of the Matrix template which is completely absurd.

A friend function that is defined inside the class will declare and define a namespace level function, and again, the declaration will only be available inside the class and accessible through ADL. When this is done inside a template it will define a non-templated free function at namespace level for each instantiation of the template that uses the function. That is, it will generate different definitions. Note that inside the class template scope, the name of the template identifies the specialization that is being instantiated, that is, inside Matrix<T>, the identifier Matrix does not name the template, but one instantiation of the template.

Test 2


namespace X {
   template <typename T>
   struct A {
      friend void function( A ) {}
   };
   template <typename T>
   void funcTion( A<T> ) {}
}
int main() {
   using namespace X;
   A<int> ai;    function(ai); funcTion(ai);
   A<double> ad; function(ad); funcTion(ad);
}

$ make test.cpp
$ nm test | grep func | c++filt
0000000100000e90 T void X::funcTion<double>(A<double>)
0000000100000e80 T void X::funcTion<int>(A<int>)
0000000100000e70 T X::function(A<double>)
0000000100000e60 T X::function(A<int>)

The output of nm is the list of symbols, and c++filt will translate the mangled names into the equivalent in C++ syntax. The output of the program clearly shows that X::funcTion is a template that has been instantiated for two types, while X::function are two overloaded non-templated functions. Again: two non-templated functions.


Claiming that it would generate the same function makes little sense, consider that it had a function call, say std::cout << lhs, the code must pick the correct overload of operator<< for the current instantiation of the function. There is no single operator<< that can take say a int and an unsigned long or std::vector<double> (Nothing inhibits you from instantiating the template with any type.

The answer by ildjarn proposes an alternative, but provides no explanation of the behavior. The alternative works because a using-directive is completely different from a using-declaration. In particular, the former (using namespace X;) modifies lookup so that the identifiers in namespace X are available at namespace level in one of the enclosing namespaces of the current piece of code (if you build a tree of namespaces, the declarations would be available where the branch containing X meets the branch containing the code where the using-directive is used).

On the other hand, a using-declaration (using std::swap;) provides a declaration of the function std::swap in the context where the using-declaration is present. This is why you must use using-declarations and not using-directives to implement your swap functions:

Test 3


namespace Y { struct Z {}; void swap( Z&,Z& ); }
namespace X {
   struct A { int a; Y::Z b; };
   void swap( A& lhs, A& rhs ) {
      //using namespace std;       // [1]
      using std::swap;             // [2]
      swap( lhs.a, rhs.a );        // [3]
      swap( lhs.b, rhs.b );        // [4]
   }
}

If we had used a using-directive [1], the symbols from the ::std namespace would be available for lookups performed inside the function ::X::swap(X::A&,X::A&) as if they had been declared in :: (which is the common ancestor of ::std and ::X). Now the swap in [3] will not find any swap function through ADL, so it will start searching for swap functions in the enclosing namespace. The first enclosing namespace is X, and it does contain a swap function, so lookup will stop and overload resolution will kick in, but X::swap is not a valid overload for swap(int&,int&), so it will fail to compile.

By using a using-declaration we bring std::swap declaration to the scope of X::swap (inside the function!). Again, ADL will not be applied in [3], and lookup will start. In the current scope (inside the function) it will find the declaration of std::swap and will instantiate the template. In [4], ADL does kick in, and it will search for a swap function defined inside the ::Y::Z function and/or in ::Y, and add that to the set of overloads found in the current scope (again, ::std::swap). At this point, ::Z::swap is a better match than std::swap (given a perfect match, a non-templated function is a better match than a templated one) and you get the expected behavior.


David Rodríguez - dribeas
  • 204,818
  • 23
  • 294
  • 489
  • @JesseGood: What you are missing is that the *argument* makes the whole difference. In particular, add a simple `A` to your [program](http://ideone.com/Ugmik) and you will notice that the error goes away. In the example you provided (which differs from the question) the function takes no arguments (alternatively, non-dependent arguments or dependent arguments that resolve to the same types for different instantiations), and that means that all instantiations of the template *define* the same function. But in the question the arguments contain the current instantiation. – David Rodríguez - dribeas Jul 20 '12 at 12:18
  • @JesseGood: Also note another side effect of how friend declarations work, because it will only be found by ADL, and in your example the function takes no argument dependent on the template, it is effectively impossible to call the function. – David Rodríguez - dribeas Jul 20 '12 at 12:31
  • Right, I agree with what you are saying. However, in the original question before it was edited (look at the edit history), the program was [like this](http://ideone.com/9mNJi) which is the way I show in my answer. – Jesse Good Jul 20 '12 at 20:16