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
_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
