[PATCH] D14933: Add the allocsize attribute to LLVM

[Mehdi AMINI via llvm-commits]

joker.eph added a comment.

Just some minor comment I wrote a few weeks ago, but didn't get to the end of the review (and won't have time in the short term).


================
Comment at: include/llvm/Analysis/MemoryBuiltins.h:170
@@ -169,3 +169,3 @@
 
-  bool bothKnown(SizeOffsetType &SizeOffset) {
+  bool bothKnown(SizeOffsetType &SizeOffset) const {
     return knownSize(SizeOffset) && knownOffset(SizeOffset);
----------------
It seems unrelated to the rest of this patch, can you commit now?

================
Comment at: include/llvm/IR/Attributes.h:154
@@ -149,1 +153,3 @@
 
+  /// \brief Returns the argument numbers form the allocsize attribute (or
+  /// pair(0, 0) if not known).
----------------
s/form/for/

Also no `\brief` anywhere unless you want multiple sentences as part of the brief (the code that has \brief is legacy).

================
Comment at: lib/Analysis/MemoryBuiltins.cpp:487
@@ +486,3 @@
+    return std::make_pair(S, Zero);
+  };
+
----------------
This is not clear to me, is this if we overflow the 32 bits that the pointer can represent? 
Is it consider UB or...?


http://reviews.llvm.org/D14933