const correctness re: reference members

Question

tl;dr is is a good idea for const methods to be able to mutate objects referenced as reference members?

---

Imagine you have some kind of work queue that uses items that look like so:

class Consumable { /* ... */ };

class Consumer
{
   // ...
   virtual void consume(const Consumable &);
};

struct WorkQueueItem
{
  Consumer & consumer;
  Consumable consumable;

  //...

  void run() { consumer.consume(consumable); }
};

Here Consumer::consume is a non-const method.

C++ will allow me to mark run() const. Is this a good idea or not, and why? Here are some conflicting rationales I came up with:

1. C++ is the expert on what counts as const and what doesn't, so you should mark anything you can const as long as your code compiles.
2. The important thing is that WorkQueueItem doesn't "own" the consumer it's pointing at, so const methods of WorkQueueItem are free to call non-const methods of WorkQueueItem::consumer.
3. We cannot tell from the form of this code alone whether it's okay to make run() const, and it's only by considering the concept of a work queue and how it functions that we can conclude that it is/isn't acceptable to do so.
4. If we have a const reference to a WorkQueueItem we should be able to call methods on it and be confident that they don't mutate any program state at all, aside from possibly anything non-const that gets passed in as arguments to the method. Consequently we should not make run() into a const method here.
5. Other (explain).

Obviously I can go with any of these rationales, but which would be considered most idiomatic for C++ code? Also, if the answer is different, which is the best idea?

Answer 1

Unfortunately, the most complex rationale is the correct one: it all depends on context, so your proposal 3.

In the following, I'll treat your Consumer& consumer reference member as a pointer Consumer* consumer because reference members have tons of subtle problems (compare Core Guidelines rule C.12: don't make members const or references).

Discussion of the proposed rationales

1. C++ is the expert on what counts as const and what doesn't, so you should mark anything you can const as long as your code compiles.

This is wrong. The C++ const system is not an exhaustive mutation checker like the one featured in Rust. In particular, const is not transitive. The const only applies to a particular object, not to other objects referenced from the const object. Here, I mean “object” in the formal C++ sense, not in the OOP sense.

2. The important thing is that WorkQueueItem doesn't "own" the consumer it's pointing at, so const methods of WorkQueueItem are free to call non-const methods of WorkQueueItem::consumer.

This is a formally correct position: As per the rules defined in the C++ language, const would only apply to the WorkQueueItem, not to the Consumer. Thus, it wouldn't break fundamental properties of the language to mark run() as const.

But this principle doesn't really provide actionable guidance. It tells us when a method definitely shouldn't be const (when mutating members of an object). Here, the mutation is on a different object, so we don't really have an argument for or against marking run() as const.

3. We cannot tell from the form of this code alone whether it's okay to make run() const, and it's only by considering the concept of a work queue and how it functions that we can conclude that it is/isn't acceptable to do so.

This is the most correct approach. I'll compare this to Core Guidelines rule Con.2 below.

4. If we have a const reference to a WorkQueueItem we should be able to call methods on it and be confident that they don't mutate any program state at all, aside from possibly anything non-const that gets passed in as arguments to the method. Consequently we should not make run() into a const method here.

As discussed for point 1, the C++ const system only describes an individual object, not an entire object graph, and definitely not the entire state of the program. A const-method is not necessarily pure in the functional programming sense.

The standard C++ language provides no facilities for marking a function as pure, though some vendor-specific solutions exist. For example, GCC provides the __attribute__((pure)) for functions that do not make observable mutations to the program state but may depend on such state, whereas __attribute__((const)) functions do not depend on program state other than the argument values. But again, this depends crucially on the C/C++ concept of an object: a function f(char*) cannot be __attribute__((const)) if it dereferences the pointer.

C++ Core Guidelines rule Con.2: By default, make member functions const

link: https://isocpp.github.io/CppCoreGuidelines/CppCoreGuidelines#con2-by-default-make-member-functions-const

This rule says that

A member function should be marked const unless it changes the object’s observable state. […]

so that violations of this rule can be described as

a member function that is not marked const, but that does not perform a non-const operation on any member variable.

This initially sounds like rationale 2, but the interesting question is what “observable state” exactly means.

It is quite common for C++ objects to be implemented in terms of pointers to different objects, e.g. in the pImpl pattern. Mutating the referenced object does not change the pointer itself, so such mutations could be declared const. In particular, this makes sense if the referenced object is a cache for some computation, since updating the cache does not change the observable behaviour (ignoring side channels).

But:

It is the job of the class to ensure such mutation is done only when it makes sense according to the semantics (invariants) it offers to its users.

This is the crux of the issue: you have to think about what guarantees or semantics your WorkQueueItem is supposed to provide, and whether these semantics mean that a mutation of the Consumer is or isn't a mutation of the WorkQueueItem. The const system is a tool for encoding these semantics in order to help the users of your class write maintainable and correct code. You cannot use the type system to discover what your intentions are, other than using the type system to warn you when your intentions are provably inconsistent.

In your particular example, I suspect that running a WorkQueueItem very much changes the state of that item – there is probably a large conceptual difference between a task that has yet to run and a task that has run. For this difference, it doesn't matter whether you represent this change in a field, e.g. bool has_run.

However, if your items are supposed to run at most once, then it could perhaps make sense to declare this operation as an rvalue-method void run() && { ... }, which is the next best thing C++ supports for affine types. But this is debatable.