[PATCH] D133425: Silence -Wctad-maybe-unsupported stemming from system headers

[David Blaikie via Phabricator via cfe-commits]

dblaikie added a comment.

> In D133425#3778433 <https://reviews.llvm.org/D133425#3778433>, @dblaikie wrote:
>
>> So previously/currently-without-this-patch the diagnostic was suppressed if the use of ctad was in a system header (suppression based on the generic/builtin diagnostic suppression infrastructure) & now it'll suppress if that happens, or if the template is defined in a system header.
>
> One thing I don't understand in the current state of things is why the diagnostic fires at all inside system headers. I thought warnings in system headers were discarded?

It doesn't fire if the location of the diagnostic (the place that's using CTAD) is in a system header. But it does fire if that place is outside a system header, but the template that is being used is in a system header (see the SysHeaderObj example above - maybe the naming is confusing? It's a `SysHeaderClass` but the object (`sho`) is not in a system header).

> It this whole issue about system headers being added with `-I` instead of `-isystem`?

Don't think so.

> FWIW, my "objection" that we should not silence the warning when users try using CTAD with arbitrary types in `std::` remains -- I think it would be making a disservice to users to let them use CTAD with classes that have not been designed with that in mind. At the end of the day, I think I'm advocating for individual classes opting-out of the warning, while the patch as currently formulated forces all classes in system headers to be opted-out of the warning.

I think the problem is that not all system headers match the style required for this constraint - so in the interests of compatibility with the complex world of existing system headers, it's suitable not to enforce this stylistic constraint (requiring explicit deduction guides even when the default is what the template author intended to allow CTAD) on system headers. With the knowledge that this does introduce false negatives, but that we don't have a strong enough signal to avoid them.


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D133425/new/

https://reviews.llvm.org/D133425