[clang] Include [[clang::require_explicit_initialization]] warnings in system headers (PR #141133)
via cfe-commits
cfe-commits at lists.llvm.org
Wed Jun 25 04:52:15 PDT 2025
higher-performance wrote:
Thanks!
One thing I want to also highlight here is that the general problem of figuring out "whom to blame" (i.e. where the diagnostic should be attributed to, and thus when to suppress the diagnostic) for an incorrect template instantiation seems difficult at best, and fundamentally impossible (without additional out-of-band information) at worst.
For example, consider this:
```
template<class T>
struct S;
template<class T>
T foo() { return typename S<typename T::value_type>::type(); }
```
If you see a call to `foo<std::string>()`, and it turns out that instantiation is wrong, _whom exactly_ should take the blame/responsibility here? Any of these are possible:
- The definition of `foo` would be to blame _if_ its author has control over all the specializations of `S`.
- The definition of `S` would be to blame _if_ its author is guaranteeing that `::type` is instantiable.
- The `foo<std::string>()` expression would be to blame _if_ `S` and `foo` are entirely orthogonal, and thus the author of `foo<std::string>()` is the one imposing instantiability as a requirement.
There is simply no way to know which is the correct choice -- and thus no way to know where the suppressions should be processed -- without knowing the implicit assumptions in the code. *Any* choice made is going to be wrong some of the time.
This is why I'm so weary of trying to solve this larger problem for the sake of this PR here -- it's not even clear to me that it's solvable in the first place, never mind the implementation challenges.
_________
Now, responding to @erichkeane's thoughts above:
1- `-W[no-]system-headers=...` seems like a reasonable way to address this -- do you see issues with it?
2- (see highlight above)
3- Default-construct + memcpy still requires the default-construction to be valid. It doesn't really matter whether it's occurring in the STL or not, right? (Just like how `std::make_unique<T>()` requires default-construction of `T` to be valid, even if it's in the STL.) If the STL is instantiating `T` then it has to know that `T` is valid to instantiate that way. For a user type, it wouldn't possibly know that, so it wouldn't do that except on behalf of the user. For its own types, then of course it has to respect its own attributes, but then there's no problem. If you have an example that neither of these addresses, please share them. The _only_ one I can think of is `std::bit_cast`, but that seems fine to ignore.
4- From my _biased_ standpoint, I'm not inherently against error-by-default (we are currently using it under `-Werror` anyway), **but** I think it is (a) strictly worse than allowing it to be either a warning or an error? (Note that I'm assuming `-Wno-system-headers=...` as a further extension here, if we end up needing it.)
5- I very much agree. To me the worst case (if they come up at all) is that some people would stop using the attribute, which is... where we were before the attribute was introduced, and where other compilers are now. At least in that case they can resort to alternate (more inconvenient) methods for discovering all construction sites. Contrast that to the current situation, where unaware users mistakenly _think_ the attribute is working when it is actually suppressed in some cases -- and thus may assume they have updated all call sites, when they actually haven't, which is _worse_ than not having it at all.
(Important side note: _we_ aren't running into this problem, because we have implemented a safety mechanism: we're hiding the attribute behind a macro that _also_ adds an in-class initializer, so we at least get a linker error if the attribute diagnosis fails for some reason. The error message is terrible, but at least we get _some_ error, and thus the attribute is still very valuable to us, even with its current flaw. But a user who is unaware of this failure mode wouldn't do that, and our solution isn't possible in C either.)
https://github.com/llvm/llvm-project/pull/141133
More information about the cfe-commits
mailing list