Is C++'s default copy-constructor inherently unsafe? Are iterators fundamentally unsafe too?

Question

I used to think C++'s object model is very robust when best practices are followed.
Just a few minutes ago, though, I had a realization that I hadn't had before.

Consider this code:

class Foo {     std::set<size_t> set;     std::vector<std::set<size_t>::iterator> vector;     // ...     // (assume every method ensures p always points to a valid element of s) }; 

I have written code like this. And until today, I hadn't seen a problem with it.

But, thinking about it a more, I realized that this class is very broken:
Its copy-constructor and copy-assignment copy the iterators inside the vector, which implies that they will still point to the old set! The new one isn't a true copy after all!

In other words, I must manually implement the copy-constructor even though this class isn't managing any resources (no RAII)!

This strikes me as astonishing. I've never come across this issue before, and I don't know of any elegant way to solve it. Thinking about it a bit more, it seems to me that copy construction is unsafe by default -- in fact, it seems to me that classes should not be copyable by default, because any kind of coupling between their instance variables risks rendering the default copy-constructor invalid.

Are iterators fundamentally unsafe to store? Or, should classes really be non-copyable by default?

The solutions I can think of, below, are all undesirable, as they don't let me take advantage of the automatically-generated copy constructor:

1. Manually implement a copy constructor for every nontrivial class I write. This is not only error-prone, but also painful to write for a complicated class.
2. Never store iterators as member variables. This seems severely limiting.
3. Disable copying by default on all classes I write, unless I can explicitly prove they are correct. This seems to run entirely against C++'s design, which is for most types to have value semantics, and thus be copyable.

Is this a well-known problem, and if so, does it have an elegant/idiomatic solution?

Answer 1

C++ copy/move ctor/assign are safe for regular value types. Regular value types behave like integers or other "regular" values.

They are also safe for pointer semantic types, so long as the operation does not change what the pointer "should" point to. Pointing to something "within yourself", or another member, is an example of where it fails.

They are somewhat safe for reference semantic types, but mixing pointer/reference/value semantics in the same class tends to be unsafe/buggy/dangerous in practice.

The rule of zero is that you make classes that behave like either regular value types, or pointer semantic types that don't need to be reseated on copy/move. Then you don't have to write copy/move ctors.

Iterators follow pointer semantics.

The idiomatic/elegant around this is to tightly couple the iterator container with the pointed-into container, and block or write the copy ctor there. They aren't really separate things once one contains pointers into the other.

Answer 2

Yes, this is a well known "problem" -- whenever you store pointers in an object, you're probably going to need some kind of custom copy constructor and assignment operator to ensure that the pointers are all valid and point at the expected things.

Since iterators are just an abstraction of collection element pointers, they have the same issue.