[PATCH] D156787: [analyzer][attr] Add docs to ownership_* attributes

[Aaron Ballman via Phabricator via cfe-commits]

aaron.ballman added a comment.

Thank you for adding documentation for these attributes, it's greatly appreciated!



================
Comment at: clang/include/clang/Basic/AttrDocs.td:1171-1175
+These attributes help the static analyzer understand custom ownership management
+functions. Neither of these affect the generated binary in any way, its only
+meant to assist the Clang Static Analyzer. You can use these annotations if your
+code uses wrapper functions around ``malloc`` or ``free``, or uses functions
+that take over ownership altogether.
----------------



================
Comment at: clang/include/clang/Basic/AttrDocs.td:1177-1180
+All 3 annotations have two parameters: the first is type of allocator used to
+allocate the memory (as of writing, the only accepted value is ``malloc``, but
+we intend to recognize ``new`` as well). We use this to check whether the
+correct deallocator is used. The second is an integer value.
----------------



================
Comment at: clang/include/clang/Basic/AttrDocs.td:1189
+``onwership_returns``: Functions with this annotation return dynamic memory.
+The second annotation parameter is the size of the returned memory in bytes.
+
----------------
I'm a bit confused on this last bit; how is the user supposed to statically know that value? I read this as claiming `my_malloc` returns 1 byte of dynamic storage; can the user tie the allocation to a parameter value instead?


================
Comment at: clang/include/clang/Basic/AttrDocs.td:1192-1193
+``ownership_takes``: Functions with this annotation deallocate one of the
+function parameters. The second annotation parameter is the index of the
+function parameter to deallocate.
+
----------------
We should document whether this is a 1-based index or a 0-based index, and we should also explain how the index works for member functions where there may be an implicit `this` argument involved.


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D156787/new/

https://reviews.llvm.org/D156787