[PATCH] D39518: [analyzer] do not crash on libcxx03 call_once implementation

[Devin Coughlin via Phabricator via cfe-commits]

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