[PATCH] D52984: [analyzer] Checker reviewer's checklist

[Henry Wong via Phabricator via cfe-commits]

MTC added a comment.

Greate idea! If we can enrich this list, it will not only help the reviewer, but also help beginners, like me, avoid some pitfalls when developing a new checker.

I agree with NoQ to classify the lists. In addition to the two categories proposed by NoQ, there is another classof issues, see below, we don't have to take them into account.

- Develop a checker(proposed by NoQ). Like:
  1. Does it have to be in the path-sensitive manner?
  2. Should we move it into `clang-tidy`?
  3. Prefer `ASTMatcher` to `StmtVisitor`?
  4. Use `CallDescription` to get a stricter filter.
  5. Could we use `checkPreCall()` instead of `checkPreStmt(const CallExpr *, )`?
  6. Could we put this function model into `StdLibraryFunctionsChecker.cpp`?
- Make a good checke(proposed by NoQ).
- More general coding standards. Like:
  1. Use `isa<>` instead of `dyn_cast<>` if we don't need the casted result.
  2. Use `dyn_cast_or_null` instead of `dyn_cast` to enable the null-check.
  3. Use `llvm::make_unique` instead of `std::make_unique` because `std::make_unique` is not included in C++11.
  4. Avoid unnecessary null-check
  5. ... and so on.

The third list can be long, and maybe we don't need to take it into account, because developers will follow this standards whether they are developing checkers or contribute to other projects, like LLDB. We need to dig deeper into the analyzer-specific coding standards.


https://reviews.llvm.org/D52984