[PATCH] D58065: [analyzer] Document the frontend library

Fri Feb 15 16:53:18 PST 2019

NoQ added a comment.

Had a look. Great stuff, just as planned :) Old fanboy wisdom: Try to avoid documenting bugs you want to fix! But i don't have many high-level comments here - only appreciation of the effort.

In D58065#1394864 <https://reviews.llvm.org/D58065#1394864>, @Szelethus wrote:

> Please, in the first round of reviews, if you could make high level comments about what should or should not be here would be great, perfectly wrapping around the 80 column limit and finding typos don't concern me as much before the actual content is ready.

This should actually be sticked on top of every phabricator page (:

================
Comment at: docs/analyzer/developer-docs/FrontendLibrary.rst:11-13
+This document will describe the frontend of the Static Analyzer, basically
+everything from compiling the analyzer from source, through it's invocation up
+to the beginning of the analysis. It will touch on topics such as
----------------
First of all, "frontend" is, as far as i understand, a weird word to use with respect to this library in general. I think what they were trying to say was something like "The Static Analyzer-specific part of the C Front End's command-line flags" (as opposed to Driver flags), but calling this UI "The Frontend" is a bit weird. We probablyshould try to somehow avoid confusion with the "compiler frontend" concept throughout this document.

================
Comment at: docs/analyzer/developer-docs/FrontendLibrary.rst:15-16
+
+* How the analyzer is compiled, how tools such as TableGen are used to generate
+  some of the code,
+* How to invoke the analyzer,
----------------
Eg., we're talking about TableGen because in our case it's all about providing values for frontend flags (Checkers.td provides values for the -analyzer-checker flag, etc.). It's not because the process of compiling the Analyzer is an essential part of the very idea of the "Frontend". So it sounds a bit strange. Across the whole Clang there are many other uses of TableGen.

================
Comment at: docs/analyzer/developer-docs/FrontendLibrary.rst:57-88
+Following this, the compilation goes on as usual. The fastest way of obtaining
+the analyzer for development is by configuring CMake with the following options:
+
+* Use the `Ninja` build system
+* Build in `Release` with asserts enabled (Only recommended for slower
+  computers!)
+* Build shared libraries
----------------
For the above reason i think this text deserves a better document to be put into; this is definitely important to know for a much wider audience than developers of libStaticAnalyzerFrontend.

================
Comment at: docs/analyzer/developer-docs/FrontendLibrary.rst:216-220
+As the analyzer matured over the years, specific terms that described one
+specific function can now mean a variety of different things. For example, in
+the early 2010s, we used the term "checks" (similarly to clang-tidy) instead of
+"checkers", and there still are some remnants of this in class/object names and
+documentation. Among the most commonly misused words is "registration".
----------------
I think originally the whole Static Analyzer was referred to as "The Checker" - and was, essentially, just one checker :)

================
Comment at: docs/analyzer/developer-docs/FrontendLibrary.rst:638-643
+Model injector
+--------------
+
+Work in progress
+
+.. TODO
----------------
I really need to refresh my knowledge on this one. Why is it not on by default? Why did we need ASTImporter for cross-translation-unit analysis when we already had this? Why do we need this when we already have ASTImporter? Etc.

CTU is, besides everything else, a great alternative to body farming. We can write down models as normal code and load them via CTU and in fact it'll help us avoid ASTImporter imperfections by simply only writing models that ASTImporter is able to understand.

Repository:
  rC Clang

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

https://reviews.llvm.org/D58065