courbet added a comment. In D153131#4653456 <https://reviews.llvm.org/D153131#4653456>, @aaronpuchert wrote:
> In D153131#4653412 <https://reviews.llvm.org/D153131#4653412>, @courbet wrote: > >> I also had some push back internally on adding this to the existing flag. >> I'm going to add `-Wthread-safety-reference-return`, can we start by not >> temporarily including it in `-Wthread-safety-reference` so that we can see >> how much work it it to fix those warnings ? > > Can you elaborate on this? What's the reasoning? Here are two reasons for > having it as part of `-Wthread-safety-reference` right from the beginning: > > - `-Wthread-safety-reference` is already separate from > `-Wthread-safety-analysis` because passing a reference does not imply an > access. If you have the warning you're arguably already opting into this, and > I don't see much of a difference between passing via parameter versus passing > by return. > - Most users don't follow all reviews or read the release notes in detail and > won't notice the new flag until it shows up in their build log. So we'd just > lose time. > > Since warning messages always indicate the warning flag and thus make > disabling it easy, I don't see an issue with enabling it right away as part > of `-Wthread-safety-reference`. > > Lastly, this doesn't seem complicated enough to warrant extended beta testing. 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. If we don't integrate we can't run the new analysis to see what we would need to fix. 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. 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
