[PATCH] D73521: [analyzer][WIP] add-new-checker.py: Introduction

Mon Mar 9 12:58:35 PDT 2020

NoQ added a comment.
Herald added subscribers: martong, steakhal.

I didn't use it yet but i like it. The code is easy to understand, so i'll be happy to maintain it as it lands.

The example checker is nice and i like how you made sure it demonstrates modern API usage. I'm a bit worried that it may quickly get out of date as we update our APIs, because this happens like ~multiple times a year, much more often than in clang-tidy. Is there a way to write a test that tests that the generated checker //compiles//? I don't have any specific ideas but it would have been awesome.

================
Comment at: clang/utils/analyzer/add-new-checker.py:79-81
+        data = subprocess.check_output(['llvm-tblgen', '-dump-json',
+                                        checkers_path,
+                                        '-I=' + checkers_include_path])
----------------
`llvm-tblgen` needs to be in the `PATH`, right? What if LLVM isn't installed on the host system or has the wrong version?

We might have to either tell the user to specify it explicitly in the invocation (and then update the LIT substitution to explicitly use the right binary), or, ideally, try to get the script installed into the build directory so that it could find the freshly built `llvm-tblgen` binary relative to itself.

================
Comment at: clang/utils/analyzer/add-new-checker.py:573
+
+REGISTER_MAP_WITH_PROGRAMSTATE(AwesomeMap, const MemRegion *, bool)
+
----------------
I think we should say explicitly that these are examples, not required code, because a lot of this may look like magic to a beginner.

I.e., "Here's how you define a state trait", etc.

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

https://reviews.llvm.org/D73521