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