clang seems to be overzealous with lifetime errors in constexpr

[TobiSchluter]

Consider this (derived from the Eigen library, tested with clang 13.0.0 and clang 12)

struct B;

#define CONSTEXPR constexpr
#define CONSTEVAL consteval

struct A {
public:
    CONSTEXPR A() { m_val = 1; }
    CONSTEXPR A(const A& other) { m_val = other.m_val;}
    CONSTEXPR ~A() {};

    CONSTEXPR int val() { return m_val; }

    int m_val;

    CONSTEXPR B operator<<(int);
};

struct B {
    A& m_a;
    CONSTEXPR B(A& ref) : m_a(ref) {}
    CONSTEXPR B& operator,(int i) { m_a.m_val = i; return *this; }
    CONSTEXPR ~B() { finished(); }
    CONSTEXPR A finished() { return m_a; }
};

CONSTEXPR B A::operator<<(int i) {
    m_val = i;
    return B(*this);
}

CONSTEVAL int f()
{
    A a{(A() << 1, 2, 3, 4).finished()};
    return a.val();
}

int g()
{
    return f();
}

Godbolt link here: https://godbolt.org/z/njfxf9jP5

As you can see in GCC's intermediate output (obtained by disabling constexpr via macros), ~B is always called before ~A. Nevertheless, clang prints

source>:44:12: error: call to consteval function 'f' is not a constant expression
    return f();
           ^
<source>:13:43: note: read of object outside its lifetime is not allowed in a constant expression
    CONSTEXPR A(const A& other) { m_val = other.m_val;}
                                          ^
<source>:28:37: note: in call to 'A(A())'
    CONSTEXPR A finished() { return m_a; }
                                    ^
<source>:27:22: note: in call to '&A() << 1->finished()'
    CONSTEXPR ~B() { finished(); }
                     ^
<source>:38:10: note: in call to '&A() << 1->~B()'
    A a{(A() << 1, 2, 3, 4).finished()};
         ^
<source>:44:12: note: in call to 'f()'
    return f();
           ^

(I think clang is wrong here, but of course gcc may be getting the destruction order wrong.)

[llvmbot]

@llvm/issue-subscribers-clang-frontend

[Phytolizer]

I've got a more minimal example that seems to be related.

#include <iostream>
#include <memory>

consteval int something() {
  int *p = std::allocator<int>().allocate(1);
  *p = 5;
  int x = *p;
  std::allocator<int>().deallocate(p, 1);
  return x;
}

int main() { std::cout << something() << '\n'; }

Godbolt link: https://godbolt.org/z/6GKnnq4c6

[shafik]

@Phytolizer Your example may not be equivalent, allocate() does not start of the element see [allocator.members]p5.

If you add std::construct_at<int>(p); it now is well-formed: https://godbolt.org/z/TTeYq6KxP

[zygoloid]

Original example reduced further:

struct B {
  int &&n;
  constexpr ~B() { int m = n; }
};
constexpr int k = B{.n = 0}.n;

It certainly looks like we're destroying the temporaries in the wrong order. We should destroy the B temporary first because its construction finished last.

Looking at CallStackFrame::createLocal, it appears that the problem is that we register cleanups at the point where we create the storage for a local object, rather than at the point where we finish the initialization, so we will incorrectly register the B cleanup too early -- before we have evaluated the initializers for its subexpressions and registered their cleanups.

In order to fix this I think we need to move the cleanup registration code out of createLocal, and call it separately at the end of initialization of the variable / temporary object.