20

Consider the following:

class A {
public:
    const int c; // must not be modified!

    A(int _c)
    :   c(_c)
    {
        // Nothing here
    }

    A(const A& copy)
    : c(copy.c)
    {
        // Nothing here
    }    
};



int main(int argc, char *argv[])
{
    A foo(1337);

    vector<A> vec;
    vec.push_back(foo); // <-- compile error!
    
    return 0;
}

Obviously, the copy constructor is not enough. What am I missing?

EDIT:
Ofc. I cannot change this->c in operator=() method, so I don't see how operator=() would be used (although required by std::vector).

7
  • 5
    What is the compile error? Don't be vague, be an ace; write a proper test-case! Commented Nov 8, 2010 at 13:38
  • 2
    I think you have a choice: either you lose the const, or you lose the ability to use the vector. If you start working around it and allow operator= to modify the const member, you have now given means for any piece of code to do the same. Commented Nov 8, 2010 at 15:57
  • const int c; // must not be modified! Your comment above, does that mean 'c' should not modified by something that uses objects of class A or by the members of class A itself? Commented Nov 8, 2010 at 17:33
  • @anand: c should only be set in the constructor. In my specific case, i have a pointer to some parent node. task * const parent; this pointer must not be re-seated, so changes are not allowed to c either in-class or externally. Commented Nov 9, 2010 at 6:54
  • 1
    @eisbaw: To me the choice of 'const int c' as a member of class A is the root cause of the problem. Hence my question. Having an operator=() and using const_cast<> to cast away const to assign a value to 'c' sounds like a hack to fix the compiler error and not a solution to the actual problem. Commented Nov 9, 2010 at 16:21

10 Answers 10

23

I'm not sure why nobody said it, but the correct answer is to drop the const, or store A*'s in the vector (using the appropriate smart pointer).

You can give your class terrible semantics by having "copy" invoke UB or doing nothing (and therefore not being a copy), but why all this trouble dancing around UB and bad code? What do you get by making that const? (Hint: Nothing.) Your problem is conceptual: If a class has a const member, the class is const. Objects that are const, fundamentally, cannot be assigned.

Just make it a non-const private, and expose its value immutably. To users, this is equivalent, const-wise. It allows the implicitly generated functions to work just fine.

Sign up to request clarification or add additional context in comments.

4 Comments

Well said. I guess it was the comment after the const int c that made me assume that there was no other work-around for @eisbaw's situation. The old adage rings true: "why make things complicated for ourselves?"
Actually, it is not that easy to "just drop 'const'". Why? Because dropping 'const' at member fields effectively enforces you to drop 'const' in the constructor arguments, which in turn forces you to drop 'const' in the place where you use it etc... (I think, the idea should be clear). The solution with std::vector of pointers should work though (although it is not very elegant). By the way, in VS2010 you can push_back elements without assignment operator into the std::vector.
@DmitriiSemikin Removing const from the field doesn't affect the parameters. If you're assigning by value, the value copies into the non-const field just fine. class C { int mX; ClassName(const int x) : mX(x) {} };
@GManNickG There is another point of view. After copying an object becomes a different object. Therefore, constness of members doesn't apply in object copying context. Unfortunately, now I have to choose between const members semantic and assigning to an object.
16

An STL container element must be copy-constructible and assignable1(which your class A isn't). You need to overload operator =.

1 : §23.1 says The type of objects stored in these components must meet the requirements of CopyConstructible types (20.1.3), and the additional requirements of Assignabletypes


EDIT :

Disclaimer: I am not sure whether the following piece of code is 100% safe. If it invokes UB or something please let me know.

A& operator=(const A& assign)
{
    *const_cast<int*> (&c)= assign.c;
    return *this;
}

EDIT 2

I think the above code snippet invokes Undefined Behaviour because trying to cast away the const-ness of a const qualified variable invokes UB.

12 Comments

@eisbaw: Sorry but you cannot assign to this->c[because it is a constant] inside the overloaded operator=.
@eisbaw: Could you not write an empty operator= that simply ignores whatever you pass to it? That maintains the required interface but preserves the integrity of c.
@Prasoon: If *this was declared const (e.g. A const obj (3); obj = A(42);), then it is UB. Otherwise it's simply a bad idea.
@Roger : Yes right. ` §7.1.​5.1/4` says Except that any class member declared mutable (7.1.1) can be modified, any attempt to modify a const object during its lifetime (3.8) results in undefined behavior. but we don't have any const object here so it should be fine though its a bad idea.
Actually your statement is not totally true. vector requires the element to be copy-constructible and assignable but not all containers enforce this (list does not enforce assignability).
|
7

You're missing an assignment operator (or copy assignment operator), one of the big three.

Comments

2

The stored type must meet the CopyConstructible and Assignable requirements, which means that operator= is needed too.

Comments

1

Probably the assignment operator. The compiler normally generates a default one for you, but that feature is disabled since your class has non-trivial copy semantics.

Comments

0

I think the STL implementation of vector functions you are using require an assignment operator (refer Prasoon's quote from the Standard). However as per the quote below, since the assignment operator in your code is implicitly defined (since it is not defined explicitly), your program is ill-formed due to the fact that your class also has a const non static data member.

C++03

$12.8/12 - "An implicitly-declared copy assignment operator is implicitly defined when an object of its class type is assigned a value of its class type or a value of a class type derived from its class type. A program is illformed if the class for which a copy assignment operator is implicitly defined has:

— a nonstatic data member of const type, or

— a nonstatic data member of reference type, or

— a nonstatic data member of class type (or array thereof) with an inaccessible copy assignment operator, or

— a base class with an inaccessible copy assignment operator.

Comments

0

Workaround without const_cast.

A& operator=(const A& right) 
{ 
    if (this == &right) return *this; 
    this->~A();
    new (this) A(right);
    return *this; 
} 

10 Comments

Isn't that just replacing one type of Undefined Behavior with another?
A const obj (42); obj = obj;
@Roger Pate. Right. The check is needed. I fixed.
@TheUndeadFish. Where is UB here?
@Alexey: UB happens for derived classes; this is just a poor way to implement copy assignment.
|
0

I recently ran into the same situation and I used a std::set instead, because its mechanism for adding an element (insert) does not require the = operator (uses the < operator), unlike vector's mechanism (push_back).

If performance is a problem you may try unordered_set or something else similar.

Comments

0

You also need to implement a copy constructor, which will look like this:

class A {
public:
    const int c; // must not be modified!

    A(int _c)
    ...

    A(const A& copy)
    ...  

    A& operator=(const A& rhs)
    {
        int * p_writable_c = const_cast<int *>(&c);
        *p_writable_c = rhs.c;
        return *this;
    }

};

The special const_cast template takes a pointer type and casts it back to a writeable form, for occasions such as this.

It should be noted that const_cast is not always safe to use, see here.

Comments

0

I just want to point out that as of C++11 and later, the original code in the question compiles just fine! No errors at all. However, vec.emplace_back() would be a better call, as it uses "placement new" internally and is therefore more efficient, copy-constructing the object right into the memory at the end of the vector rather than having an additional, intermediate copy.

cppreference states (emphasis added):

std::vector<T,Allocator>::emplace_back

Appends a new element to the end of the container. The element is constructed through std::allocator_traits::construct, which typically uses placement-new to construct the element in-place at the location provided by the container.

Here's a quick demo showing that both vec.push_back() and vec.emplace_back() work just fine now.

Run it here: https://onlinegdb.com/BkFkja6ED.

#include <cstdio>
#include <vector>

class A {
public:
    const int c; // must not be modified!

    A(int _c)
    :   c(_c)
    {
        // Nothing here
    }

    // Copy constructor 
    A(const A& copy)
    : c(copy.c)
    {
        // Nothing here
    }    
};

int main(int argc, char *argv[])
{
    A foo(1337);
    A foo2(999);

    std::vector<A> vec;
    vec.push_back(foo); // works!
    vec.emplace_back(foo2); // also works!
    
    for (size_t i = 0; i < vec.size(); i++)
    {
        printf("vec[%lu].c = %i\n", i, vec[i].c);
    }
    
    return 0;
}

Output:

vec[0].c = 1337
vec[1].c = 999

Comments

Your Answer

By clicking “Post Your Answer”, you agree to our terms of service and acknowledge you have read our privacy policy.

Start asking to get answers

Find the answer to your question by asking.

Ask question

Explore related questions

See similar questions with these tags.