[PATCH] D80286: [Analyzer] Allow creation of stack frame for functions without definition

[Artem Dergachev via Phabricator via cfe-commits]

NoQ accepted this revision.
NoQ added a comment.
This revision is now accepted and ready to land.

This looks straightforward, thanks!



================
Comment at: clang/test/Analysis/explain-svals.cpp:97
 void test_6() {
-  clang_analyzer_explain(conjure_S()); // expected-warning-re{{{{^lazily frozen compound value of temporary object constructed at statement 'conjure_S\(\)'$}}}}
+  clang_analyzer_explain(conjure_S()); // expected-warning-re{{{{^lazily frozen compound value of parameter 0 of function 'clang_analyzer_explain\(\)'$}}}}
   clang_analyzer_explain(conjure_S().z); // expected-warning-re{{{{^value derived from \(symbol of type 'int' conjured at statement 'conjure_S\(\)'\) for field 'z' of temporary object constructed at statement 'conjure_S\(\)'$}}}}
----------------
Wonderful!

I think "parameter 1" would make a bit more sense. Or, even better, "1st parameter" like in regular warnings.

(this probably needs to be fixed in an older patch)


================
Comment at: clang/test/Analysis/temporaries.cpp:893
     glob = 1;
-    // FIXME: Why is destructor not inlined in C++17
     clang_analyzer_checkInlined(true);
 #ifdef TEMPORARY_DTORS
----------------
martong wrote:
> baloghadamsoftware wrote:
> > I wonder whether `clang_analyzer_checkInlined()` works correctly with this patch: it seems it only checks for stack frame which now any function with definition can have.
> So, why not try that in a test with a function that does not have a definition?
> it seems it only checks for stack frame which now any function with definition can have

Before that, it checks that `clang_analyzer_checkInlined()` was evaluated during analysis. You can't evaluate this line if you didn't analyze `~C()` either as top-level function or through inlining.


================
Comment at: clang/unittests/StaticAnalyzer/ParamRegionTest.cpp:87
                   std::make_unique<ParamRegionTestAction>(),
+                  "void foo(int n); "
+                  "void bar(int l); "
----------------
martong wrote:
> I think a raw string literal with clang-formatted code in it would make the test more valuable.
I also think it's going to be valuable to test what's going to happen if the forward-declaration is //after// the definition; that's where we were previously confused.


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

https://reviews.llvm.org/D80286