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

[Artem Dergachev via Phabricator via cfe-commits]

NoQ added inline comments.
Herald added a subscriber: gamesh411.


================
Comment at: www/analyzer/checker_dev_manual.html:678
 
+<h2 id=links>Making Your Check Better</h2>
+<ul>
----------------
Probably `Checker` here as well.


================
Comment at: www/analyzer/checker_dev_manual.html:681
+<li>User facing documentation is important for adoption! Make sure the checker list is updated
+    at the homepage of the analyzer. Also ensure the description is clear to
+    non-analyzer-developers in <tt>Checkers.td</tt>.</li>
----------------
I think a link to the checker list would be great here.


================
Comment at: www/analyzer/checker_dev_manual.html:738-740
+  <li>In <tt>State->getSVal(Region)</tt>, <tt>Region</tt> is not necessarily a <tt>TypedValueRegion</tt>
+      and the optional type argument is not specified. It is likely that the checker
+      may accidentally try to dereference a void pointer.</li>
----------------
I think [[ https://reviews.llvm.org/D52984#inline-479362 | this problem ]] isn't fully fixed. Eg., "In `State->getSVal(Region)`, if `Region` is not known to be a `TypedValueRegion` and the optional type argument is not specified, the checker may accidentally try to dereference a void pointer", and i think most other bullets here should be re-phrased a bit.


================
Comment at: www/analyzer/checker_dev_manual.html:772
+    <tt>generateNonFatalErrorNode</tt> (or sequence of such if the split is intended) is
+    immediately followed by return from the checker callback</li>
+  <li>Multiple implementations of <tt>evalCall</tt> in different checkers should not conflict.</li>
----------------
A `.` is missing here.


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

https://reviews.llvm.org/D52984