aaronpuchert added a comment. In D153131#4653564 <https://reviews.llvm.org/D153131#4653564>, @courbet wrote:
> We have a large number of users of `-Werror -Wthread-safety-analysis` > internally. When we make the new warnings part of that flag we cannot > integrate because we're breaking all these users. The proposal was to include it in `-Wthread-safety-reference`, not `-Wthread-safety-analysis`. See https://clang.llvm.org/docs/DiagnosticsReference.html#wthread-safety for the existing flags and their relations. > If we don't integrate we can't run the new analysis to see what we would need > to fix. Can you not add `-Wno-error=thread-safety-reference-return` together with the integration? Or are there too many places adding it independently? > Introducing a new flag allows us to: > > - keep the current analysis running for users of `-Wthread-safety-analysis`. > - progressively add `-Wthread-safety-analysis-reference-return` to these > users across the codebase, fixing them or disabling analysis as needed. That is true, but these advantages seem to apply to a small number of users only (those aware of the new flag). If you integrate Clang trunk, it would be Ok if you leave it off by default for a couple of weeks, but turn it on before the next release. I'm not generally against new flags, but this is more of a "gap closing" than a new feature, so an on-by-default (under `-Wthread-safety-reference`, not `-Wthread-safety-analysis`) warning should be the right choice. Changes that result in new warnings are not uncommon, and often we don't create a new flag for them at all. Here it's Ok due to the large number of warnings, but it fits too well into `-Wthread-safety-reference` to not be triggered by that. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D153131/new/ https://reviews.llvm.org/D153131 _______________________________________________ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
