[PATCH] D52390: [analyzer] Add StackSizeChecker to StaticAnalyzer

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

Szelethus added inline comments.


================
Comment at: include/clang/StaticAnalyzer/Checkers/StackUsageMeasuringVisitor.h:102-111
+  bool hasEmptyFlaggedUsageStored(const Stmt *S) const {
+    auto iter = StmtSubtreeUsages.find(S);
+    return iter != StmtSubtreeUsages.end() &&
+           iter->second == Usage::EmptyFlagged;
+  }
+  bool hasEmptyFlaggedUsageStored(const Decl *D) const {
+    auto iter = DeclSubtreeUsages.find(D);
----------------
whisperity wrote:
> Szelethus wrote:
> > Would `isEmptyFlagged` be a better method name?
> Then calling this method would mean answering this question: "A Stmt/Decl is empty flagged?" This is a "What?" moment.
Right. And it doesn't actually describe what the function does. `hasEmptyFlaggedSubDecl`?


================
Comment at: lib/StaticAnalyzer/Checkers/StackSizeChecker.cpp:25
+#include "clang/StaticAnalyzer/Core/PathSensitive/CheckerContext.h"
+#include <sstream>
+
----------------
whisperity wrote:
> Szelethus wrote:
> > Don't include standard stream libraries: https://llvm.org/docs/CodingStandards.html#include-iostream-is-forbidden
> > Note that using the other stream headers (`<sstream>` for example) is not problematic in this regard 
Oh, right, sorry about that. In either case, every other checker uses LLVM streams, so I guess `llvm::raw_*_ostream` are the preferred stream classes anyways.


================
Comment at: lib/StaticAnalyzer/Checkers/StackSizeChecker.cpp:41
+    : public Checker<check::PreCall, check::PostCall, check::EndFunction> {
+  mutable std::unique_ptr<BugType> StackOverflowBugType;
+
----------------
whisperity wrote:
> Szelethus wrote:
> > I think you don't need to make this `mutable` anymore, you can just initialize it in the constructor.
> `generateError` is const-qualified but writes this.
But I'm not sure that it should. At least, when I wrote my checker, I didn't have to: https://github.com/llvm-mirror/clang/blob/master/lib/StaticAnalyzer/Checkers/UninitializedObject/UninitializedObjectChecker.cpp#L166


Repository:
  rC Clang

https://reviews.llvm.org/D52390