dcoughlin added a comment. In https://reviews.llvm.org/D39518#914515, @george.karpenkov wrote:
> Updated the diff, addressed review concerns. > Made it more explicit in comments in tests that we do not crash on libcxx03 > implementation, but that we don't model it either. > > @dcoughlin sorry I disagree on previous testing notes (requiring to > explicitly test that the std::call_once on libcxx03 is ignored). > Current tests test exactly the specification we want them to test: libcxx11 > is modeled, libcxx03 does not crash. At a minimum we need to test that calling call_once() doesn't emit a diagnostic. If the analyzer were to emit a diagnostic on every call, that would be very bad. It would probably also be good to also test that the arguments are escaped as well. > Testing actual behavior on libcxx03 would entail many problems, with very > little benefit: > > a) The fact that libcxx03 std::call_once is ignored is not part of the > specification, it's just not specified > b) Splitting libcxx03 into a separate file would case many issues: You don't have to copy the whole file. Please, just add a test (somewhere, anywhere) making sure that that the analyzer does what we expect on a call to call_once() under libcxx03. This should not be difficult and it tests a supported analyzer configuration. > c) Having separate behaviors with `ifndef` would make the file larger, > increase the cognitive overhead, and also make writing future tests more > difficult. This is also something easy to forget while writing future tests. > Also it's not clear how `ifndef` would even help, since LIT directives are > not controlled by the preprocessor. You are testing with `-verify` and `// expected-warning`. Note that `-verify` does respect the preprocessor; this is a pattern used throughout the clang tests. https://reviews.llvm.org/D39518 _______________________________________________ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
