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

[Umann Kristóf via Phabricator via cfe-commits]

Szelethus added a comment.

We probably should also add an entry about some code conventions we use here, for example, the use of `auto` was debated recently when used with `SVal::getAs`, maybe something like this:

- As an LLVM subproject, the code in the Static Analyzer should too follow the https://llvm.org/docs/CodingStandards.html, but we do have some further restrictions/additions to that:
  - Prefer the usage of `auto` when using `SVal::getAs`, and `const auto *` when using `MemRegion::getAs`. The coding standard states that "//Use auto if and only if it makes the code more readable or easier to maintain.//", and even though `SVal::getAs<T>` returns with `llvm::Optional<T>`, which is not completely trivial, `SVal` is one of the most heavily used types in the Static Analyzer, and this convention makes the code significantly more readable.
  - Use `llvm::SmallString` and `llvm::raw_svector_ostream` to construct report messages. The coding standard explicitly **doesn't** forbid the use of the `<sstream>` library, but we do.
  - Do not forward declare or define static functions inside an anonymous namespace in checker files.
  - Document checkers on top of the file, rather then with comments before the checker class.
  - Checker files usually contain the checker class, the registration functions, and of course the actual implementation (usually several more functions and data structures) of the checker logic. To make the code more readable, keep the checker class on top of the file, forward declare all data structures and implementation functions below that, and put the registration function on the very bottom. The actual logic should be in between. For example:

  //===----- MyChecker.cpp -----------------------------------------*- C++ -*-==//
  //
  //                     The LLVM Compiler Infrastructure
  //
  // This file is distributed under the University of Illinois Open Source
  // License. See LICENSE.TXT for details.
  //
  //===----------------------------------------------------------------------===//
  //
  // MyChecker is a tool to show an example how a checker file should be
  // organized.
  //
  //===----------------------------------------------------------------------===//
  
  #include "ClangSACheckers.h"
  #include "clang/StaticAnalyzer/Core/BugReporter/BugType.h"
  #include "clang/StaticAnalyzer/Core/Checker.h"
  #include "clang/StaticAnalyzer/Core/PathSensitive/CheckerContext.h"
  
  namespace {
  
  // Any documentation you'd like to put here should instead be put on top of the
  // file, where the checker is described.
  class MyChecker : public Checker<check::EndFunction> {
  public:
    MyChecker() = default;
    void checkEndFunction(const ReturnStmt *RS, CheckerContext &C) const;
  };
  
  /// Helper class that is only here for this example.
  class Helper {
  public:
    /// Helps.
    void help();
  };
  
  } // end of anonymous namespace
  
  /// Determines whether the problem is solvable.
  static bool isProblemSolvable();
  
  //===----------------------------------------------------------------------===//
  // Methods for MyChecker.
  //===----------------------------------------------------------------------===//
  
  void MyChecker::checkEndFunction(const ReturnStmt *RS,
                                   CheckerContext &C) const {
    // TODO: Implement the logic.
  }
  
  //===----------------------------------------------------------------------===//
  // Methods for Helper.
  //===----------------------------------------------------------------------===//
  
  void Helper::help() {
    // TODO: Actually help.
  }
  
  //===----------------------------------------------------------------------===//
  // Utility functions.
  //===----------------------------------------------------------------------===//
  
  static bool isProblemSolvable() {
    return false;
  }
  
  void ento::registerMyChecker(CheckerManager &Mgr) {
    auto Chk = Mgr.registerChecker<MyChecker>();
  }


https://reviews.llvm.org/D52984