[PATCH] D14933: Add the allocsize attribute to LLVM

[George Burgess IV via llvm-commits]

george.burgess.iv added inline comments.

================
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
----------------
joker.eph wrote:
> 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>])`
> 
> The two parameters thing is not "super clean"

Agreed.

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

I like the idea of making the names more descriptive, but I'm not sure that saying `= 1` is the right thing here. Both of these are parameter numbers, so I feel that putting `NumElts = 1` may be a bit misleading. :)

================
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
----------------
joker.eph wrote:
> 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.
Good catch; I'll try "zero-indexed" instead.

================
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).
----------------
joker.eph wrote:
> 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
Good to know -- thanks!


http://reviews.llvm.org/D14933