aaron.ballman added a comment.

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

> The tests verify a set of builtins do not exist when the associated feature 
> flag is not present.  They sit within CodeGen because the tests were 
> plentiful and it did not seem worth duplicating them.

You don't need to duplicate hundreds of tests to have appropriate Sema tests 
for this functionality. You could have added a single test file to test/Sema 
that tests each of the builtins you care about the semantic diagnostic behavior 
of. Doing this from codegen on a hundreds of individual tests that don't run 
everywhrere is disruptive because it makes modifying diagnostic behavior 
considerably harder than it should be. We separate codegen and sema tests for a 
reason.

I'll leave the test coverage but add `-std=c99` to the verify lines because 
that will unblock this patch. However, I'd still appreciate if the AArch64 
maintainers would clean up these tests in a more appropriate fashion, because 
adding the -std= line also loses test coverage (especially when we switch the 
default language mode). This will need to happen sooner rather than later 
(relatively speaking; nothing is on fire requiring immediate attention) -- I 
expect we'll bump clang's default from C17 to C23 sometime in the next few 
years, at which point all of these tests will fail to compile because there 
will be no support for implicit function declarations whatsoever.


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

https://reviews.llvm.org/D122983

_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

Reply via email to