Szelethus accepted this revision.
Szelethus added a comment.
This revision is now accepted and ready to land.
LGTM! I think code readability improved geatly.
> This creates a certain problem in `RetainCountChecker` (surprise!!~)
We constantly bully this checker, but still not enough :^)
================
Comment at:
clang/include/clang/StaticAnalyzer/Core/BugReporter/PathDiagnostic.h:284
+ getValidSourceLocation(const Stmt *S, LocationOrAnalysisDeclContext LAC,
+ bool UseEnd = false);
+
----------------
Lets explain what `UseEnd` is: `Whether to use getEndLoc() rather then
getBeginLoc() for \p S`.
================
Comment at:
clang/include/clang/StaticAnalyzer/Core/PathSensitive/ExplodedGraph.h:276
+
+ /// Find the next statement that is going to be executed after this node.
+ /// Useful for explaining control flow that follows the current node.
----------------
Is this correct? Shouldn't we instead say that "Find the next statement that
was symbolically executed on this path of execution"?
================
Comment at:
clang/include/clang/StaticAnalyzer/Core/PathSensitive/ExplodedGraph.h:282
+
+ /// Find the statement that was executed immediately before that node.
+ /// Useful when the node corresponds to a CFG block entrance.
----------------
before this node?
================
Comment at:
clang/include/clang/StaticAnalyzer/Core/PathSensitive/ExplodedGraph.h:288
+
+ /// Find the statement that was executed at or immediately before that node.
+ /// Useful when any nearby statement will do.
----------------
before this node?
================
Comment at: clang/lib/StaticAnalyzer/Core/BugReporter.cpp:2185
} else {
- assert(ErrorNode);
- hash.AddPointer(GetCurrentOrPreviousStmt(ErrorNode));
----------------
Why delete the assert?
================
Comment at: clang/lib/StaticAnalyzer/Core/ExplodedGraph.cpp:302-303
// Find the node's current statement in the CFG.
- if (const Stmt *S = PathDiagnosticLocation::getStmt(this))
+ // FIXME: getStmtForDiagnostics() does nasty things in order to provide
+ // a valid statement for body farms, do we need this behavior here?
+ if (const Stmt *S = getStmtForDiagnostics())
----------------
But still fails...
================
Comment at: clang/lib/StaticAnalyzer/Core/PathDiagnostic.cpp:565-566
// FIXME: Ironically, this assert actually fails in some cases.
//assert(L.isValid());
return L;
----------------
I guess didn't change much :^)
Repository:
rC Clang
CHANGES SINCE LAST ACTION
https://reviews.llvm.org/D67382/new/
https://reviews.llvm.org/D67382
_______________________________________________
cfe-commits mailing list
[email protected]
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
