3

---- Begin Edit ----
User @user17732522 pointed out the flaw that invokes UB is from the fact pop_back() invalidates the references used according to the vector library documentation. And constexpr evaluation is not required to detect this when it occurs as it's not part of the C++ core.

However, the fix, which was also pointed out by @user17732522, is simple. Replace occurrences of these two consecutive lines of code:

v.pop_back();
v.emplace_back(...);

with these two lines:

std::destroy_at(&v[0]);  // optional since A has a trivial destructor
std::construct_at<A, int>(&v[0], ...);

---- Begin Original ----

While references are invalidated upon the destroy_at, they are reified automatically by the construct_at. See: https://eel.is/c++draft/basic#life-8,

It's well established you can't modify const values unless they were originally non const. But there appears an exception. Vectors containing objects with const members.

Here's how:

#include <vector>
#include <iostream>

    struct A {
        constexpr A(int arg) : i{ arg } {}
        const int i;
    };

int main()
{
    std::vector<A> v;
    v.emplace_back(1);  // vector of one A initialized to i:1
    A& a = v[0];

    // prints: 1 1
    std::cout << v[0].i << " " << a.i << '\n';

    //v.resize(0); // ending the lifetime of A and but now using the same storage
    v.pop_back();
    v.emplace_back(2);  // vector of one A initialized to i:2

    // prints: 2 2
    std::cout << v[0].i << " " << a.i << '\n';

}

Now this seems to violate the general rule that you can't change the const values. But using a consteval to force the compiler to flag UB, we can see that it is not UB

consteval int foo()
{
    std::vector<A> v;
    v.emplace_back(1);  // vector of one A initialized to i:1
    A& a = v[0];
    v.pop_back();
    v.emplace_back(2);
    return a.i;
}

// verification the technique doesn't produce UB
constexpr int c = foo();

So either this is an example of modifying a const member inside a vector w/o UB or the UB detection using consteval is flawed. Which is it or am I missing something else?

doug
  • 3,840
  • 1
  • 14
  • 18
  • 1
    Compiler not flagging UB can mean two things: (1) it is not an UB; (2) compiler is not sophisticated enough to recognize UB here. – Revolver_Ocelot Apr 03 '22 at 19:05
  • 3
    The ability to “trick” the compiler doesn’t alter the language of the standard – Taekahn Apr 03 '22 at 19:05
  • The compiler is supposed to flag all UB when evaluating compile time constants. This isn't flagged. On the other compiler's I've tested it on. – doug Apr 03 '22 at 19:09
  • For `v.emplace_back(2); // vector of one A initialized to i:1` Why? The `A` has its `i` initialized to 2. – Adrian Mole Apr 03 '22 at 19:13
  • @RichardCritten So it seems. But compilers are required to check for UB and none I've tried do. Are they all buggy? – doug Apr 03 '22 at 19:13
  • Does this help: https://stackoverflow.com/questions/58454657/why-does-a-consteval-function-allow-undefined-behavior – Jakob Stark Apr 03 '22 at 19:14

1 Answers1

11

It is UB to modify a const object.

However it is generally not UB to place a new object into storage previously occupied by a const object. That is UB only if the storage was previously occupied by a const complete object (a member subobject is not a complete object) and also doesn't apply to dynamic storage, which a std::vector will likely use to store elements.

std::vector is specified to allow creating new objects in it after removing previous ones, no matter the type, so it must implement in some way that works in any case.

What you are doing has undefined behavior for a different reason. You are taking a reference to the vector element at v[0]; and then you pop that element from the vector. std::vector's specification says that this invalidates the reference. Consequently, reading from the reference afterwards with a.i has undefined behavior. (But not via v[0]).

So your code has undefined behavior (since you are doing this in both examples).

However, it is unspecified whether UB in standard library clauses of the standard such as using the invalidated reference needs to be diagnosed in constant expression evaluation. Only core language undefined behavior needs to be diagnosed (and even then there are exceptions that obviously shouldn't be required to be diagnosed, since it is impossible, e.g. order-of-evaluation undefined behavior). Therefore the compiler does not need to give you an error for the UB here.

Also, consteval is not required here. constexpr would have done the same since you already force constant evaluation with constexpr on the variable.

user17732522
  • 53,019
  • 2
  • 56
  • 105
  • I really like this answer. I also always struggle to find relevant information on "*[...] and also doesn't apply to dynamic storage*". Could you please provide the relevant standard section that implies that? – Fureeish Apr 03 '22 at 19:29
  • 1
    "However it is generally not UB to place a new object into storage previously occupied by a const object." How is that different from just changing the const value? Note that there seems to be no problem with the use of a reference to that value since it is the same memory location. – doug Apr 03 '22 at 19:30
  • @doug the difference lies in, for example, the lifetime of an object and rules around it. – Fureeish Apr 03 '22 at 19:31
  • @Fureeish I agree but why does the consteval function not flag UB when returning a reference to the initial int after ending it's lifetime? Bug? – doug Apr 03 '22 at 19:33
  • @doug "there seems to be no problem with the use of a reference to that value since it is the same memory location" IIRC it is an UB to use reference to the variable whose lifetime ended, even if new object was constructed in tis place. – Revolver_Ocelot Apr 03 '22 at 19:35
  • 1
    @Fureeish https://www.eel.is/c++draft/basic#life-10 lists only static, thread and automatic storage duration. – user17732522 Apr 03 '22 at 19:36
  • 1
    I do not understand your answer. `a` is clearly a reference to the original object at `v[0`], which is is subsequently destroyed, and then the reference is later read through. IFAIK it does not matter that a new object was placed back into that memory, the lifetime of the referent ended. A reference is not just to a place in memory, but to a specific object in that memory. As soon as its lifetime ends, the reference is invalidated. What am I missing? – Chris Uzdavinis Apr 03 '22 at 19:37
  • @doug The standard says that it is different, so it is. The reference to the old vector element is invalidated and cannot be used anymore. That is what I overlooked and fixed just now in my answer. The situation would be more nuanced if you had created the new object in the storage yourself. – user17732522 Apr 03 '22 at 19:37
  • @Revolver_Ocelot Whether or not a reference to the old object will become a reference to the newly created object depends on whether the objects are _transparently replaceable_, see https://www.eel.is/c++draft/basic#life-8. But that doesn't matter here, since the specification of `std::vector` says that the reference will be invalidated. – user17732522 Apr 03 '22 at 19:39
  • 1
    @ChrisUzdavinis Please note that I updated my answer, I had not noticed that OP used the old reference before. However, see also my previous comment. – user17732522 Apr 03 '22 at 19:41
  • So the UB is associated with the stale reference? That is what surprised me originally when I was wondering why it seemed to work. So that suggests the problem is a compiler bug because the stale reference isn't detected? – doug Apr 03 '22 at 19:46
  • 1
    @doug It doesn't need to detect it, see my edited answer. Not producing any diagnostic is allowed and if you had placed the new object manually with `std::construct_at` at the location instead of using `pop_back` and `emplace_back`, then using the old reference would be completely fine, see my previous comments. – user17732522 Apr 03 '22 at 19:48
  • "However, it is unspecified whether UB in standard library clauses of the standard such as using the invalidated reference needs to be diagnosed in constant expression evaluation." Cool. If you link to a reference on this exception I'll mark this as the answer. – doug Apr 03 '22 at 19:53
  • @doug https://www.eel.is/c++draft/expr.const#5.sentence-2 – user17732522 Apr 03 '22 at 19:55
  • Thanks. Now the interesting question is whether peeling out the minimal code from vector required to replicate would be detected. I do know that the standard library only has to produce correct results and isn't bound to use only non-UB constructs as it's compiler specific. – doug Apr 03 '22 at 20:51
  • @doug The standard library does not need to follow the language rules for user code, but will in practice except for some compiler specific guarantees and built-ins. This will be especially true for code used in constant expressions. But as I mentioned above, if you replicate the `std::vector` implementation yourself, you will probably end up with code where what you are doing in the question has defined behavior. – user17732522 Apr 03 '22 at 20:55
  • Found this in buried in MSVC emplace_back: return __builtin_is_constant_evaluated(); Implimentation is not in the library source. So can't implement in core. – doug Apr 03 '22 at 21:17
  • 1
    @doug https://en.cppreference.com/w/cpp/types/is_constant_evaluated – user17732522 Apr 03 '22 at 21:18
  • So clearly, if one just uses the allocator to do the same sequences shown in the vector, then there would be no UB. This is what vector does but it would be difficult to carve out an exception for references and say they are only invalid until a new object occupies the location of the old one. However, that is specified for the allocator https://eel.is/c++draft/basic#life-8 – doug Apr 04 '22 at 04:11
  • @Fureeish Turns out you were onto something. Easy fix to avoid UB – doug Apr 05 '22 at 02:56