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 _______________________________________________ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits