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

Thu Sep 8 14:21:49 PDT 2022

ldionne added a comment.

In D133425#3777598 <https://reviews.llvm.org/D133425#3777598>, @aaron.ballman wrote:

> In D133425#3775353 <https://reviews.llvm.org/D133425#3775353>, @ldionne wrote:
>
>> Wouldn't re-applying https://github.com/llvm/llvm-project/commit/fcd549a7d8284a8e7c763fee3da2206acd8cdc4f (which had been reverted IIUC) be a more precise fix for this problem? We'd suppress the warning, but only for classes that we know are OK to use with CTAD.
>
> Oh that would be a great solution to this! Do you recall why that change was reverted?

I don't, but I can try to re-apply the patch and we'll know :-).

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 this whole issue about system headers being added with `-I` instead of `-isystem`?

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.

Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D133425