Szelethus added a comment.
In D142354#4078450 <https://reviews.llvm.org/D142354#4078450>, @NoQ wrote:
> Interesting, what specific goals do you have here? Are you planning to find
> specific bugs (eg. force-unwrap to a wrong type) or just to model the
> semantics?
Hi!
Meant to write this comment yesterday, but here we go. My idea was aiming for
both of the goals you mentioned:
1. Emit diagnostics on improper usages of `std::variant`, which mostly boils
down retrieving a field through a wrong type, yes. This will be, in my view,
the main showpiece of the thesis.
2. Model the semantics.
In this or in a followup patch I think we should demonstrate with a few tests
what we expect the checker to be capable of.
> In the latter case, have you explored the possibility of force-inlining the
> class instead, like I suggested in the thread?
I have done some tests then, and figured that the analyzer can't really follow
what happens in std::variant. I admit, now that I've taken a more thorough
look, I was wrong! Here are some examples.
std::variant<int, std::string> v = 0;
int i = std::get<int>(v);
clang_analyzer_eval(i == 0); // expected-warning{{TRUE}}
10 / i; // FIXME: This should warn for division by zero!
std::variant<int, std::string> v = 0;
std::string *sptr = std::get_if<std::string>(&v);
clang_analyzer_eval(sptr == nullptr); // expected-warning{{TRUE}}
*sptr = "Alma!"; // FIXME: Should warn, sptr is a null pointer!
void g(std::variant<int, std::string> v) {
if (!std::get_if<std::string>(&v))
return;
int *iptr = std::get_if<int>(&v);
clang_analyzer_eval(iptr == nullptr); // expected-warning{{TRUE}}
*iptr = 5; // FIXME: Should warn, iptr is a null pointer!
}
In neither of these cases was a warning emitted, but that was a result of bug
report suppression, because the exploded graph indicates that the errors were
found.
We will need to teach these suppression strategies in these cases, the
heuristic is too conservative, and I wouldn't mind some `NoteTag`s to tell the
user something along the lines of "Assuming this variant holds and std::string
value".
> Have you found a reasonable amount of code that uses `std::variant`, to test
> your checker on?
Acid <https://github.com/EQMG/Acid> has a few, csv-parser
<https://github.com/ashaduri/csv-parser> in one place, there is a couple in
config-loader <https://github.com/netcan/config-loader> and cista
<https://github.com/felixguendling/cista>, and a surprising amount in userver
<https://github.com/userver-framework/userver>. Though its a question how
painful it is to set up their dependencies.
CHANGES SINCE LAST ACTION
https://reviews.llvm.org/D142354/new/
https://reviews.llvm.org/D142354
_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
