ES.50: Don’t cast away const and `mutable`

[KindDragon]

ES.50: Don’t cast away const
Reason

It makes a lie out of const. Note

Usually the reason to “cast away const” is to allow the updating of some transient information of an otherwise immutable object. Examples are caching, memoization, and precomputation. Such examples are often handled as well or better using mutable or an indirection than with a const_cast.

Shouldn't only thread safe class members marked as mutable?

You Don’t Know const and mutable @hsutter

[Amaroker]

You only need to mark a member variable mutable if you will modify it in a const member function. Then, to ensure thread safety, you may need to protect the mutable member with a mutex (or make it atomic).

The member variable itself (non-public of course) doesn't need to be thread safe. The important thing is that only one thread at a time can have access to such modifiable data.

[KindDragon]

Yes, I agree with you. But rule ES.50 doesn't mention that (no rule mention mutable) and developer using that rule can try just mark member variable as mutable to avoid const_cast.

[Amaroker]

OK, I agree that thread-safety should be mentioned in ES.50.

There's an entry in the to-do list that I assume will cover it more in detail:

Const member functions should be thread safe

[KindDragon]

But I don't what to suggest if developer doesn't know is class should be thread safe (thread-safety come with cost)

[Amaroker]

I think that "const member functions should be thread safe" is an excellent guideline in modern C++.

If your const member functions are critical to performance, then you may disregard the guideline at your own risk. But then you also need a strategy how to prevent mixing such unsafe classes with the rest of your thread-safe code.

So, maybe it's better to not use const and mutable at all for the members in such cases.

[KindDragon]

Ok, but sometimes const required by interface in third party library

[BjarneStroustrup]

I added an example using mutable. I think the discussion of thread safety belongs in the concurrency section.

[KindDragon]

Thank you

[Amaroker]

@BjarneStroustrup , there are several mistakes in the examples.

Common for all examples:

• return ->second; should be return p->second;
• if (auto p = cache.find(x))
  p is an iterator, not a bool. It is unsafe to use p directly as shown, since x might not be found in the map.
• Missing semicolon at the end of the class declaration.

In example 2:

• const_cast<map<int,int>>(cache)[x] = val;

Invalid use of const_cast.

In example 3

• get_val should be declared const

In example 4

• cache.find(x))
  cache is a pointer in this example, so dot operator can't be used here.
• cache should not be declared mutable.

[KindDragon]

In example 3 and 4

You only need to mark a member variable mutable if you will modify it in a const member function. Then, to ensure thread safety, you may need to protect the mutable member with a mutex (or make it atomic).

I think we need use std::shared_mutex to access cache member variable. This is the whole purpose of this issue

[BjarneStroustrup]

I added "const" to the two versions of get_val() where it was missing.
And 'no' multi-threading is not the whole purpose of this issue; you can get into serious problems with "casting away const" even in single-threaded code.
I'll consider data races when I revise the examples, but for now, I suspect that this is best left for the concurrency section, possibly with a cross-reference.