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
_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
