dcoughlin accepted this revision.
dcoughlin added a comment.
This revision is now accepted and ready to land.
Yay! This is going to fix some really confusing false positives.LGTM with
Gabor's comments addressed.
One note. I am not a big fan of using clang_analyzer_explain() in tests for
functionality. It is great for debugging, but for testing it exposes too much
of the implementation details of the region hierarchy and store. Exposing this
kind of state is fine in tests specific to the store/region store, but in my
opinion it should be another clang_analyzer_ entry point and should be limited
to regions.
Similarly we should have a clang_analyzer_ entry point for testing symbols but
that doesn't expose the guts of MemRegions. And (maybe?) another one for
SymExprs.
================
Comment at: test/Analysis/explain-svals.cpp:103
+ C *c = 0;
+ // FIXME: we need to be explaining '40' rather than '0' here; not explainer
bug.
+ clang_analyzer_explain(&c->y); // expected-warning-re{{{{^concrete memory
address '0'$}}}}
----------------
It would be good to mention that this is a modeling bug and point to where it
could be fixed. Otherwise the FIXME is not super helpful to future maintainers.
https://reviews.llvm.org/D31982
_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
