[Posted also to comp.lang.c++, which has much lower latency than this
group. --Scott

Consider a class with two containers, where the sum of the sizes of the
containers is cached.  The class invariant is that as long as the cache is
claimed to be up to date, the sum of the sizes of the containers is accurately
cached:

 class Widget {
 public:
   ...
 private:
   std::vector<int> v1;
   std::vector<int> v2;
   mutable std::size_t cachedSumOfSizes;
   mutable bool cacheIsUpToDate;

   void checkInvariant() const
   { assert(!cacheIsUpToDate || cachedSumOfSizes == v1.size()+v2.size()); }
 };

Assume that checkInvariant is called at the beginning and end of every public
member function.  Further assume that the class declares no copy or more
operations, i.e., no copy or move constructor, no copy or move assignment
operator.

Suppose I have an object w where v1 and v2 have nonzero sizes, and
cacheIsUpToDate is true.  Hence cachedSumOfSizes == v1.size()+v2.size().  If w
is copied, the compiler-generated copy operation will be fine, in the sense that
w's invariant will remain fulfilled.  After all, copying w does not change it in
any way.

But if w is moved, the compiler-generated move operation will "steal" v1's and
v2's contents, setting their sizes to zero.  That same compiler-generated move
operation will copy cachedSumOfSizes and cacheIsUpToDate (because moving
built-in types is the same as copying them), and as a result, w will be left
with its invariant unsatisfied:  v1.size()+v2.size() will be zero, but
cachedSumOfSizes won't be, and cacheIsUpToDate will remain true.

When w is destroyed, the assert inside checkInvariant will fire when it's called
from w's destructor. That means that the compiler-generated move operation for
Widget broke the Widget invariant, even though the compiler-generated copy
operations for Widget left it intact.

The above scenario suggests that compiler-generated move operations may be
unsafe even when the corresponding compiler-generated copy operations are safe.
 Is this a valid analysis?

Scott

--
* C++ and Beyond: Meyers, Sutter, & Alexandrescu, Oct. 24-27 near
Seattle (http://cppandbeyond.com/)
* License my training materials for commercial
(http://tinyurl.com/yfzvkp9) or personal use
(http://tinyurl.com/yl5ka5p).

[ comp.std.c++ is moderated.  To submit articles, try just posting with ]
[ your news-reader.  If that fails, use mailto:std-c++@netlab.cs.rpi.edu]
[              --- Please see the FAQ before posting. ---               ]
[ FAQ: http://www.comeaucomputing.com/csc/faq.html                      ]

On Aug 15, 10:38 pm, Scott Meyers <NeverR...@aristeia.com> wrote:
> But if w is moved, the compiler-generated move operation will "steal" v1's
and
> v2's contents, setting their sizes to zero.
>

Side point: is this true?  I didn't think there were any guarantees
that the moved-from state of a container is its empty state.


--
[ comp.std.c++ is moderated.  To submit articles, try just posting with ]
[ your news-reader.  If that fails, use
mailto:std-c++@netlab.cs.rpi.edu<std-c%2B%2B@netlab.cs.rpi.edu>
]
[              --- Please see the FAQ before posting. ---               ]
[ FAQ: http://www.comeaucomputing.com/csc/faq.html                      ]

* Scott Meyers, on 15.08.2010 23:38:
>
> [Posted also to comp.lang.c++, which has much lower latency than this
> group. --Scott

Well, the [comp.lang.c++] discussion seems to be over & done.


> Consider a class with two containers, where the sum of the sizes of the
> containers is cached.  The class invariant is that as long as the cache is
> claimed to be up to date, the sum of the sizes of the containers is
> accurately
> cached:
>
> class Widget {
> public:
>   ...
> private:
>   std::vector<int> v1;
>   std::vector<int> v2;
>   mutable std::size_t cachedSumOfSizes;
>   mutable bool cacheIsUpToDate;
>
>   void checkInvariant() const
>   { assert(!cacheIsUpToDate || cachedSumOfSizes == v1.size()+v2.size()); }
> };
>
[snip]
>
> When w is destroyed, the assert inside checkInvariant will fire when
> it's called
> from w's destructor. That means that the compiler-generated move
> operation for
> Widget broke the Widget invariant, even though the compiler-generated copy
> operations for Widget left it intact.

Here's another example of N3090 implicitly generated move constructors
breaking C++98 class invariants, not involving any explicit check of
the class invariant but merely assuming that it holds  --  as it would
in C++98:


<code>
#include <vector>
#include <algorithm>
#include <utility>      // std::move


//--------------------------- Old C++98 code:

enum PositionState { empty, nought, cross };

class SomeonesClass
{
private:
   std::vector< PositionState >    positions_;

public:
   SomeonesClass(): positions_( 9 ) {}

   SomeonesClass& operator=( SomeonesClass const& other )
   {
       for( int i = 0;  i < 9;  ++i )
       {
           positions_.at( i ) = other.positions_.at( i );
       }
       return *this;
   }

#ifdef  EMULATE_N3090
   SomeonesClass( SomeonesClass&& other )
       : positions_( std::move( other.positions_ ) )
   {}
#endif
};


//--------------------------- New C+0x driver code:

template< class Type >
void fastSwap( Type& a, Type& b )
{
   Type    temp( std::move( a ) );

   a = std::move( b );
   b = std::move( temp );
}

int main()
{
   SomeonesClass  board1;
   SomeonesClass  board2;

   fastSwap( board1, board2 );
}
</code>


I'm sure that there must be schemes for fixing this without
sacrificing the efficiency gains from move operations for existing
code.

My view is that applying a possibly code-breaking optimization should
be the programmer's active, explicit decision, through code and/or
tool usage, not something applied by default pulling the rug from
under one's feet...


Cheers & hth.,

- Alf

--
blog at <url: http://alfps.wordpress.com>

[ comp.std.c++ is moderated.  To submit articles, try just posting with ]
[ your news-reader.  If that fails, use mailto:std-c++@netlab.cs.rpi.edu]
[              --- Please see the FAQ before posting. ---               ]
[ FAQ: http://www.comeaucomputing.com/csc/faq.html                      ]

Fuz wrote:
>
> Side point: is this true?  I didn't think there were any guarantees
> that the moved-from state of a container is its empty state.

You appear to be right, now that I check the draft standard.  From
what I can tell, there is nothing said about the state of a moved-from
container.

Thanks for pointing this out.

Scott

--
* C++ and Beyond: Meyers, Sutter, & Alexandrescu, Oct. 24-27 near
Seattle (http://cppandbeyond.com/)
* License my training materials for commercial
(http://tinyurl.com/yfzvkp9) or personal use
(http://tinyurl.com/yl5ka5p).

[ comp.std.c++ is moderated.  To submit articles, try just posting with ]
[ your news-reader.  If that fails, use mailto:std-c++@netlab.cs.rpi.edu]
[              --- Please see the FAQ before posting. ---               ]
[ FAQ: http://www.comeaucomputing.com/csc/faq.html                      ]

On Aug 15, 5:38 pm, Scott Meyers <NeverR...@aristeia.com> wrote:
...
> The above scenario suggests that compiler-generated move operations may be
> unsafe even when the corresponding compiler-generated copy operations are safe.
>  Is this a valid analysis?
>
> Scott

The implicit move constructor (assignment operator) can't preserve
invariants due to adopted copy semantics for built-in types.

I think it would be more logical if the standard adopted swap
operation instead of copy for moving built-in types. One may argue
that swap operations on built-in types are slower than copies, but
that can probably be remedied by hardware, there might even be
processor instructions that do swap as fast as assignment. <dreaming>I
would go even further suggesting a new binary swap operator (say
operator<>) to be added to the language</dreaming>. Having swap
operator, move constructors and assignments can been written in terms
of swap, and implicitly generated ones would preserve the invariants.
This simple change in the standard wouldn't break any code (which
isn't already broken). And as a side effect you would rarely (if ever)
need to write user-defined move constructors or assignments. For
example, standard containers now must have user-defined move ctor/
assignment, where they must empty themselves to provide user-expected
behavior. If swap were adopted, they wouldn't need user-defined move
ctor/assignment, the defaults would be correct:

// don't nitpick irrelevant issues

template<...>
class vector
{
  pointer data;
...
  // this would be unnecessary with swap
  vector(vector&& other)
  {
       // destroy data
       // data = other.data
       // other.data = 0
  }
};


--
[ comp.std.c++ is moderated.  To submit articles, try just posting with ]
[ your news-reader.  If that fails, use mailto:std-c++@netlab.cs.rpi.edu]
[              --- Please see the FAQ before posting. ---               ]
[ FAQ: http://www.comeaucomputing.com/csc/faq.html                      ]