[PATCH] D45685: [Sema] Add -wtest global flag that silences -Wself-assign for overloaded operators.

[Richard Smith - zygoloid via Phabricator via cfe-commits]

rsmith added a comment.

I would feel a lot more comfortable adding a `-wtest` flag if we had more than one warning that it would control. If there's really only one warning out of >600 that qualifies for this treatment, I really don't think we have a good argument to introduce a new feature here, and it'll just be dead weight we're carrying forever. My inclination is to say that the single `-Wno-` flag is sufficient until we actually have multiple warnings that this would control, and this is a premature generalization until that point. ("We're just about to add these warnings" carries a lot more weight for me here than "We have ideas of warnings we might add", but if we're in the former situation, there seems to be no harm in waiting.)

I'm also concerned about the deployability and utility of this feature. Identifying "test code" is not going to be easy in a lot of build systems. Even if successfully deployed, this would mean that refactorings moving code between files (eg, out of test code into shared infrastructure code) could affect whether clang accepts the program (eg, under `-Werror`), which seems pretty undesirable. And likewise, tests that check that (for instance) certain macro expansions produce valid code would stop being reliable -- the expansion might be valid in test code but not valid in non-test code. But for me that's a minor concern at worst: if there are users who are happy with the tradeoffs here, I'd be OK with us carrying support for them. The major concern here is: are there users who would enable this feature? (Briefly taking off my Clang hat and putting on my Google hat, I think we -- as the people who originally had major problems with the expanded `-Wself-assign` warning -- would be very unlikely to ever use `-wtest` because of the refactoring issue.)

Finally, let's assume that this is successful and we end up with dozens of warnings covered by `-wtest`. How confident are we that we're not going to want a case-by-case way to turn them back on? That is, how sure are we that this isn't just another warning group (albeit one that only really makes sense to turn off, not to turn on)? For `-w`, this isn't really a concern, because `-w` is very much a "just compile it, I do not care about bugs or code quality" flag which doesn't really make sense to only partially deploy.



================
Comment at: include/clang/Basic/Diagnostic.td:102-104
+class ShowInTests {
+  bit HideInTests = 0;
+}
----------------
This does not seem like it should ever be necessary.


Repository:
  rC Clang

https://reviews.llvm.org/D45685