[PATCH] D122983: [C11/C2x] Change the behavior of the implicit function declaration warning

[Erich Keane via Phabricator via cfe-commits]

erichkeane added a comment.

In D122983#3426661 <https://reviews.llvm.org/D122983#3426661>, @paulwalker-arm wrote:

> Thanks for this.  I can see about cleaning up the tests but I'm still not sure what the advantage is.  The affected RUN lines are already `-fsyntax-only` tests.  Is it about where the test files live or is there something else I should be considering?  The benefit of the current tests is that it's easy to spot holes as a single function tests both requirements.  My fear is that separating the tests based on Sema/CodeGen could mean we'll miss something and never know.

There are two big problems with the way these are split up.  First, the 'list' of these functions that you expect to not be declared under a flag don't exist all in 1 place, and end up being difficult to follow/track down as they are spread out as much as they are.  Second, they are in tests that have a "REQUIRES" clause in it, which causes them to not be caught in many configurations.  We typically avoid doing -verify in CodeGen (unless the diagnostic is ACTUALLY in CodeGen) as a matter of business.  The diagnostic you're using to validate these is likely a poor choice (as it is a non-standard extension based warning) though, which is what makes this so difficult on folks trying to modernize Clang to follow standards.  IMO, if these had been C++ tests (which would have 'not found' failures), or validated the existence of these in some other mechanism, we likely would have never noticed other than via an audit.


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

https://reviews.llvm.org/D122983