Szelethus added a comment.
Nice! I have no objections (other than the nits) to this! Ill take a second
look later, because I really need to play around with `CFG` a bit to give a
definitive LGTM.
================
Comment at:
clang/include/clang/StaticAnalyzer/Core/PathSensitive/ExprEngine.h:723
+ /// should always inline simply because it's small enough.
+ bool isSmall(AnalysisDeclContext *ADC) const;
+
----------------
Your explanation of state splitting, and early returns and the like could be
added here as well :)
================
Comment at: clang/lib/Analysis/CFG.cpp:4684
+ if (size() <= 3)
+ return true;
+
----------------
What are the cases for the size being 2 or 1? Empty function? Is a size of 1
even possible? Can we enforce something here with asserts?
================
Comment at: clang/lib/Analysis/CFG.cpp:4686
+
+ // Traverse the CFG until we find a branch. TODO: While this should still be
+ // very fast, maybe we should cache the answer.
----------------
Break TODO to new line.
================
Comment at: clang/lib/StaticAnalyzer/Core/ExprEngineCallAndReturn.cpp:890
// Do not inline large functions.
- if (CalleeCFG->getNumBlockIDs() > Opts.MaxInlinableSize)
+ if (CalleeCFG->getNumBlockIDs() >= Opts.MaxInlinableSize)
return false;
----------------
Nice.
================
Comment at: clang/test/Analysis/inline-if-constexpr.cpp:16
+void bar() {
+ clang_analyzer_eval(f7()); // expected-warning{{TRUE}}
+}
----------------
Aha, we wouldve seen UNKNOWN because the analyzer wouldn't inline these many
functions, right? Neat!
================
Comment at: clang/unittests/Analysis/CFGTest.cpp:117
+ expectLinear(true, "void foo() { do {} while (false); }");
+ expectLinear(true, "void foo() { foo(); }"); // Not our problem.
}
----------------
What do you mean?
Repository:
rC Clang
CHANGES SINCE LAST ACTION
https://reviews.llvm.org/D61051/new/
https://reviews.llvm.org/D61051
_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
