std::construct_at() seems to bypass [[clang::require_explicit_initialization]]

[higher-performance]

#include <memory>
#include <utility>

struct S {
    unsigned char v [[clang::require_explicit_initialization]];
};

template <class T, class... U>
constexpr T* my_construct_at(T* ptr, U&&... args) {
  return ::new (static_cast<void*>(ptr)) T(std::forward<U>(args)...);
}

int main() {
    unsigned char buf[sizeof(S)];
    std::construct_at(reinterpret_cast<S*>(buf));
    // my_construct_at(reinterpret_cast<S*>(buf));
}

This code produces:

<source>:10:42: error: field in 'S' requires explicit initialization but is not explicitly initialized [-Werror,-Wuninitialized-explicit-init]
   10 |   return ::new (static_cast<void*>(ptr)) T(std::forward<U>(args)...);
      |                                          ^
<source>:16:5: note: in instantiation of function template specialization 'my_construct_at<S>' requested here
   16 |     my_construct_at(reinterpret_cast<S*>(buf));
      |     ^
<source>:5:19: note: 'v' declared here
    5 |     unsigned char v [[clang::require_explicit_initialization]];
      |                   ^

However, it runs fine if my_construct_at(...) is commented out.

It appears the compiler is treating std::construct_at specially somehow -- either disabling warnings on it, or rewriting it internally in some way. This should not be possible. It breaks the initialization guarantee of [[clang::require_explicit_initialization]].

[llvmbot]

@llvm/issue-subscribers-clang-frontend

Author: None (higher-performance)

``` #include <memory> #include <utility>

struct S {
unsigned char v [[clang::require_explicit_initialization]];
};

template <class T, class... U>
constexpr T* my_construct_at(T* ptr, U&&... args) {
return ::new (static_cast<void*>(ptr)) T(std::forward<U>(args)...);
}

int main() {
unsigned char buf[sizeof(S)];
std::construct_at(reinterpret_cast<S*>(buf));
// my_construct_at(reinterpret_cast<S*>(buf));
}

This code produces:

<source>:10:42: error: field in 'S' requires explicit initialization but is not explicitly initialized [-Werror,-Wuninitialized-explicit-init]
10 | return ::new (static_cast<void*>(ptr)) T(std::forward<U>(args)...);
| ^
<source>:16:5: note: in instantiation of function template specialization 'my_construct_at<S>' requested here
16 | my_construct_at(reinterpret_cast<S*>(buf));
| ^
<source>:5:19: note: 'v' declared here
5 | unsigned char v [[clang::require_explicit_initialization]];
| ^

However, it runs fine if `my_construct_at(...)` is commented out.

It appears the compiler is treating `std::construct_at` specially somehow -- either disabling warnings on it, or rewriting it internally in some way. This should not be possible. It breaks the initialization guarantee of `[[clang::require_explicit_initialization]]`.
</details>

[efriedma-quic]

The bug is a bad interaction with warning suppression in system headers. If you pass -Wsystem-headers, you get the warning.

[higher-performance]

Oh interesting. It seems like the warning should be attributed to user code here, not the system header.

What do you see as the proper fix here? Is it possible to bypass the suppression for this specific warning?

[efriedma-quic]

clang has a "ShowInSystemHeader" bit that can be set on diagnostics to make them appear by default in system headers, so that's a possible solution. Not sure if there's some better way to handle the situation in this particular context.

[higher-performance]

Oh I wasn't aware of that, that sounds like the way to go. I'll put up a PR for it, thanks!

[AaronBallman]

I think there's a broader problem that we need to solve regarding issuing user-actionable diagnostics from system headers, along the lines of #141908

However, specific to this attribute, this problem was known at the time of the RFC for the attribute (https://discourse.llvm.org/t/rfc-add-clang-attribute-to-ensure-that-fields-are-initialized-explicitly/80626):

I expect such a feature would be very difficult for most users to utilize in practice, as uninitialized structs are very common, and it is quite intractable for users to modify third-party & system headers to annotate everything fully.

And we agreed in the comments that system headers are a no-go for right now (https://discourse.llvm.org/t/rfc-add-clang-attribute-to-ensure-that-fields-are-initialized-explicitly/80626/4):

I agree that this is probably a no-go in practice. Patterns like: are way too common to ever allow using this broadly without rewriting all the code.

There was a claim that there's an approach we could try:

However, if the need for this nevertheless arises, I believe it can be achieved coherently with the current design, in a similar manner as to opt-out attributes for a class-level attribute.

But I that requires every third-party library to be Clang-aware to use those attributes, and older third-party libraries which predate the attribute have limited solutions. I don't think that's a viable approach.

I do not think we should enable "show in system headers" for this diagnostic because the first time a diagnostic is issued in a system header where the user has no ability to fix the warning aside from removing the attribute entirely will result in either the diagnostic being disabled entirely or the attribute being removed, which defeats the purpose to supporting the attribute in the first place. And we will assuredly have those kinds of false positives because "system header" is far more broad than just C runtime library or STL; it's common for folks to consider any third-party library as a system header specifically so that unactionable diagnostics are disabled in them.

We could decide that perhaps this particular warning should be an error instead; then it participates in SFINAE, you always get the diagnostic, etc. While I think some libraries like the STL will be written in a way to handle that with SFINAE fallbacks, I don't think that will happen in general. But also, we will need to audit all our builtins because stuff like __is_trivially_constructible, __is_aggregate, __builtin_is_cpp_trivially_relocatable (?maybe?), etc don't pay attention to the attribute currently and we'd need that to be consistent because type traits are built on top of those builtins and are used to ensure they do the proper kinds of initialization in generic libraries.