[PATCH] D14274: Add alloc_size attribute to clang

[George Burgess IV via cfe-commits]

george.burgess.iv added inline comments.

================
Comment at: include/clang/Basic/Attr.td:714
@@ +713,3 @@
+  let Spellings = [GCC<"alloc_size">];
+  let Subjects = SubjectList<[Function]>;
+  let Args = [IntArgument<"ElemSizeParam">, IntArgument<"NumElemsParam", 1>];
----------------
aaron.ballman wrote:
> Should this be using HasFunctionProto instead of Function?
Nice catch!

================
Comment at: include/clang/Basic/AttrDocs.td:214
@@ +213,3 @@
+  that we can lower to in the near future. This should cover a large number of
+  nonconst cases.
+  }];
----------------
majnemer wrote:
> nonconst -> non-const?
Danke

================
Comment at: lib/AST/ExprConstant.cpp:1109
@@ -1022,1 +1108,3 @@
     void set(APValue::LValueBase B, unsigned I = 0, bool BInvalid = false) {
+#ifndef NDEBUG
+      // We only allow a few types of invalid bases. Enforce that here.
----------------
aaron.ballman wrote:
> Is there a reason this assert only fires in !NDEBUG mode? Is it too expensive to enable for any assert build?
I was under the impression that asserts are only enabled if !NDEBUG. The code only exists to make a few assertions, so I think guarding it with "if assertions are enabled" makes that purpose more clear. If my assumption is wrong, I'm happy to remove the ifndef.

================
Comment at: lib/Sema/SemaDeclAttr.cpp:728
@@ +727,3 @@
+
+  if (!FPT->getParamType(FuncParamNo - 1)->isIntegerType()) {
+    auto SrcLoc = Attr.getArgAsExpr(AttrArgNo)->getLocStart();
----------------
aaron.ballman wrote:
> Are character types and Boolean types also okay?
Boolean makes no sense, but char is fine here. Thanks

================
Comment at: lib/Sema/SemaDeclAttr.cpp:766
@@ +765,3 @@
+    S.Diag(Attr.getLoc(), diag::warn_attribute_wrong_decl_type)
+        << Attr.getName() << 4;
+    return;
----------------
aaron.ballman wrote:
> Please, no magic numbers. I don't think 4 is even the correct value, that's ExpectedParameter.
Doesn't matter; dead code.


http://reviews.llvm.org/D14274