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

Just add tests, i guess!

Also i'll have a look at whether the checker is in a good shape to be enabled 
by default. I suspect that mismatched iterators might be very safe to enable. 
With all these solver and rearrangement issues, if they only cause false 
negatives, it's not blocking enabling the checker by default. The same applies 
to the debate around `find`.

One thing that's most likely necessary to do before enabling the checker by 
default is to add a bug report visitor, so that it added notes when iterators 
appear for the first time or change their state. Without such notes it's 
usually very hard to understand warnings. Especially because we're dropping the 
path within inlined functions that have no interesting events, but when the 
iterator originates from or gets updated within such function, this information 
becomes crucial. The visitor might have to hop from one object to another 
similarly to `trackNullOrUndefValue()` (i.e., by adding more instances of 
itself that track objects that are being copied or moved into the object of 
interest at the program point that is currently being visited).



================
Comment at: test/Analysis/mismatched-iterator.cpp:157
+void bad_empty(std::vector<int> &v1, std::vector<int> &v2) {
+  is_cend(v1, v2.cbegin()); // expected-warning@149{{Iterators of different 
containers used where the same container is expected}}
+}
----------------
Maybe use `@-8` instead, so that we only had to update it when *this* test 
changes?


https://reviews.llvm.org/D32906



_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

Reply via email to