erichkeane marked 9 inline comments as done.
erichkeane added inline comments.
================
Comment at: lib/Sema/SemaDeclAttr.cpp:1608-1612
+ IndexVal += 1 + isInstanceMethod(FuncDecl);
+
+ if (!checkParamIsIntegerType(*this, FuncDecl, TmpAttr, ParamExpr, IndexVal,
+ /*AttrArgNo=*/0, /*AllowDependentType=*/true))
+ return;
----------------
aaron.ballman wrote:
> Hmmm, I think this might be a bit more clean as:
> ```
> QualType Ty = getFunctionOrMethodParamType(D, IndexVal);
> if (!Ty->isIntegralType(Context)) {
> Diag(ParamExpr->getLocStart(), diag::err_attribute_integers_only) <<
> TmpAttr <<
> FuncDecl->getParamDecl(IndexVal)->getSourceRange();
> return;
> }
>
> // We cannot use the Idx returned from checkFunctionOrMethodParameterIndex
> // because that has corrected for the implicit this parameter, and is zero-
> // based. The attribute expects what the user wrote explicitly.
> llvm::APSInt Val;
> ParamExpr->EvaluateAsInt(Val, S.Context);
>
> // Use Val.getZExtValue() when you need what the user wrote.
> ```
> This matches more closely with how other attributes handle the situation
> where the Attr object needs what the user wrote (like format_arg). I hadn't
> noticed that checkParamIsIntegerType() turns around and calls
> checkFunctionOrMethodParameterIndex() again, and this would simplify your
> patch a bit.
>
> What do you think?
Looks better, I hadn't realized checkParamIsIntegerType was redoing work either.
I had to make a pair of small changes to this (including omitting DependentType
from the integral type check), but it otherwise works nicely. Thanks!
https://reviews.llvm.org/D29599
_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
