Page MenuHomePhabricator

[analyzer] Add docs for cplusplus.InnerPointer
AcceptedPublic

Authored by rnkovacs on Apr 4 2019, 12:24 PM.

Details

Summary

Tried to pick two interesting examples from the tests.
This check has no options.

Diff Detail

Event Timeline

rnkovacs created this revision.Apr 4 2019, 12:24 PM
Herald added a project: Restricted Project. · View Herald TranscriptApr 4 2019, 12:24 PM
Szelethus accepted this revision.Apr 7 2019, 11:09 AM

Woohoo!

docs/analyzer/checkers.rst
225–226

Hmm. While this page is a documentation, I would still expect regular users to browse through it -- are we sure that we want to add future plans for a non-alpha checker? I'm not against it, just a question.

This revision is now accepted and ready to land.Apr 7 2019, 11:09 AM
dkrupp added inline comments.Apr 8 2019, 12:02 AM
docs/analyzer/checkers.rst
225–226

I think it is a good idea. A non-alpha checker can also be further developed, by anyone else. It is good that we don't forget about further features. This note also highlights the limitations of the checker.

Szelethus added inline comments.Apr 10 2019, 3:00 PM
docs/analyzer/checkers.rst
225–226

How about this: "Future plans include to add support for blahblah". The current statement should rather be a TODO in the code.

NoQ added inline comments.Apr 12 2019, 6:20 PM
docs/analyzer/checkers.rst
225–226

I suggest presenting it as "The checker is currently limited to std::strings and doesn't recognize some of the more sophisticated approaches to passing unowned pointers around, such as std::string_views". It sounds a bit more negative than it deserves to sound, but that's the most documentation-like text i managed to come up with so far >.< Maybe put it under a "Known Limitations:" marker and/or expand the main part of the documentation in order to keep the reader's impression balanced, eg. "Many container methods in the C++ standard library are known to invalidate "references" (including actual references, iterators and raw pointers) to elements of the container. Using such references after they are invalidated causes undefined behavior, which is a common source of memory errors in C++ that this checker is capable of finding."

While this page is a documentation, I would still expect regular users to browse through it

I'd love our users to browse it! Maybe we should consider adding a documentation link to our HTML report headers as the documentation gets good enough.

232

The test_ part doesn't add much here, maybe drop it?