Untangle static analyzer and SSAF tests

Clang Frontend Static Analyzer
1

The clang static analyzer and SSAF (scalable static analysis framework) are independent projects, they should have separate test directories and separate “run the tests” targets.

To ensure this, I propose the following changes:

  • Move the SSAF tests from clang/test/Analysis/Scalable to e.g. clang/test/SSAF or any other name preferred by the SSAF developers.
  • Rename clang/test/Analysis to clang/test/StaticAnalyzer to reflect the fact that apart from the recently added SSAF tests it only contains tests for the static analyzer (see this older discourse thread where I verified this and removed a handful of misplaced tests).
  • Restore that check-clang-analysis only runs the static analyzer tests.
    • Alternatively, if you think that this traditional name is too general, introduce a target e.g. check-clang-static-analyzer that only runs the static analyzer tests.
  • Introduce a target that only runs the SSAF tests (if there isn’t one already existing).

See also [SSAF] Mark two failing tests as unsupported by NagyDonat · Pull Request #223976 · llvm/llvm-project · GitHub where the testing of these two projects interfere with each other.

@steakhal @Xazax-hun @NoQ @necto @ziqingluo-90 What do you think about this?

(Please also forward this topic to anybody on the SSAF team who may be interested but doesn’t follow the static analyzer discourse.)

2

I do support the separation. Purely practical benefit: if SSAF is not part of CSA, when I want to run all CSA tests (using llvm-lit.py clang/test/Analysis) I don’t want the other tests to mix in.
I also agree that “test/Analysis” is too vague and invites other static analysis tests. Additionally, it seems that CSA files are spread over “lib/Analysis”, “lib/StaticAnalyzer”, “include/clang/Analysis”, and “include/clang/StaticAnalyzer”, which is confusing.

As an alternative which I am also open to, we could keep Analysis, but group CSA tests in “test/Analysis/StaticAnalyzer” and SSAF in “test/Analysis/SSAF”. This way if you want to run the same set, you can still run lit test/Analysis.

1 Like
3

As an alternative which I am also open to, we could keep Analysis, but group CSA tests in “test/Analysis/StaticAnalyzer” and SSAF in “test/Analysis/SSAF”. This way if you want to run the same set, you can still run lit test/Analysis.

+1 With this we would hit two birds with one stone. It would reduce the complexity IMHO.

4

My personal opinion would be that yes, we should untangle these directories. For SSAF, we should stick to the Scalable name. For the static analyzer, we should also go with the name of the directory: StaticAnalyzer

I have a couple of questions though:
What else lives in the Analysis test folder? I’m fairly certain we could sort them case by cases basis when proposing the migration.

Next question is the cost. How is it going to distrupt downstream users. I suppose we should ignore blame the migration commit and handle the outlier cases before hand.
Second, in git, having downstream additional tests, would re-create this Analysis directory for them, so how are we going to make sure that they dont accidentally end up eith both Analysis and StaticAnalyzer test dirs?

So far I’ve seen consensus about:

  • we should change something because what we have today causes friction
  • we ahould have dedicated test dirs for CSA and SSAF

My next question would be, should we entirely eliminate the Analysis dir, and move the tests to more appropriate places?

5

CFG tests come to mind, such as cfg.cpp. They’re invoked via -analyzer-checker=debug.DumpCFG, and they test the CFG under the analyzer-specific CFG build options, but in theory they have nothing to do with the static analyzer, so it should be possible to untangle them.

6

As an alternative which I am also open to, we could keep Analysis, but group CSA tests in “test/Analysis/StaticAnalyzer” and SSAF in “test/Analysis/SSAF”.

I think clang/test/StaticAnalyzer is significantly better than clang/test/Analysis/StaticAnalyzer because it is analogous with clang/lib/StaticAnalyzer, clang/unittests/StaticAnalyzer and clang/include/clang/StaticAnalyzer (under include we presumably have an extra clang layer because we want that spelled out in the #include directives).

This way if you want to run the same set, you can still run lit test/Analysis.

Running the static analyzer and the SSAF tests together is not very useful, they are independent tools. Even if somebody wants to do so, they could still easily use lit test/StaticAnalyzer; lit test/Scalable.

In early 2025 (before the addition of Scalable), I confirmed/ensured that practically [1] all tests in the Analysis folder either invoked the analyzer with %clang_analyze_cc1, tested analyzer-related features (e.g. help printouts for analyzer options) or tested closely related tools (scan-build, exploded-graph-rewriter).

Based on this I was confident that all tests in the Analysis folder “belong to” the analyzer, because I didn’t think about the possibility that some tests just use the analyzer as a tool for validating something – like the CFG tests mentioned by @NoQ do.

  1. Does anybody know about any other such “uses the analyzer but tests something else” test group?
  2. Are these CFG tests (31 files) significant enough to have their own separate directory apart from the analyzer tests?

[1] Now that I ran a quick check I noticed that I forgot to remove/relocate PR9741.cpp at that time. We should re-check the directory for such stragglers, but we can be confident that there is at most one or two such exception among the many hundreds of test files.

I will discuss this with my colleagues, in particular with @gamesh411 who usually works on the integration of the upstream and downstream changes.

My first guess is that it would be easier to ensure a clean transition if the Analysis dir does not exist at all in the new directory layout. Then e.g. we could include test under the test-clang-analysis target which fails if clang/test/Analysis exists (and prints a helpful message that describes the transition).