[PATCH] D14274: Add alloc_size attribute to clang

[Aaron Ballman via cfe-commits]

aaron.ballman added inline comments.

================
Comment at: lib/AST/ExprConstant.cpp:1110
@@ -1022,1 +1109,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.
----------------
> 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.

That's a good point; I forgot that we use NDEBUG as our release+asserts flag (derp). That being said, this is basically the same as:
```
assert((!BInvalid || isa<MemberExpr>(B.get<Expr*>()) || tryUnwrapAllocSizeCall(B.get<Expr*>())) && "Unexpected type of invalid base");
```
Uncertain if that is an improvement or not, but it is shorter-ish. :-)

================
Comment at: lib/Sema/SemaDeclAttr.cpp:728
@@ +727,3 @@
+
+  const ParmVarDecl *Param = FD->getParamDecl(FuncParamNo - 1);
+  if (!Param->getType()->isIntegerType() && !Param->getType()->isCharType()) {
----------------
> Boolean makes no sense, but char is fine here. Thanks

isIntegerType() already covers isCharType(). I think what you want is:
```
if (!Param->getType()->isIntegerType() || Param->getType()->isBooleanType())
```


http://reviews.llvm.org/D14274