[PATCH] D14933: Add the allocsize attribute to LLVM

[Mehdi AMINI via llvm-commits]

joker.eph accepted this revision.
joker.eph added a reviewer: joker.eph.
joker.eph added a comment.
This revision is now accepted and ready to land.

LGTM, but please see inline comment.


================
Comment at: docs/LangRef.rst:1253
@@ -1252,1 +1252,3 @@
     parentheses.
+``allocsize(<n>[, <m>])``
+    This attribute indicates that the annotated function will always return at
----------------
The two parameters thing is not "super clean", but I can see how it models the reality (i.e. `malloc` vs `calloc`).

Maybe specify it as: `allocsize(<EltSize>[, <NumElts = 1>])`


================
Comment at: docs/LangRef.rst:1255
@@ +1254,3 @@
+    This attribute indicates that the annotated function will always return at
+    least a given number of bytes (or null). Its arguments are base-0 parameter
+    numbers; if one argument is provided, then it's assumed that at least
----------------
Is "base-0" the right term? (I read it like "base-2" for binary, or "base-16" for hex). 
If there is better it would be nice.

================
Comment at: include/llvm/IR/Attributes.h:154
@@ -149,1 +153,3 @@
 
+  /// Returns the argument numbers for the allocsize attribute (or pair(0, 0)
+  /// if not known).
----------------
You should use brief only if you want multiple sentences to be part of the brief, otherwise we have "autobrief" turned on: http://lists.llvm.org/pipermail/llvm-dev/2015-May/085973.html

================
Comment at: include/llvm/IR/Attributes.h:279
@@ +278,3 @@
+  /// \brief Add the allocsize attribute to the attribute set at the given
+  /// index. Because attribute sets are immutable, this returns a new set.
+  AttributeSet addAllocSizeAttr(LLVMContext &C, unsigned Index,
----------------
Is brief intended?

================
Comment at: include/llvm/IR/Attributes.h:337
@@ +336,3 @@
+  /// \brief Get the allocsize argument numbers (or pair(0, 0) if unknown).
+  std::pair<unsigned, Optional<unsigned>>
+  getAllocSizeArgs(unsigned Index) const;
----------------
No brief

================
Comment at: include/llvm/IR/Attributes.h:504
@@ +503,3 @@
+  /// \brief Retrieve the allocsize args, if the allocsize attribute exists.
+  /// If it doesn't exist, pair(0, 0) is returned.
+  std::pair<unsigned, Optional<unsigned>> getAllocSizeArgs() const;
----------------
brief, and below.


http://reviews.llvm.org/D14933