[PATCH] D114721: [clang][dataflow] Add unit tests for PostOrderCFGView

Gábor Horváth via Phabricator via cfe-commits cfe-commits at lists.llvm.org
Mon Nov 29 10:34:48 PST 2021


xazax.hun added a comment.

I don't have strong opinions, just some small preferences, feel free to ignore them:

- Using ASTMatchers feels a bit heavy weight for this task to me. Simply enumerating the function definitions in the `TranslationUnitDecl` should be sufficient for these tests.
- The tests might be a bit too strict, but this might be by design. In fact, there are more than one (reverse) post orders traversals of the same Cfg. Do we want to bake that particular order into the tests? This makes it harder to do changes but makes sure the tests notify us about such changes. Alternatively, we could do property-based testing, and verify that the view has the fundamental property we are interested in. Namely, if we visit nodes in the right order, we always visit all the predecessors of a node first (before visiting the node itself). Property-based tests are resilient to changes in which order we pick, and they can also make it somewhat easier to write tests (we no longer need to know about basic block IDs).



================
Comment at: clang/unittests/Analysis/Analyses/PostOrderCFGViewTest.cpp:23
+
+namespace clang {
+namespace analysis {
----------------
We could use `using namespace` here.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D114721



More information about the cfe-commits mailing list