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

[Whisperity via Phabricator via cfe-commits]

whisperity added inline comments.



Comment at: docs/analyzer/developer-docs/FrontendLibrary.rst:85
+-fuse-ld=lld \
+../llvm
+

george.karpenkov wrote:
> Information on compiling LLVM IMO should not be here.
> Also, why Sphinx? Why X86? Why LLD and not gold?
> Information on compiling LLVM IMO should not be here.
> Also, why Sphinx? Why X86? Why LLD and not gold?

Analyser for development -- however, you build `release` instead of (at least) 
`RelWithDebInfo`.

`EXPORT_COMPILE_COMMANDS` is also totally unnecessary for an "analyser for 
development".

Perhaps individual recommended settings could be linked back to the 
configuration guide's sections? (Unfortunately that guide can't link to 
individual //options// sadly.)



Comment at: docs/analyzer/developer-docs/FrontendLibrary.rst:94
+Other documents detail the difference between the *driver* and the *frontend*
+of clang far more precisely, but we'll touch on this briefly: When you input
+``clang`` into the command line, you invoke the driver. This compiler driver

When we talk about the project, use **C**lang. When talking about the binary, 
`clang` in monospace is good style.



Comment at: docs/analyzer/developer-docs/FrontendLibrary.rst:140
+
+Although we don't support running the analyzer without enabling the entire core
+package, it is possible, but might lead to crashes and incorrect reports.

`core` for emphasis.


Repository:
  rC Clang

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

https://reviews.llvm.org/D58065



___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

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

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

Szelethus added a reviewer: aaron.ballman.
Szelethus added a comment.

Oops.


Repository:
  rC Clang

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

https://reviews.llvm.org/D58065



___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

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

[Mandeep Singh Grang via Phabricator via cfe-commits]

mgrang added inline comments.



Comment at: docs/analyzer/developer-docs/FrontendLibrary.rst:12
+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

typo: it's --> its



Comment at: docs/analyzer/developer-docs/FrontendLibrary.rst:42
+
+The following section is sort of a summary, and severeral items will be later
+revisited in greater detail.

typo: severeral --> several


Repository:
  rC Clang

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

https://reviews.llvm.org/D58065



___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

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

[George Karpenkov via Phabricator via cfe-commits]

george.karpenkov added a comment.

High-level feedback: mixing of abstraction levels is wrong for the "bundled" 
documentation. This might also work better as a blogpost, if you want to jump 
from topic to topic.




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

NoQ wrote:
> 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.
+1, not sure what the word "frontend" adds here.
IMO "frontend" in folder/library name is more of a relic in this case.



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

NoQ wrote:
> 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.
Strictly speaking for tests it's better to use `check-clang-analyzer`.
Also again strictly speaking it's not possible to compile analyzer without 
compiling `clang`.



Comment at: docs/analyzer/developer-docs/FrontendLibrary.rst:85
+-fuse-ld=lld \
+../llvm
+

Information on compiling LLVM IMO should not be here.
Also, why Sphinx? Why X86? Why LLD and not gold?


Repository:
  rC Clang

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

https://reviews.llvm.org/D58065



___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

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

[Artem Dergachev via Phabricator via cfe-commits]

NoQ added a comment.

In D58065#1400615 , @Szelethus wrote:

> I've also wasted my second weekend trying to make Static Analyzer unit tests 
> run under check-clang-analysis


:) I tried to have a quick look but got confused pretty quickly.

In D58065#1400615 , @Szelethus wrote:

> In D58065#1400154 , @NoQ wrote:
>
> > Old fanboy wisdom: Try to avoid documenting bugs you want to fix!
>
>
> Oh, which one do you mean?


Nothing in particular yet, just my own mistakes :p


Repository:
  rC Clang

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

https://reviews.llvm.org/D58065



___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

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

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

Szelethus added a comment.

Thanks for the review on my patches! I'm a little busy atm (I've also wasted my 
second weekend trying to make Static Analyzer unit tests run under 
check-clang-analysis), but I'll see to fixing these in the coming weeks.

In D58065#1400154 , @NoQ wrote:

> Old fanboy wisdom: Try to avoid documenting bugs you want to fix!


Oh, which one do you mean?


Repository:
  rC Clang

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

https://reviews.llvm.org/D58065



___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

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

[Artem Dergachev via Phabricator via cfe-commits]

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 , @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



___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

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

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

Szelethus added a comment.
Herald added a subscriber: jdoerfert.

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.


Repository:
  rC Clang

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

https://reviews.llvm.org/D58065



___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

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

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

Szelethus created this revision.
Szelethus added reviewers: NoQ, george.karpenkov, xazax.hun, dkrupp, rnkovacs, 
a.sidorin, aaron.ballman.
Herald added subscribers: cfe-commits, gamesh411, donat.nagy, mikhail.ramalho, 
szepet, baloghadamsoftware, whisperity.
Herald added a project: clang.

Since I've had my fair share of complaining about the lack of documentation, 
it's time I did my part, hence this patch, which provides documentation for a 
good majority of the frontend library.

Preview:
http://cc.elte.hu/clang-docs/docs/analyzer-frontend-html/analyzer/developer-docs/FrontendLibrary.html

Several more items could be added, describing how `AnalysisConsumer` interacts 
with `AnalysisManager` would be the most important of those. For now, I put the 
greatest emphasis on checker registration.

What should this be expanded with? Is this document too large, maybe I should 
split it up? I haven't documented much so far, so I'd happily take any feedback.


Repository:
  rC Clang

https://reviews.llvm.org/D58065

Files:
  docs/analyzer/developer-docs.rst
  docs/analyzer/developer-docs/FrontendLibrary.rst
  include/clang/StaticAnalyzer/Frontend/CheckerRegistry.h

Index: include/clang/StaticAnalyzer/Frontend/CheckerRegistry.h
===
--- include/clang/StaticAnalyzer/Frontend/CheckerRegistry.h
+++ include/clang/StaticAnalyzer/Frontend/CheckerRegistry.h
@@ -16,46 +16,6 @@
 #include 
 #include 
 
-// FIXME: move this information to an HTML file in docs/.
-// At the very least, a checker plugin is a dynamic library that exports
-// clang_analyzerAPIVersionString. This should be defined as follows:
-//
-//   extern "C"
-//   const char clang_analyzerAPIVersionString[] =
-// CLANG_ANALYZER_API_VERSION_STRING;
-//
-// This is used to check whether the current version of the analyzer is known to
-// be incompatible with a plugin. Plugins with incompatible version strings,
-// or without a version string at all, will not be loaded.
-//
-// To add a custom checker to the analyzer, the plugin must also define the
-// function clang_registerCheckers. For example:
-//
-//extern "C"
-//void clang_registerCheckers (CheckerRegistry ) {
-//  registry.addChecker("example.MainCallChecker",
-//"Disallows calls to functions called main");
-//}
-//
-// The first method argument is the full name of the checker, including its
-// enclosing package. By convention, the registered name of a checker is the
-// name of the associated class (the template argument).
-// The second method argument is a short human-readable description of the
-// checker.
-//
-// The clang_registerCheckers function may add any number of checkers to the
-// registry. If any checkers require additional initialization, use the three-
-// argument form of CheckerRegistry::addChecker.
-//
-// To load a checker plugin, specify the full path to the dynamic library as
-// the argument to the -load option in the cc1 frontend. You can then enable
-// your custom checker using the -analyzer-checker:
-//
-//   clang -cc1 -load  -analyze
-// -analyzer-checker=
-//
-// For a complete working example, see examples/analyzer-plugin.
-
 #ifndef CLANG_ANALYZER_API_VERSION_STRING
 // FIXME: The Clang version string is not particularly granular;
 // the analyzer infrastructure can change a lot between releases.
Index: docs/analyzer/developer-docs/FrontendLibrary.rst
===
--- /dev/null
+++ docs/analyzer/developer-docs/FrontendLibrary.rst
@@ -0,0 +1,643 @@
+
+Frontend Library (libStaticAnalyzerFrontend)
+
+
+.. contents:: Table of Contents
+   :depth: 4
+
+Introduction
+
+
+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
+
+* How the analyzer is compiled, how tools such as TableGen are used to generate
+  some of the code,
+* How to invoke the analyzer,
+* How crucial objects of the analyzer are initialized before the actual
+  analysis begins, like
+
+  * The `AnalyzerOptions` class, which entails how the command line options are
+parsed,
+  * The `CheckerManager` class, which entails how the checkers of the analyzer
+are registered and loaded into it,
+  * No list is complete without at least a third item.
+
+* How certain errors are handled with regards to backward compatibility,
+
+starting from how an entry in the TableGen gets processed during the
+compilation of the project, how this process begins runtime when the analyzer
+is invoked, up to the point where the actual analysis begins.
+
+The document will rely on the reader having a basic understanding about what
+checkers are, have invoked the analyzer at least a few times from the command
+line.