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