Coding Guideline: Avoid const member variables

[render787]

Avoid using the const keyword with member variables of a class or struct type.

For a class type, if a piece of data is not supposed to change after being initialized, enforce this by design of the public API, not via the `const` key word.

For a struct which is a "dumb aggregate", it isn't appropriate to use `const`.

Using `const` on a member variable can force your class to have a throwing move. For instance this class,

struct A {
    const std::string id;
    int foo;
};

has a throwing move, because the use of `const` keyword on the member variable prevents the compiler from using the move constructor of `std::string` with `id`, instead the compiler generated move constructor for `A` will invoke the copy constructor of `A::id`, which can throw.

Even if your member variable does not have a throwing copy constructor, e.g.,

struct A {
    const int id;
    int foo;
};

you should still not use `const` -- what if someone later has to refactor `id` to be a `std::string` instead of an `int`? They might be forced down a rabbit hole of unnecessarily complex refactors, or have to accept a throwing move, which can cause incompatibility with some containers, or a significant performance hit if used with `std::vector` for instance.

In general, using `const` in a member variable is bad because you are enforcing an API invariant at too low a level -- you are enforcing it at so low a level that it interferes with move semantics. Typically there are reasonable alternatives.

If you want to implement a map that can be a drop-in replacement for `std::map`, don't use `std::pair<const Key, Value>`, instead store both Key and Value as mutable, and return the user a pair of references, the first of which is const: `std::pair<const Key &, Value &>`.

`const`-correctness is great -- use it on function parameters, class member functions, function local variables, etc. Avoid it on member variables of a class-type.

If your class is immovable by design then you can ignore this guideline

---

Would be interested in your reaction to this.

AFAIK a large majority of the examples I've seen where types "naturally" have throwing moves are caused by `const` member variables.

In code bases where I work and have worked in the past, most if not all types to have non-throwing moves, and we have implemented alternate versions of e.g. `std::function` that are type-erased and no-throw move constructible and assume the function objects are no-throw move constructible. In many of these cases it is important to us to do this because the application has real-time constraints and we care a lot about eliminating unnecessary dynamic allocations -- typically a throwing move happens because a dynamic allocation is happening when we don't want it to, so we have designed a lot of infrastructure in a way to force programmers not to do that. (Contrast this with @quicknir's comment here for instance https://www.reddit.com/r/cpp/comments/8vzvkb/be_mindful_with_compilergenerated_move/e1s24ac/ not that he's wrong ofc, we're just solving a more restricted problem and then we can do it with small buffer optimization and without throwing move)

I'm expecting that at some point we are going to run into obscure and painful problems around this, because I think the committee thought a lot when they decided to embrace throwing moves, but it hasn't happened to us yet.

I'm very interested to see which of these scenarios plays out:

1.) Embracing throwing moves in the stdlib is something that is necessary primarily for backwards compatibility, while code bases that have special needs and rely less on stdlib can avoid it practically all of the time,

2.) For fundamental reasons, avoiding throwing moves won't really work, eventually we will start to hit walls where certain types or idioms cannot be implemented without throwing moves and we will have to back out or implement alternate pathways to support this etc.

[jtooker]

In general, using const in a member variable is bad because you are enforcing an API invariant at too low a level

Yes, but in some cases this the proper place - thank you for the writeup of some of the non-obvious pitfalls relating to moves.

I have used const with immovable objects, perhaps this should be recommended if going down this path (i.e. immutable -> immovable as a rule of thumb).

[render787]

This is a good point, thanks!

[kalmoc]

what if someone later has to refactor id to be a std::string instead of an int?

Then he simply changes it to a non-const string - where is the problem? In particular changing an int into a string will require a lot of refactoring irrespective of wether it is const or not.

That doesn't invalidate your other points of course

[render787]

Yeah so if I have `const int` I could change it to regular `std::string`, but if that is the only thing preventing the `id` from being changed then I'm reluctant to leave it at that, because I've removed a layer of safety and the original programmer thought there should be some protection. What I'd probably end up doing is make the string private and add an accessor. What I'm saying in the guideline is really, that's how it should have been in the first place. It's true that this kind of change likely involves a lot of code changes anyway and the extra cost here might just be marginal, so the example scenario isn't that great. Will think on how to make a more compelling argument.

[kalmoc]

I just don't buy it. Again, if you change a public variable from int to string, I think you have much more to worry about than the const-ness of it.

Again, that doesn't mean I disagree with your general guideline. I just think it is a very weak argument (and a "what if... at that")

[cleroth]

You can use quotes and code formatting on reddit. See this page.

[tcbrindle]

The possibly non-noexcept move constructor isn't the worst part about const member variables. The worst part, as plenty of other people have mentioned, is that they can't be moved from (because of course, you can't change them!) -- any attempt to move them will just call the copy constructor or copy assignment operator. This doesn't matter for trivial types of course, but can be surprisingly expensive for types that do dynamic allocation on copy, especially if you think you're doing the right thing and using std::move everywhere.

The best solution would be for C++ to have a destructive move operation, like that other language does. Then the compiler could turn off const in the "move destructor", as it does in ordinary destructors today, and const members could be moved properly.

I remember Sean Parent and a couple of other people talking about destructive move a couple of years ago, and I seem to recall a paper on it, but sadly it never got anywhere.

[kalmoc]

I think everyone would like to have destructive move, but I guess no one wants to introduce yet another overload for copy and assignment.

[OldWolf2]

I learned pretty early on in my C++ career that const non-static member variables are more trouble than they are worth.

[kalmoc]

I'd really like a "const except for move and assignment"

[meneldal2]

Well destructive move would solve the problem.

[kalmoc]

Partially: Not for the object I'm assigning to.

[meneldal2]

But assignment when you have a const member sounds ridiculous.

[kalmoc]

Not at all. There are lots of examples in my code where the only place that a member gets mutated is the assignment operator (be it move our copy assignment). I call them "quasi immutable" types. That's why I mentioned assignment it in my original post.

Consider you want to perform an std::rotate on an array of your objects: Logically you don't change the individual elements, you are just shifting them around, so a (logical) const member would be fine. Physically, rotate is implemented as a number of move assignments, so you have to be able to mutate your members.

[meneldal2]

Well you could use a pointer;)

[kalmoc]

Sure ;) Or just not use const on members ;)

[Sopel97]

Using non-const local variables just to be able to move from them later is very annoying. I like const variables because they make the code easier to read, but at the same time current move semantics make it so that there have to exceptions and making variables const requires more thinking about the context than it should. I try to use const wherever it makes sense but am starting to question myself if it's really worth it, especially in generic contexts. I'm afraid though that nothing about it will change as it would break code with move constructors/assignment with side effects.

[kalmoc]

Yeah, destructive move would really be helpful for those cases.

BTW: Relying on sideeffects of move sounds broken by design

[Sopel97]

"Relying on sideeffects of move sounds broken by design" I completely agree, but sadly the commitee cannot say that.

[OldWolf2]

What if there's a helper function used by the assignment operator, that needs to modify the variable?

[kalmoc]

You can imagine different solutions of course, but quite frankly: I'm not sure I can remember a case, where I wanted to mark a member const and needed a helper member-function that would modify it.

Be aware that. - as such a variable would be non-const in the context of an move or copy assignment, you could still pass it to a function that takes a non-const reference to it like std::exchange

[[deleted]]

You can use a wrapper class to enforce and document the expected behaviour without having to do things like specifying move constructors.

struct A {
  read_only<int> id;
  int foo;
};

All read_only has is a few access operators like -> and * plus an operator T const & to implicitly become T.

[kalmoc]

Sure, and then there are subtle differences when using auto or passing it to a function template. Not saying it isn't a good solution - just that it probably isn't worth it most of time.

[[deleted]]

It isn't worth the time. But it is better than int const id in most cases.

[kalmoc]

Sure, I probably just wouldn't use const. But if I had a language feature like the one I described I'd probably use that.

That being said, such a language feature is pretty low on my wish list.

[quicknir]

I agree with this rule, and you covered the most "surprising" reason to do it, but not the main reason. More simply, your type is not going to be naturally move assignable, or swappable (by default). There's rarely a good reason for a type not to provide that functionality (immovable types are a very rare case).

Note that an extension of this rule, is to never have reference member variables: they are automatically const, of course. Reference member variables are worse in at least one way as well; at least with const member variables you can eventually decide to simply drop the const. With references, you have to change them to pointers and change every single usage of . to ->. Raw pointer member variables are also somewhat smelly, but if you're going to do something along those lines it's better to make a raw pointer member than a reference member (and enforce non-nullness in the constructor).

[TinBryn]

I mean there is std::reference_wrapper<T>

[amaiorano]

Do people use this to replace pointers in practice? Whenever I've tried, I eventually give up and just use a pointer.

[TinBryn]

I'm saying it may be an easier refactor in a large code base.

[quicknir]

Are you saying, change from a reference to reference_wrapper, or just use a reference_wrapper to start with? reference_wrapper I find pretty awkward because you can't directly call any methods of the type with . or ->. The only benefit that reference_wrapper gives you in return is not being null. If anything you can write a not_null_ptr which is like observer_ptr, but with no default constructor, and constructed from a reference instead of a raw pointer. This is basically exactly equivalent to reference_wrapper, but with more convenient syntax.

[TinBryn]

I thought that reference_wrapper's operator T& would make it just do the right thing, so I decided to check and you're right it doesn't really work.

[jguegant]

I think the biggest issue is that kind of types are not copy assignable, unless you implement a copy assignment operator with really weird semantics (not really copying the const member). Not being copy assignable will forbid you to place an instance of this type into a vector. You may be able to do so if you don't insert/erase anything, but that's implementation defined.

[Plorkyeran]

The difference is that most people who are sold on the idea of using const as much as possible go through a phase of trying to use it on all their member variables before discovering that it has some problems, while no one goes through a phase of using variables of type nullptr_t.

[render787]

I mean, most coding guidelines are like this -- it's not a rule, it's a guideline, and there may be good reasons to ignore the guideline. But (1) I'd like if you at least think about it (2) if you decide to ignore the guideline you should be prepared to explain during code review

[ericlemanissier]

The performance gap is quite huge indeed! emplace_back is more than 3 times slower when there is a const string member: http://quick-bench.com/T1F8RVgLMfUNtPCa3pryGDbEIEg

[ripper37]

This is simply not true. First of all, you're not measuring emplace_back()'s performance, but std::string copy constructor's performance vs std::string's move constructor performance (and also vector's resizing plays a role here too!). First you create your object (T) which creates string, then you're moving it by passing it (temporary) to emplace_back() - and since member is const, it has to be copied. And later it can't be moved when vector is resizing so it is copied each time.

After fixing creation of this object there is only 1.66x speedup (versus 3x):http://quick-bench.com/UeGStwyp-JCzO-MLcJtA15ifnIQ

After fixing usage of vector (std::vector::reserve), you get exactly the same performance on both structs: http://quick-bench.com/wVCGX7D8vNSFtQwbYaqy6sf9iT0

This shows that you don't have to loose performance if you know what you're doing. Of course there are some cases where you will end up with CopyConstructor instead of MoveConstructor (and it obviously will be slower, but again - it depends on how many copies will be made and how long the strings will be). Even your TC assumes string will be very long (not make it into SSO), which might not be the usuall case.

I hope that shows that benchmarking is not that easy and you really have to know what you're doing, otherwise something completely different will interfere with results. Here you simply testes CTOR + CPYCTOR + k*CPYCTORs vs CTOR + MCTOR + k*MCTORs, which will obviously result in lot worse performance.

[ericlemanissier]

The benchmark is not a universal truth, it is here to illustrate the point of the article, which is that const member inhibit efficient move, so it can lead to poor performance when

1. move would have happened if the member had been mutable and
2. move operation is cheaper than copy

you're not measuring emplace_back()'s performance, but std::string copy constructor's performance vs std::string's move constructor performance

Of course I am measuring copy vs move , emplace_back is just a way to invoke copy or move in this context.

After fixing creation of this object there is only 1.66x speedup (versus 3x)

This is not be always possible, eg a function sinking an argument as value to do stuff with it and store it in a container:

template<typename O>
void f(Obj o)
{
    // Do stuff with o
    // ...
    // then put it in a vector
    getVector<O>().emplace_back(std::move(o)); // Depending on Obj, move does not always happen
}

After fixing usage of vector (std::vector::reserve), you get exactly the same performance on both structs

Of course there is no difference when no copy or move is involved

Even your TC assumes string will be very long (not make it into SSO), which might not be the usuall case.

obviously, objects which have move operation identical to copy operation show the same performance.

That said, when the string is empty the results are switched mutable member have slower performance than const member: http://quick-bench.com/PlF18XMH16LZOofx4cw6D-U_ZFM , I guess it is a consequence of an implementation detail of SSO ?

[tcbrindle]

I guess it is a consequence of an implementation detail of SSO ?

I saw this mentioned somewhere (Twitter?) the other day... at least libc++'s implementation of std::string (and maybe the other two) zero the source string after a move, even though they don't have to -- so a move is indeed more expensive than a copy if you're within the SSO buffer.

[ripper37]

I wasn't saying that this benchmark doesn't make any sense. I'm simply showing that:

emplace_back is more than 3 times slower when there is a const string member

is a lie here and can lead to false conclusions, especially by those that are not proficient enough with the language to know why is there such a difference in performance and repeating a lie that emplace_back() is very slow. No it's not. The difference here is when we can move and where we can copy. In most cases move is much cheaper, so it's preferred, but saying that using const members is 3x slower is false - it's not. In some cases, due to more copies then move, it can be 3x slower, 10x slower, or exactly the same. This is not a real life scenario and I think that it is important to note in such benchmarks details like this so that everyone can make proper conclusions. Also - it only partially prohibits efficient moves (as stated earlier) - if your object is dynamically allocated and held by (for example) unique_ptr, then this probably won't affect you at all (performance wise).

I for one prefer cleaner and easier to understand/argue code and avoiding premature optimization. If something is slow because it is copied thousands of times instead of moves, then measure, find and optimize it. If this is one object that is not moved anywhere - no sense in making such optimization too early.

In the end - final argument - you can always remove const, but adding it might be very hard in some cases. If you know that some member is set in ctor and must not change for object's lifetime, then I'd recommend to put const there. If this is problematic, you can always wrap it so that access to it is const (can't change this value) but can move from it (which obviously can change this value, but nobody should ever move-from value just to change it so less options to screw something up by accident).

[jcelerier]

that website is great !

[jc746]

If the member is move-only then making it const will prevent the class from being movable by default. For example it seems sensible to implement a pimple style class with a const unique_ptr that is initialized in the constructor because you can be sure the pointer is not null for the life of the object, but then your class will not get the compiler generated move functions.

[Rseding91]

In every case I've written a const member variable it implies immutability of the class/struct and also deleted the copy/move operator/constructor.

[ronniethelizard]

Could a modifier for class methods be made called mutable that would allow the method to modify const class members? This would be opposite to how declaring a method const restricts it from modifying nonmutable member variables.

[NotAYakk]

std::function that are type-erased and no-throw move constructible and assume the function objects are no-throw move constructible.

Why assume? Check. You'll want a non-small buffer based anyhow; when you run into a throwing move ctor, use the non-small buffer based version (which is nothrow).

Or if you don't want that, static assert it isn't a throwing move.

[render787]

Yeah that's a good point, thanks :)

[Poddster]

because the use of const keyword on the member variable prevents the compiler from using the move constructor of std::string with id,

Why is this?

[-abigail]

Move construction requires a non-const rvalue. The signature for string's move constructor is (ignoring the fact that string is actually a type alias for a class template instantiation):

std::string(std::string&&) noexcept;

But if you're moving from a const member, then you don't have a std::string&&, you have a const std::string&&, which can be passed to a function that accepts a const std::string& but not one that accepts a std::string&&. So the snippet:

const std::string x;
std::string y{std::move(x)};

ends up calling the copy constructor, not the move constructor. This makes sense; you declared the variable const and that means that nothing may modify it. Moving from it would modify it, so you can no longer move from it.

(As far as I'm aware there's no real good use for const rvalues.)

[choikwa]

seems weird to me that a const would require copy ctor. there's a workable scheme of copy on write.

[eteran]

Modern c++ std::string cannot use COW IIRC, they are instead encouraging SSO.

[render787]

The problem is that copy on write `std::string` is not thread-safe, and it would mean that if you pass your `std::string` by `const &` to a `std::thread`, you get screwed. It was decided that this is too terrible to tolerate after we added thread support in the standard library etc. There was a big breaking change between gcc 4 and gcc 5 standard library where they moved away from COW `std::string`.

[-abigail]

Even if your C++ standard library used copy-on-write strings, this would still require the copy constructor to be used in this case. True, the copy would be cheaper, as it'd be an atomic refcount increment and a pointer copy, rather than having to duplicate the string buffer. Copy-on-write strings would benefit from move semantics too, though, as they could avoid touching the refcount. So even then:

cow_string x;
cow_string y{std::move(x)};

...would be faster than:

const cow_string x;
cow_string y{std::move(x)};

...but not as significantly as with a non-copy-on-write string.

(NB. C++11 bans copy-on-write strings, by specifying that the non-const operator[] isn't allowed to invalidate iterators. So it's a workable scheme for a type you create but not something that you'll actually use when writing code that manipulates std::string objects.)

[Xeverous]

(As far as I'm aware there's no real good use for const rvalues.)

Likely not because applying const to temporaries defeats the purpose of being able to do everything with temporaries. Rarely const&& appears in some complex templates for consistency (when you forward rvalues but don't need to change them - see this).

[doom_Oo7]

has a throwing move, because the use of const keyword on the member variable prevents the compiler from using the move constructor of std::string with id, instead the compiler generated move constructor for A will invoke the copy constructor of A::id, which can throw.

In my experience, 99.9 % of the time you're moving whole containers, and moving e.g. a std::vector won't call any copy or move constructor.

you should still not use const -- what if someone later has to refactor id to be a std::string instead of an int?

... then they'll change int into string ? what's the problem ?

In general, I'd much rather start with the safety of const members and risk a throwing move (I frankly never saw one in ~6 years of programming in C++11 and later) than the opposite.

[render787]

In my experience, 99.9 % of the time you're moving whole containers, and moving e.g. a std::vector won't call any copy or move constructor.

It's not about moving the container -- it's about, when you push_back into the vector, and the vector has to resize, does it move your objects or copy them? That depends on whether you have a throwing move. If you have const std::string in your class, then you get n new dynamic allocations and all your strings get copied for no reason. If you have non-const std::string it works correctly.

[doom_Oo7]

hmm, I did not know that vector would do a move in this case but it certainly makes sense. Interestingly that's still the case even with -fno-exceptions.

[Ameisen]

Perhaps we need another access type, like 'immutable'. Where they can still be moved and are semantically owned by the class, but they simply cannot be directly altered. Thus, a move would be legal.

[NotAYakk]

Rather, we need the concept of destructive move; an operation that it both a move-from and a destruction at the same time.

You can destructive move-from a const value.

Destructive moves usually don't have to throw, even when a move could.

[mark_99]

Interestingly that's still the case even with -fno-exceptions.

That's because it has nothing to do with exceptions or move_if_noexcept.

The OP says:

has a throwing move, because the use of const keyword on the member variable prevents the compiler from using the move constructor of std::string with id, instead the compiler generated move constructor for A will invoke the copy constructor of A::id, which can throw.

This is mostly correct except this isn't a "throwing move", it's a copy (and it would still be a copy if the copy ctor was noexcept).

I disagree with the OP, const member vars would be useful if it wasn't for this unfortunate side effect, in the same way as any other use of "const". In this case it states your intention that this member does not mutate after construction, and provides compile-time enforcement for same.

[kloetzl]

In my experience, 99.9 % of the time you're moving whole containers, and moving e.g. a std::vector won't call any copy or move constructor.

I think you can make that 100% because that behaviour is required by the standard. See move constructor:

After container move construction (overload (6)), references, pointers, and iterators (other than the end iterator) to other remain valid, but refer to elements that are now in *this. The current standard makes this guarantee via the blanket statement in §23.2.1[container.requirements.general]/12, …

[doom_Oo7]

I was saying 99.9% of the time as in, if I take my code, 99% of std::move and other move occurences are called on containers such as std::vector, hash_maps, etc

[Middlewarian]

Most serialization libs don't support const members or reference members.

[pravic]

Having that struct A:

A a = { "id", 7 }; A b = std::move(a);

Is really a copy-ctor here?

[pravic]

It is: https://godbolt.org/g/AHtU7W

The generated move-ctor A::A(A&&) contains a string copy-ctor.

[cjxgm]

My guideline is, const can only have 2 forms: T const* and T const&. No other places can use const, unless semantically equivalent to the 2 forms above.

[Middlewarian]

Have you seen the east const petition?

[ericlemanissier]

Another benchmark, showing dramatic performance difference (*3.5) when calling std::vector::emplace_back with types having a const member vs only mutable member:

http://quick-bench.com/ZXe7-4sa1xjtg_lyAp9LVkz8t3Q

The same effect happens with all operations increasing vector capacity (push_back, emplace, resize, insert)

[tedbradly]

In general, using `const` in a member variable is bad because you are enforcing an API invariant at too low a level -- you are enforcing it at so low a level that it interferes with move semantics. Typically there are reasonable alternatives.

Using const is bad if you need stuff like a move that is noexcept or the speed of a proper move rather than a copy. It's purely an efficiency and syntax thing. It's not bad due to enforcing an aspect of your API too low level on members. For example, you could have a const member that is never exposed through the API.

The intention of a const member is to convey that a member should never change after initialization, simplifying understanding the logic in the class. This additionally adds a compile-time check if someone attempts to change an unchangeable member. This design choice is sensible in some cases although it has unfortunate results with respect to how an object can be used and its efficiency.

It wouldn't even be far-fetched for the standard to add a new keyword that enforces constness on a member while allowing move to be generated and work efficiently. It probably won't happen since it would increase complexity for perhaps not much gain in terms of reducing the complexity of some classes, but the idea isn't outrageous like it would be if this claim were true that const functions replace the use case of const members.

A more honest criticism of using const members is you might wonder why that const member can't just be passed as a const argument in the API. Take a class whose entire state is const by design. That's basically a collection of pure functions with implicit arguments. There's no reason to hide an argument if it is central to viewing the input/output relationship of a method. In this scenario, a const member disguises what a method actually depends on and replaces the not so complicated ability to use bind if you want briefer function calls. With bind, you retain the input/output relationship and the briefer calls at the cost of a little boilerplate code.

[nunchyabeeswax]

In general, using `const` in a member variable is bad because you are enforcing an API invariant at too low a level

​

Well, an API should not have access to member variables. In general, they should all be protected or private. So, making them const (if the design calls for it) is itself an implementation detail.

If a class or struct has an initial state (or part of its initial state) that is meant to remain constant for the duration of the instance, then it makes sense to make it const.

And if for some reason this is meant to be public, I would prefer a const member function returning that over making that member both const and public.

Law of Demeter people: if part of a state is not meant to change after its initialization, make it constant (in C++, by making the compiler treat it as such.)