Boost logo

Boost :

Subject: Re: [boost] [move][container] Review Request (new versions of Boost.Move and Boost.Container in sandbox and vault)
From: Howard Hinnant (howard.hinnant_at_[hidden])
Date: 2009-09-07 10:00:32


On Sep 7, 2009, at 7:53 AM, David Abrahams wrote:

>>> The programmer has not required any value swapping just a
>>> "transfer" and
>>> I think the programmer expects all previous v1 values should have
>>> been
>>> destroyed.
>>
>> To be honest, it is exactly because of this argument why I don't like
>> the clear() in the assignment operator of vector. The words "The only
>> requirement is that the object remain in a self consistent state (all
>> internal invariants are still intact)."
>> don't forbid to implement
>> move-assignment as swap,
>
> The other requirement is that user-observable side-effects of
> assignment
> from an lvalue must be preserved. If you own a T that's an
> implementation detail, you can swap it away and nobody's the wiser.
> If,
> as in vector<T>, pair<T>, shared_ptr<T>, etc., you have a T that the
> user can observe, you need to make sure it is destroyed.

In addition to the example Dave provided, here is an example which you
can compile with a C++98 compiler (after adding boost and changing the
namespace of std::shared_ptr):

#include <iostream>
#include <fstream>
#include <sstream>
#include <vector>
#include <memory>
#include <cassert>

int main()
{
     typedef std::shared_ptr<std::ofstream> Resource;
     std::vector<Resource> x;
     const int N = 10;
     for (int i = 1; i <= N; ++i)
     {
         std::ostringstream filename;
         filename << "file" << i << ".dat";
         x.push_back(Resource(new std::ofstream(filename.str().c_str
())));
         *x.back() << "This is " << filename.str() << '\n';
     }
     std::vector<Resource> y(1, Resource(new std::ofstream
("last_file.dat")));
     x = y;
     for (int i = 1; i <= N; ++i)
     {
         std::ostringstream filename;
         filename << "file" << i << ".dat";
         std::ifstream infile(filename.str().c_str());
         std::string line;
         getline(infile, line);
         std::cout << line << '\n';
     }
}

All this does is create a vector<shared_ptr<ofstream>>, fill it up
with some files, write to those files, and then copy-assign the vector
with another vector. Then it opens the files that were just copy-
assigned on top of and prints out the contents. For me this program
prints out:

This is file1.dat
This is file2.dat
This is file3.dat
This is file4.dat
This is file5.dat
This is file6.dat
This is file7.dat
This is file8.dat
This is file9.dat
This is file10.dat

Now, notice that after:

     x = y;

the value of 'y' is never used again. Indeed, after this assignment
this program doesn't care about the value of 'y'. In C++0x I ought to
be able to optimize this program with:

     x = std::move(y);

and have the behavior of the program remain unchanged.

Using an experimental C++0x compiler and library (you might can do
this experiment g++-4.4, I'm not sure) I make this substitution and
indeed, I get the same output (remove all of the *.dat files prior to
each run). This experimental library has:

template <class _Tp, class _Allocator>
inline
vector<_Tp, _Allocator>&
vector<_Tp, _Allocator>::operator=(vector&& __x)
{
     clear();
     swap(__x);
     return *this;
}

However, if I remove the "clear()" and rerun the program (remember to
remove the *.dat files first), then it outputs:

(a bunch of newlines)

Without the "clear()" my "optimization" has altered the visible
behavior of this program (Bad!(tm)).

Finally, std::remove_if is a generic algorithm that will move assign
from a value, and not destruct that moved-from value nor assign a new
value to that moved-from value. It simply leaves it alone for the
client to deal with. If given a copy constructible type,
std::remove_if should have the same behavior (except for performance)
whether it internally uses copy assignment or move assignment.

-Howard


Boost list run by bdawes at acm.org, gregod at cs.rpi.edu, cpdaniel at pacbell.net, john at johnmaddock.co.uk