views:

238

answers:

5

The following pattern has arisen in a program I'm writing. I hope it's not too contrived, but it manages to mutate a Foo object in the const method Foo::Questionable() const, without use of any const_cast or similar. Basically, Foo stores a reference to FooOwner and vice versa, and in Questionable(), Foo manages to modify itself in a const method by calling mutate_foo() on its owner. Questions follow the code.

#include "stdafx.h"
#include <iostream>
using namespace std;

class FooOwner;

class Foo {
    FooOwner& owner;
    int data;

public:
    Foo(FooOwner& owner_, int data_)
        : owner(owner_),
          data(data_)
    {
    }

    void SetData(int data_)
    {
        data = data_;
    }

    int Questionable() const;       // defined after FooOwner
};

class FooOwner {
    Foo* pFoo;

public:
    FooOwner()
        : pFoo(NULL)
    {}

    void own(Foo& foo)
    {
        pFoo = &foo;
    }

    void mutate_foo()
    {
        if (pFoo != NULL)
            pFoo->SetData(0);
    }
};

int Foo::Questionable() const
{
    owner.mutate_foo();     // point of interest
    return data;
}

int main()
{
    FooOwner foo_owner;
    Foo foo(foo_owner, 0);      // foo keeps reference to foo_owner
    foo_owner.own(foo);         // foo_owner keeps pointer to foo

    cout << foo.Questionable() << endl;  // correct?

    return 0;
}

Is this defined behavior? Should Foo::data be declared mutable? Or is this a sign I'm doing things fatally wrong? I'm trying to implement a kind of lazy-initialised 'data' which is only set when requested, and the following code compiles fine with no warnings, so I'm a little nervous I'm in UB land.

Edit: the const on Questionable() only makes immediate members const, and not the objects pointed to or referenced by the object. Does this make the code legal? I'm confused between the fact that in Questionable(), this has the type const Foo*, and further down the call stack, FooOwner legitimately has a non-const pointer it uses to modify Foo. Does this mean the Foo object can be modified or not?

Edit 2: perhaps an even simpler example:

class X {
    X* nonconst_this;   // Only turns in to X* const in a const method!
    int data;

public:
    X()
        : nonconst_this(this),
          data(0)
    {
    }

    int GetData() const
    {
        nonconst_this->data = 5;    // legal??
        return data;
    }
};
A: 

You have reached circular dependencies. See FAQ 39.11 And yes, modifying const data is UB even if you have circumvented the compiler. Also, you are severely impairing the compiler's capacity to optimize if you don't keep your promises (read: violate const).

Why is Questionable const if you know that you will modify it via a call to its owner? Why does the owned object need to know about the owner? If you really really need to do that then mutable is the way to go. That is what it is there for -- logical constness (as opposed to strict bit level constness).

From my copy of the draft n3090:

9.3.2 The this pointer [class.this]

1 In the body of a non-static (9.3) member function, the keyword this is an rvalue a prvalue expression whose value is the address of the object for which the function is called. The type of this in a member function of a class X is X*. If the member function is declared const, the type of this is const X*, if the member function is declared volatile, the type of this is volatile X*, and if the member function is declared const volatile, the type of this is const volatile X*.

2 In a const member function, the object for which the function is called is accessed through a const access path; therefore, a const member function shall not modify the object and its non-static data members.

[Note emphasis mine].

On UB:

7.1.6.1 The cv-qualifiers

3 A pointer or reference to a cv-qualified type need not actually point or refer to a cv-qualified object, but it is treated as if it does; a const-qualified access path cannot be used to modify an object even if the object referenced is a non-const object and can be modified through some other access path. [ Note: cv-qualifiers are supported by the type system so that they cannot be subverted without casting (5.2.11). —end note ]

4 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.

dirkgently
I've read elsewhere that `const` is not used for optimization, for reasons like `const_cast` being legitimately used elsewhere. Also, `const` on a method only applies to the immediate members, not pointed-to or referenced objects. Does this make the mutate_foo() call legal?
AshleysBrain
Okay, I should have said this: Take the `const` out, if you don't really mean it. If you do however, then take the time to help the compiler along. Also, just because it compiles, it doesn't mean you cannot invoke UB.
dirkgently
No it does not make the call legal. You "promised" that you don't change the object. It is important for the compiler when it has to decide if a piece of code has side-effects or not, and, rather more importantly, for error checking yourself! You are shooting yourself in the foot. Share with all of us the love and hate we have for the type system and stick to the rules, it may become your friend :)
AndreasT
I cannot see any data defined const in this example at all. See the top answer by UncleBens for detailed explanation.
Suma
@Suma: UncleBens' answer is incorrect as far as the standard is concerned. And please refer to FAQ 18.12 and 18.13.
dirkgently
Okay, before you downvote, put in a good reason. Don't just blindly downvote.
dirkgently
If you want to modify a specific member of `this` from a `const` method, then yeah, declare it mutable rather than storing a redundant pointer. But there's nothing incorrect or UB about modifying a non-const pointer to `this` or a subobject. That issue is called aliasing; it has nothing to do with `const` and everything to do with `restrict`.
Potatoswatter
@Potatoswatter: There is unfortunately no `restrict` in near future for C++0X. However, as for what we do have, is a const access path modifying a member not declared mutable as a side effect is UB.
dirkgently
I'm -1'ing because you've incorrectly diagnosed UB. Modifying an object is only UB if that object is `const`, consider: `int i = 5; const int const_cast<int` or `int i = 5; const int* j =
GMan
...still point/refer to a *non-const object*, which is completely modifiable. (The reason we say `const_cast` is *dangerous* is because it *can* cause UB, not that it *will*.)
GMan
@Dirk: Whether or not `restrict` is C++ is irrelevant, and you haven't backed up your UB claim whatsoever. C++ does not discriminate against duplicate access paths or have a concept of an access path at all.
Potatoswatter
@*: Why I consider modification of non-`mutable` members via a `const` access path -- quoted the standard and edited my answe.
dirkgently
Note the last part: "any attempt to modify a **const object** ... results in undefined behavior." A const object is different than a cv-qualified type referring to an object, just like the first part of the quote is saying ("cv-qualified type need not actually point or refer to a cv-qualified object"). The example I gave demonstrates this: `j` is a cv-qualified type pointing to a non cv-qualified object. You can take the cv-qualifications off the type, and modify the object. *Only* when the original object is cv-qualified (like if it were `const int i = 5`) does removing the cv-qualifiers...
GMan
...and modifying the object result in undefined behavior. (Because we made an "...attempt to modify a const object...".)
GMan
Do you agree this (i.e. the OP's example) is a case of a const access path modifying an object?
dirkgently
And going by your reasoning, you don't know if some object of type `Foo` will ever be declared `const` and then have `Questionable` called on it. You are making enough ammunition to shoot your foot off.
dirkgently
@dirkgently: Yes. And what that means is that every member has `const` applied to it's *type* (in the manner I said on Uncle's answer.) But then we're back to my example, where we have a cv-qualified type which may or may not refer to a cv-qualified object. (And in the OP's case, a non-cv-qualified object.) And I agree! But we aren't discussing if we've provided a way to possibly shoot our feet off (we have), we're discussing if it's *always* going to shoot our feet off.
GMan
@GMan: I don't agree to your interpretation here. In the scope of `Questionable` all `Foo` objects should obey 7.1.6.1#3 and I don't see why you think that this is okay sometimes and not others.
dirkgently
Claim #1 : The standard explicitly says you cannot modify (even) a (originally non-cv qualified) object's members via a const access path. (I think GMan misinterprets this.) Claim #2: The hallmark of UB is that it _may sometimes work_. So, relying on something that works sometimes is not a good argument in favor of ruling out UB. Sutter's Exceptional C++ deals with just this (and why you need `mutable`).
dirkgently
@dirk: They *do* obey 7.1.6.1/3, I don't see where I said they didn't. It just means they all get `const` smacked onto them. Note it doesn't say "it's undefined behavior", it says you cannot do it. That means it's *ill-formed* to try and modify something through a const-access path (the entire purpose of const!). That is, it's saying this is illegal: `const int j = 5; // error, j is const!`, no more. (In other words, it's saying respect the const that is added to all the members.) And I'm not relying on any UB, I'm saying there is none in the OP's code. I never argued it was good code.
GMan
@dirk: The pointer, i.e. access path that `mutate_foo` uses is non-const. Therefore it can be used to mutate the object, which is not const, despite the existence of a `const` pointer to the object in some caller's stack frame. Simply forming a `const` pointer in function A does not alter the semantics of the rest of the program. If the object itself were `const`, there would be no way to form a non-const path to it without `const_cast`, and taking advantage of such a path would produce UB.
Potatoswatter
A: 

The const keyword is only considered during compile time checks. C++ provides no facilities to protect your class against any memory access, which is what you are doing with your pointer/reference. Neither the compiler nor the runtime can know if your pointer points to an instance that you declared const somewhere.

EDIT:

Short example (might not compile):

// lets say foo has a member const int Foo::datalength() const {...}
// and a read only acces method const char data(int idx) const {...}

for (int i; i < foo.datalength(); ++i)
{
     foo.questionable();  // this will most likely mess up foo.datalength !!
     std::cout << foo.data(i); // HERE BE DRAGONS
}

In this case, the compiler might decide, ey, foo.datalength is const, and the code inside the loop promised not to change foo, so I have to evaluate datalength only once when I enter the loop. Yippie! And if you try to debug this error, which will most likely only turn up if you compile with optimizations (not in the debug builds) you will drive yourself crazy.

Keep the promises! Or use mutable with your braincells on high alert!

AndreasT
Not true, const is not (and cannot) be used for compiler optimizations, as it is legal to cast away the const unless the data pointed to are really defined const (which is very non-typical and rare occasion, as this means static initialized const data or const data members, which need to be intialized in a constructor and never changed again).
Suma
casting away const is something the compiler knows about.
AndreasT
+5  A: 

IMO, you are not doing anything technically wrong. May-be it would be simpler to understand if the member was a pointer.

class X
{
    Y* m_ptr;
    void foo() const {
        m_ptr = NULL; //illegal
        *m_ptr = 42; //legal
    }
};

const makes the pointer const, not the pointee.

Consider the difference between:

const X* ptr;
X* const ptr;  //this is what happens in const member functions

As to references, since they can't be reseated anyway, the const keyword on the method has no effect whatsoever on reference members.

In your example, I don't see any const objects, so you are not doing anything bad, just exploiting a strange loophole in the way const correctness works in C++.

UncleBens
If you are correct, then I suppose you can circumvent the constness of any const method simply by having a `T* nonconst_this` member of a class, initialize it to `this`, then modify an object as you like in any const method - and that's legal? That would be a pretty big const loophole!
AshleysBrain
@UncleBens: -1. You are wrong about `const` qualified member functions. Refer to the standard (see relevant quote in my answer).
dirkgently
@dirkgently: What is he wrong on?
GMan
@GMan: Well, for a start, in a const method, all data members are treated as const. Thus, his statement that references are unaffected by const is flat out wrong. If you have a const method, then the reference becomes a const reference.
DeadMG
@GMan: The type of `this`. `this` is implicitly a `const` pointer to an object of some type. In a `const` qualified member functions, no further `const`-ness is added to the pointer but the pointee that is the object to which it points. HTH.
dirkgently
GMan
...use of const-reference. So when you say "If you have a const method, then the reference becomes a const reference.", if you mean `T that conversion would make the program ill-formed. And certainly it isn't the case that `T
GMan
@GMan: Let's clear up syntax first: `T* const` is constant pointer to an object of type T (including the special value `0` or `nullptr`).
dirkgently
GMan
@*: See the second item of my quote from the standard to realize why modifying members in a roundabout way is UB.
dirkgently
@GMan: Ah, I misread.
dirkgently
@dirkgently: Happens, it's alright. Do note I've moved the discussion on whether or not it's UB to your answer.
GMan
I've edited the question - anyone care to comment on the legality of the line `nonconst_this->data = 5;`?
AshleysBrain
@AshleysBrain: See my answer as an extension to Uncle's.
GMan
+12  A: 

Consider the following:

int i = 3;

i is an object, and it has the type int. It is not cv-qualified (is not const or volatile, or both.)

Now we add:

const int& j = i;
const int* k = &i;

j is a reference which refers to i, and k is a pointer which points to i. (From now on, we simply combine "refer to" and "points to" to just "points to".)

At this point, we have two cv-qualified variables, j and k, that point to to a non-cv-qualified object. This is mentioned in §7.1.​5.1/3:

A pointer or reference to a cv-qualified type need not actually point or refer to a cv-qualified object, but it is treated as if it does; a const-qualified access path cannot be used to modify an object even if the object referenced is a non-const object and can be modified through some other access path. [Note: cv-qualifiers are supported by the type system so that they cannot be subverted without casting (5.2.11). ]

What this means is that a compiler must respect that j and k are cv-qualified, even though they point to a non-cv-qualified object. (So j = 5 and *k = 5 are illegal, even though i = 5 is legal.)

We now consider removing the const from those:

const_cast<int&>(j) = 5;
*const_cast<int*>(k) = 5;

This is legal (§refer to 5.2.11), but is it undefined behavior? No. See §7.1.​5.1/4:

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. Emphasis mine.

Remember that i is not const and that j and k both point to i. All we've done is tell the type system to remove the const-qualifier from the type so we can modify the pointed to object, and then modified i through those variables.

This is exactly the same as doing:

int& j = i; // removed const with const_cast
int* k = &i;

j = 5;
*k = 5;

And this is trivially legal. We now consider that i was this instead:

const int i = 3;

What of our code now?

const_cast<int&>(j) = 5;
*const_cast<int*>(k) = 5;

It now leads to undefined behavior, because i is a const-qualified object. We told the type system to remove const so we can modify the pointed to object, and then modified a const-qualified object. This is illegal, as quoted above.

Again, more apparent as:

int& j = i; // removed const with const_cast...
int* k = &i; // but this is not legal!

j = 5;
*k = 5;

Note that simply doing this:

const_cast<int&>(j);
*const_cast<int*>(k);

Is perfectly legal and defined, as no const-qualified objects are being modified.


Now consider:

struct foo
{
    foo(void) :
    me(this), self(*this), i(3)
    {}

    void bar(void) const
    {
        me->i = 5;
        self.i = 5;
    }

    foo* me;
    foo& self;
    int i;
};

What does const on bar do to the members? It makes access to them go through something called a cv-qualified access path. (It does this by changing the type of this from T* const to cv T const*, where cv is the cv-qualifiers on the function. And then remember all members accesses are really done through this->.)

So what are the members types during the execution of bar? They are:

// const-pointer-to-non-const, where the pointer points cannot be changed
foo* const me;

// foo& const is ill-formed, cv-qualifiers do nothing to reference types
foo& self; 

// same as const int
int const i; 

Of course, this is a red-herring, as the important thing is the const-qualification of the pointed to objects, not the pointers. (Had k above been const int* const, the latter const is irrelevant.) We now consider:

int main(void)
{
    foo f;
    f.bar(); // UB?
}

Within bar, both me and self point to a non-const foo, so just like with int i above we have well-defined behavior. Had we had:

const foo f;
f.bar(); // UB!

We would of had UB, just like with const int, because we would be modifying a const-qualified object.

In your question, you have no const-qualified objects, so you have no undefined behavior.


And just to add an appeal to authority, consider the const_cast trick by Scott Meyers, used to recycle a const-qualified function in a non-const function:

struct foo
{
    const int& bar(void) const
    {
        int* result = /* complicated process to get the resulting int */
        return *result; 
    }

    int& bar(void)
    {
        // we wouldn't like to copy-paste a complicated process, what can we do?
    }

};

He suggests:

int& bar(void)
{
    const foo& self = *this; // add const
    const int& result = self.bar(); // call const version
    return const_cast<int&>(result); // take off const
}

Or how it's usually written:

int& bar(void)
{
    return const_cast<int&>( // (3) remove const from result
            static_cast<const foo&>(*this) // (1) add const to this
            .bar() // (2) call const version
            ); 
}

Note this is, again, perfectly legal and well-defined. Specifically, because this function must be called on a non-const-qualified foo, we are perfectly safe in stripping the const-qualification from the return type of int& boo() const. (Unless someone shoots themselves with a const_cast.)


To summarize:

struct foo
{
    foo(void) :
    i(),
    self(*this), me(this),
    self_2(*this), me_2(this)
    {}

    const int& bar(void) const
    {
        return i; // always well-formed, always defined
    }

    int& bar(void) const
    {
        // always well-formed, always well-defined
        return const_cast<int&>(
                static_cast<const foo&>(*this).
                bar()
                );
    }

    void baz(void) const
    {
        // always ill-formed, i is a const int in baz
        i = 5; 

        // always ill-formed, me is a foo* const in baz
        me = 0;

        // always ill-formed, me_2 is a const foo* const in baz
        me_2 = 0; 

        // always well-formed, defined if the foo pointed to is non-const
        self.i = 5;
        me->i = 5; 

        // always ill-formed, type points to a const (though the object it 
        // points to may or may not necessarily be const-qualified)
        self_2.i = 5; 
        me_2->i = 5; 

        // always well-formed, always defined, nothing being modified
        // (note: if the result/member was not an int and was a user-defined 
        // type, if it had its copy-constructor and/or operator= parameter 
        // as T& instead of const T&, like auto_ptr for example, this would 
        // be defined if the foo self_2/me_2 points to was non-const
        int r = const_cast<foo&>(self_2).i;
        r = const_cast<foo* const>(me_2)->i;

        // always well-formed, always defined, nothing being modified.
        // (same idea behind the non-const bar, only const qualifications
        // are being changed, not any objects.)
        const_cast<foo&>(self_2);
        const_cast<foo* const>(me_2);

        // always well-formed, defined if the foo pointed to is non-const
        // (note, equivalent to using self and me)
        const_cast<foo&>(self_2).i = 5;
        const_cast<foo* const>(me_2)->i = 5;

        // always well-formed, defined if the foo pointed to is non-const
        const_cast<foo&>(*this).i = 5;
        const_cast<foo* const>(this)->i = 5;
    }

    int i;

    foo& self;
    foo* me;
    const foo& self_2;
    const foo* me_2;
};

int main(void)
{
    int i = 0;
    {
        // always well-formed, always defined
        int& x = i;
        int* y = &i;
        const int& z = i;
        const int* w = &i;

        // always well-formed, always defined
        // (note, same as using x and y)
        const_cast<int&>(z) = 5;
        const_cast<int*>(w) = 5;
    }

    const int j = 0;
    {
        // never well-formed, strips cv-qualifications without a cast
        int& x = j;
        int* y = &j;

        // always well-formed, always defined
        const int& z = i;
        const int* w = &i;

        // always well-formed, never defined
        // (note, same as using x and y, but those were ill-formed)
        const_cast<int&>(z) = 5;
        const_cast<int*>(w) = 5;
    }

    foo x;
    x.bar(); // calls non-const, well-formed, always defined
    x.bar() = 5; // calls non-const, which calls const, removes const from
                 // result, and modifies which is defined because the object
                 // pointed to by the returned reference is non-const,
                 // because x is non-const.

    x.baz(); // well-formed, always defined

    const foo y;
    y.bar(); // calls const, well-formed, always defined
    const_cast<foo&>(y).bar(); // calls non-const, well-formed, 
                               // always defined (nothing being modified)
    const_cast<foo&>(y).bar() = 5; // calls non-const, which calls const,
                                   // removes const from result, and
                                   // modifies which is undefined because 
                                   // the object pointed to by the returned
                                   // reference is const, because y is const.

    y.baz(); // well-formed, always undefined
}

I refer to the ISO C++03 standard.

GMan
Probably the best SO answer I've read for a long while. Clear, well explained and answers exactly what I was after. Thanks, hope you get the rep you deserve for this :)
AshleysBrain
A: 

Without actually getting to whether it is/should/could be allowed, I would greatly advice against it. There are mechanisms in the language for what you want to achieve that don't require writing obscure constructs that will most probably confuse other developers.

Look into the mutable keyword. That keyword can be used to declare members that can be modified within const member methods as they do not affect the perceivable state of the class. Consider class that gets initialized with a set of parameters and performs a complex expensive calculation that may not be needed always:

class ComplexProcessor
{
public:
   void setInputs( int a, int b );
   int getValue() const;
private:
   int complexCalculation( int a, int b );
   int result;
};

A possible implementation is adding the result value as a member and calculating it for each set:

void ComplexProcessor::setInputs( int a, int b ) {
   result = complexCalculation( a, b );
}

But this means that the value is calculated in all sets, whether it is needed or not. If you think on the object as a black box, the interface just defines a method to set the parameters and a method to retrieve the calculated value. The instant when the calculation is performed does not really affect the perceived state of the object --as far as the value returned by the getter is correct. So we can modify the class to store the inputs (instead of the outputs) and calculate the result only when needed:

class ComplexProcessor2 {
public:
   void setInputs( int a, int b ) {
      a_ = a; b_ = b;
   }
   int getValue() const {
      return complexCalculation( a_, b_ );
   }
private:
   int complexCalculation( int a, int b );
   int a_,b_;
};

Semantically the second class and the first class are equivalent, but now we have avoided to perform the complex calculation if the value is not needed, so it is an advantage if the value is only requested in some cases. But at the same time it is a disadvantage if the value is requested many times for the same object: each time the complex calculation will be performed even if the inputs have not changed.

The solution is caching the result. For that we can the result to the class. When the result is requested, if we have already calculated it, we only need to retrieve it, while if we do not have the value we must calculate it. When the inputs change we invalidate the cache. This is when the mutable keyword comes in handy. It tells the compiler that the member is not part of the perceivable state and as such it can be modified within a constant method:

class ComplexProcessor3 {
public:
   ComplexProcessor3() : cached_(false) {}
   void setInputs( int a, int b ) {
      a_ = a; b_ = b;
      cached_ = false;
   }
   int getValue() const {
      if ( !cached_ ) {
         result_ = complexCalculation( a_, b_ );
         cached_ = true;
      }
      return result_;
   }
private:
   int complexCalculation( int a, int b );
   int a_,b_;
   // This are not part of the perceivable state:
   mutable int result_;
   mutable bool cached_;
};

The third implementation is semantically equivalent to the two previous versions, but avoid having to recalculate the value if the result is already known --and cached.

The mutable keyword is needed in other places, like in multithreaded applications the mutex in classes are often marked as mutable. Locking and unlocking a mutex are mutating operations for the mutex: its state is clearly changing. Now, a getter method in an object that is shared among different threads does not modify the perceived state but must acquire and release the lock if the operation has to be thread safe:

template <typename T>
class SharedValue {
public:
   void set( T v ) {
      scoped_lock lock(mutex_);
      value = v;
   }
   T get() const {
      scoped_lock lock(mutex_);
      return value;
   }
private:
   T value;
   mutable mutex mutex_;
};

The getter operation is semantically constant, even if it needs to modify the mutex to ensure single threaded access to the value member.

David Rodríguez - dribeas