llvm / llvm-project

The LLVM Project is a collection of modular and reusable compiler and toolchain technologies.
http://llvm.org
Other
29.05k stars 11.98k forks source link

Permitting static constexpr variables in consteval functions #82994

Open mpusz opened 8 months ago

mpusz commented 8 months ago

It seems that Clang does not support static constexpr variables in constexpr functions properly:

https://godbolt.org/z/fx1jTModK

The value of a static constexpr variable set in a constexpr function should be available at runtime.

mpusz commented 8 months ago

BTW, if I make the functions constexpr (and not consteval) the code works as expected.

llvmbot commented 8 months ago

@llvm/issue-subscribers-clang-frontend

Author: Mateusz Pusz (mpusz)

It seems that Clang does not support static constexpr variables in constexpr functions properly: https://godbolt.org/z/fx1jTModK The value of a static constexpr variable set in a constexpr function should be available at runtime.
shafik commented 8 months ago

Confirmed, it would be nice to have a reduction, it is not obvious what is going on here but maybe @cor3ntin might have an idea.

llvmbot commented 8 months ago

@llvm/issue-subscribers-c-20

Author: Mateusz Pusz (mpusz)

It seems that Clang does not support static constexpr variables in constexpr functions properly: https://godbolt.org/z/fx1jTModK The value of a static constexpr variable set in a constexpr function should be available at runtime.
frederick-vs-ja commented 8 months ago

Reduced example (Godbolt link):

#include <cstddef>
#include <cstdio>

template<class T, std::size_t N>
struct my_array {
    T elems_[N];
};

consteval my_array<char, 2> get_array()
{
    my_array<char, 2> result;
    result.elems_[0] = 'a';
    result.elems_[1] = '\0';
    return result;
}

#ifdef CONSTEVAL_DISABLED
constexpr
#else
consteval
#endif
const char* get_buffer_ptr()
{
    static constexpr auto arr = get_array();
    return arr.elems_;
}

int main()
{
    constexpr auto p = get_buffer_ptr();
    static_assert(p[0] == 'a');  // passes with Clang
    static_assert(p[1] == '\0'); // passes with Clang
    // if get_buffer_ptr is consteval, an empty NTBS is printed with Clang,
    // even though static_assert's pass
    std::puts(p);
}

It seems that Clang mistakenly skipped the constant-initialization of static constexpr variables (or mistakenly used zero-initialization only) in consteval functions.

@shafik I think this should be labeled "c++23" as the examples are only well-formed since P2647R1.

shafik commented 8 months ago

@cor3ntin @AaronBallman this feels vaguely familiar but I can't the problem it remind me of

pitust commented 8 months ago
consteval const int* a() {
    static constexpr int b = 3;
    return &b;
}
int c() { return *a(); }

here is an even more minimal reduction (https://godbolt.org/z/5dEr7sdbs)

llvmbot commented 8 months ago

@llvm/issue-subscribers-clang-codegen

Author: Mateusz Pusz (mpusz)

It seems that Clang does not support static constexpr variables in constexpr functions properly: https://godbolt.org/z/fx1jTModK The value of a static constexpr variable set in a constexpr function should be available at runtime.
cor3ntin commented 8 months ago
consteval const int* a() {
    static constexpr int b = 3;
    return &b;
}
int c() { return *a(); }

consteval is an amazing source of runtime bugs!

Because the function a is immediate, its body is not emitted at codegen. and b would have been emitted by the body of a, so it's not emitted.

Maybe we need to have a visitor to extract static constexpr vars from immediate functions (and emit them)? That should be enough.

@erichkeane @AaronBallman

AaronBallman commented 8 months ago

CC @rjmccall @efriedma-quic

zygoloid commented 8 months ago

An extra visitor would be a little expensive. Perhaps local static variables in consteval functions should be handed to the ASTConsumer immediately by Sema, rather than assuming that the consumer will find them indirectly.

rjmccall commented 8 months ago

Hmm. I suppose it's not technically as simple as looking for the declarations referenced by the constant value of the call, since you could leak out a reference to a static local via either a template argument or a local / lambda class. I don't love the idea of forcing IRGen to eagerly track every static local in every consteval function just in case one of them leaks out that way, though. An alternative approach would be to just completely defer to CodeGen and teach it to lazily emit these variables without requiring them to ever be handed to the consumer. (The consumer doesn't usually have to be told about static locals, IIRC; it just implicitly knows about them through the containing function.) That might be a nice compile-time optimization for certain code patterns even without constexpr / consteval — it would let us be lazy about emitting global variables for all static locals with constant initializers.

shafik commented 8 months ago

CC @Fznamznon

shafik commented 2 months ago

Note clang does not behave the same as edg/MSVC here: https://godbolt.org/z/Gzjjj7dP4