Reland "[Attr] Fix parameter indexing for several attributes"

Relands r326602 (reverted in r326862) with new test and fix for
PR36620.

Differential Revision: https://reviews.llvm.org/D43248

llvm-svn: 327405
diff --git a/clang/lib/Sema/SemaDeclAttr.cpp b/clang/lib/Sema/SemaDeclAttr.cpp
index 1453fd2..6df1324 100644
--- a/clang/lib/Sema/SemaDeclAttr.cpp
+++ b/clang/lib/Sema/SemaDeclAttr.cpp
@@ -312,7 +312,7 @@
 template <typename AttrInfo>
 static bool checkFunctionOrMethodParameterIndex(
     Sema &S, const Decl *D, const AttrInfo &AI, unsigned AttrArgNum,
-    const Expr *IdxExpr, uint64_t &Idx, bool AllowImplicitThis = false) {
+    const Expr *IdxExpr, ParamIdx &Idx, bool CanIndexImplicitThis = false) {
   assert(isFunctionOrMethodOrBlock(D));
 
   // In C++ the implicit 'this' function parameter also counts.
@@ -332,23 +332,22 @@
     return false;
   }
 
-  Idx = IdxInt.getLimitedValue();
-  if (Idx < 1 || (!IV && Idx > NumParams)) {
+  unsigned IdxSource = IdxInt.getLimitedValue(UINT_MAX);
+  if (IdxSource < 1 || (!IV && IdxSource > NumParams)) {
     S.Diag(getAttrLoc(AI), diag::err_attribute_argument_out_of_bounds)
-      << getAttrName(AI) << AttrArgNum << IdxExpr->getSourceRange();
+        << getAttrName(AI) << AttrArgNum << IdxExpr->getSourceRange();
     return false;
   }
-  Idx--; // Convert to zero-based.
-  if (HasImplicitThisParam && !AllowImplicitThis) {
-    if (Idx == 0) {
+  if (HasImplicitThisParam && !CanIndexImplicitThis) {
+    if (IdxSource == 1) {
       S.Diag(getAttrLoc(AI),
              diag::err_attribute_invalid_implicit_this_argument)
-        << getAttrName(AI) << IdxExpr->getSourceRange();
+          << getAttrName(AI) << IdxExpr->getSourceRange();
       return false;
     }
-    --Idx;
   }
 
+  Idx = ParamIdx(IdxSource, D);
   return true;
 }
 
@@ -773,18 +772,15 @@
 /// AttrArgNo is used to actually retrieve the argument, so it's base-0.
 template <typename AttrInfo>
 static bool checkParamIsIntegerType(Sema &S, const FunctionDecl *FD,
-                                    const AttrInfo &AI, unsigned AttrArgNo,
-                                    bool AllowDependentType = false) {
+                                    const AttrInfo &AI, unsigned AttrArgNo) {
   assert(AI.isArgExpr(AttrArgNo) && "Expected expression argument");
   Expr *AttrArg = AI.getArgAsExpr(AttrArgNo);
-  uint64_t Idx;
+  ParamIdx Idx;
   if (!checkFunctionOrMethodParameterIndex(S, FD, AI, AttrArgNo + 1, AttrArg,
                                            Idx))
     return false;
 
-  const ParmVarDecl *Param = FD->getParamDecl(Idx);
-  if (AllowDependentType && Param->getType()->isDependentType())
-    return true;
+  const ParmVarDecl *Param = FD->getParamDecl(Idx.getASTIndex());
   if (!Param->getType()->isIntegerType() && !Param->getType()->isCharType()) {
     SourceLocation SrcLoc = AttrArg->getLocStart();
     S.Diag(SrcLoc, diag::err_attribute_integers_only)
@@ -807,25 +803,24 @@
   }
 
   const Expr *SizeExpr = AL.getArgAsExpr(0);
-  int SizeArgNo;
+  int SizeArgNoVal;
   // Parameter indices are 1-indexed, hence Index=1
-  if (!checkPositiveIntArgument(S, AL, SizeExpr, SizeArgNo, /*Index=*/1))
+  if (!checkPositiveIntArgument(S, AL, SizeExpr, SizeArgNoVal, /*Index=*/1))
     return;
-
   if (!checkParamIsIntegerType(S, FD, AL, /*AttrArgNo=*/0))
     return;
+  ParamIdx SizeArgNo(SizeArgNoVal, D);
 
-  // Args are 1-indexed, so 0 implies that the arg was not present
-  int NumberArgNo = 0;
+  ParamIdx NumberArgNo;
   if (AL.getNumArgs() == 2) {
     const Expr *NumberExpr = AL.getArgAsExpr(1);
+    int Val;
     // Parameter indices are 1-based, hence Index=2
-    if (!checkPositiveIntArgument(S, AL, NumberExpr, NumberArgNo,
-                                  /*Index=*/2))
+    if (!checkPositiveIntArgument(S, AL, NumberExpr, Val, /*Index=*/2))
       return;
-
     if (!checkParamIsIntegerType(S, FD, AL, /*AttrArgNo=*/1))
       return;
+    NumberArgNo = ParamIdx(Val, D);
   }
 
   D->addAttr(::new (S.Context)
@@ -1425,18 +1420,19 @@
 }
 
 static void handleNonNullAttr(Sema &S, Decl *D, const AttributeList &AL) {
-  SmallVector<unsigned, 8> NonNullArgs;
+  SmallVector<ParamIdx, 8> NonNullArgs;
   for (unsigned I = 0; I < AL.getNumArgs(); ++I) {
     Expr *Ex = AL.getArgAsExpr(I);
-    uint64_t Idx;
+    ParamIdx Idx;
     if (!checkFunctionOrMethodParameterIndex(S, D, AL, I + 1, Ex, Idx))
       return;
 
     // Is the function argument a pointer type?
-    if (Idx < getFunctionOrMethodNumParams(D) &&
-        !attrNonNullArgCheck(S, getFunctionOrMethodParamType(D, Idx), AL,
-                             Ex->getSourceRange(),
-                             getFunctionOrMethodParamRange(D, Idx)))
+    if (Idx.getASTIndex() < getFunctionOrMethodNumParams(D) &&
+        !attrNonNullArgCheck(
+            S, getFunctionOrMethodParamType(D, Idx.getASTIndex()), AL,
+            Ex->getSourceRange(),
+            getFunctionOrMethodParamRange(D, Idx.getASTIndex())))
       continue;
 
     NonNullArgs.push_back(Idx);
@@ -1460,12 +1456,12 @@
       S.Diag(AL.getLoc(), diag::warn_attribute_nonnull_no_pointers);
   }
 
-  unsigned *Start = NonNullArgs.data();
+  ParamIdx *Start = NonNullArgs.data();
   unsigned Size = NonNullArgs.size();
   llvm::array_pod_sort(Start, Start + Size);
   D->addAttr(::new (S.Context)
-             NonNullAttr(AL.getRange(), S.Context, Start, Size,
-                         AL.getAttributeSpellingListIndex()));
+                 NonNullAttr(AL.getRange(), S.Context, Start, Size,
+                             AL.getAttributeSpellingListIndex()));
 }
 
 static void handleNonNullAttrParameter(Sema &S, ParmVarDecl *D,
@@ -1486,8 +1482,8 @@
     return;
 
   D->addAttr(::new (S.Context)
-             NonNullAttr(AL.getRange(), S.Context, nullptr, 0,
-                         AL.getAttributeSpellingListIndex()));
+                 NonNullAttr(AL.getRange(), S.Context, nullptr, 0,
+                             AL.getAttributeSpellingListIndex()));
 }
 
 static void handleReturnsNonNullAttr(Sema &S, Decl *D,
@@ -1588,7 +1584,7 @@
                              unsigned SpellingListIndex) {
   QualType ResultType = getFunctionOrMethodResultType(D);
 
-  AllocAlignAttr TmpAttr(AttrRange, Context, 0, SpellingListIndex);
+  AllocAlignAttr TmpAttr(AttrRange, Context, ParamIdx(), SpellingListIndex);
   SourceLocation AttrLoc = AttrRange.getBegin();
 
   if (!ResultType->isDependentType() &&
@@ -1598,28 +1594,22 @@
     return;
   }
 
-  uint64_t IndexVal;
+  ParamIdx Idx;
   const auto *FuncDecl = cast<FunctionDecl>(D);
   if (!checkFunctionOrMethodParameterIndex(*this, FuncDecl, TmpAttr,
-                                           /*AttrArgNo=*/1, ParamExpr,
-                                           IndexVal))
+                                           /*AttrArgNo=*/1, ParamExpr, Idx))
     return;
 
-  QualType Ty = getFunctionOrMethodParamType(D, IndexVal);
+  QualType Ty = getFunctionOrMethodParamType(D, Idx.getASTIndex());
   if (!Ty->isDependentType() && !Ty->isIntegralType(Context)) {
     Diag(ParamExpr->getLocStart(), diag::err_attribute_integers_only)
-        << &TmpAttr << FuncDecl->getParamDecl(IndexVal)->getSourceRange();
+        << &TmpAttr
+        << FuncDecl->getParamDecl(Idx.getASTIndex())->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, Context);
-
-  D->addAttr(::new (Context) AllocAlignAttr(
-      AttrRange, Context, Val.getZExtValue(), SpellingListIndex));
+  D->addAttr(::new (Context)
+                 AllocAlignAttr(AttrRange, Context, Idx, SpellingListIndex));
 }
 
 /// Normalize the attribute, __foo__ becomes foo.
@@ -1679,15 +1669,15 @@
     Module = &S.PP.getIdentifierTable().get(ModuleName);
   }
 
-  SmallVector<unsigned, 8> OwnershipArgs;
+  SmallVector<ParamIdx, 8> OwnershipArgs;
   for (unsigned i = 1; i < AL.getNumArgs(); ++i) {
     Expr *Ex = AL.getArgAsExpr(i);
-    uint64_t Idx;
+    ParamIdx Idx;
     if (!checkFunctionOrMethodParameterIndex(S, D, AL, i, Ex, Idx))
       return;
 
     // Is the function argument a pointer type?
-    QualType T = getFunctionOrMethodParamType(D, Idx);
+    QualType T = getFunctionOrMethodParamType(D, Idx.getASTIndex());
     int Err = -1;  // No error
     switch (K) {
       case OwnershipAttr::Takes:
@@ -1718,14 +1708,13 @@
       } else if (K == OwnershipAttr::Returns &&
                  I->getOwnKind() == OwnershipAttr::Returns) {
         // A returns attribute conflicts with any other returns attribute using
-        // a different index. Note, diagnostic reporting is 1-based, but stored
-        // argument indexes are 0-based.
+        // a different index.
         if (std::find(I->args_begin(), I->args_end(), Idx) == I->args_end()) {
           S.Diag(I->getLocation(), diag::err_ownership_returns_index_mismatch)
-              << *(I->args_begin()) + 1;
+              << I->args_begin()->getSourceIndex();
           if (I->args_size())
             S.Diag(AL.getLoc(), diag::note_ownership_returns_index_mismatch)
-                << (unsigned)Idx + 1 << Ex->getSourceRange();
+                << Idx.getSourceIndex() << Ex->getSourceRange();
           return;
         }
       }
@@ -1733,13 +1722,12 @@
     OwnershipArgs.push_back(Idx);
   }
 
-  unsigned* start = OwnershipArgs.data();
-  unsigned size = OwnershipArgs.size();
-  llvm::array_pod_sort(start, start + size);
-
+  ParamIdx *Start = OwnershipArgs.data();
+  unsigned Size = OwnershipArgs.size();
+  llvm::array_pod_sort(Start, Start + Size);
   D->addAttr(::new (S.Context)
-             OwnershipAttr(AL.getLoc(), S.Context, Module, start, size,
-                           AL.getAttributeSpellingListIndex()));
+                 OwnershipAttr(AL.getLoc(), S.Context, Module, Start, Size,
+                               AL.getAttributeSpellingListIndex()));
 }
 
 static void handleWeakRefAttr(Sema &S, Decl *D, const AttributeList &AL) {
@@ -3057,12 +3045,12 @@
 /// http://gcc.gnu.org/onlinedocs/gcc/Function-Attributes.html
 static void handleFormatArgAttr(Sema &S, Decl *D, const AttributeList &AL) {
   Expr *IdxExpr = AL.getArgAsExpr(0);
-  uint64_t Idx;
+  ParamIdx Idx;
   if (!checkFunctionOrMethodParameterIndex(S, D, AL, 1, IdxExpr, Idx))
     return;
 
   // Make sure the format string is really a string.
-  QualType Ty = getFunctionOrMethodParamType(D, Idx);
+  QualType Ty = getFunctionOrMethodParamType(D, Idx.getASTIndex());
 
   bool NotNSStringTy = !isNSStringType(Ty, S.Context);
   if (NotNSStringTy &&
@@ -3085,15 +3073,8 @@
     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;
-  IdxExpr->EvaluateAsInt(Val, S.Context);
-
-  D->addAttr(::new (S.Context)
-             FormatArgAttr(AL.getRange(), S.Context, Val.getZExtValue(),
-                           AL.getAttributeSpellingListIndex()));
+  D->addAttr(::new (S.Context) FormatArgAttr(
+      AL.getRange(), S.Context, Idx, AL.getAttributeSpellingListIndex()));
 }
 
 enum FormatAttrKind {
@@ -4487,13 +4468,13 @@
       << AL.getName() << /* arg num = */ 1 << AANT_ArgumentIdentifier;
     return;
   }
-  
-  uint64_t ArgumentIdx;
+
+  ParamIdx ArgumentIdx;
   if (!checkFunctionOrMethodParameterIndex(S, D, AL, 2, AL.getArgAsExpr(1),
                                            ArgumentIdx))
     return;
 
-  uint64_t TypeTagIdx;
+  ParamIdx TypeTagIdx;
   if (!checkFunctionOrMethodParameterIndex(S, D, AL, 3, AL.getArgAsExpr(2),
                                            TypeTagIdx))
     return;
@@ -4501,8 +4482,9 @@
   bool IsPointer = AL.getName()->getName() == "pointer_with_type_tag";
   if (IsPointer) {
     // Ensure that buffer has a pointer type.
-    if (ArgumentIdx >= getFunctionOrMethodNumParams(D) ||
-        !getFunctionOrMethodParamType(D, ArgumentIdx)->isPointerType())
+    unsigned ArgumentIdxAST = ArgumentIdx.getASTIndex();
+    if (ArgumentIdxAST >= getFunctionOrMethodNumParams(D) ||
+        !getFunctionOrMethodParamType(D, ArgumentIdxAST)->isPointerType())
       S.Diag(AL.getLoc(), diag::err_attribute_pointers_only)
           << AL.getName() << 0;
   }
@@ -4542,19 +4524,18 @@
                                     AL.getAttributeSpellingListIndex()));
 }
 
-static void handleXRayLogArgsAttr(Sema &S, Decl *D,
-                                  const AttributeList &AL) {
-  uint64_t ArgCount;
+static void handleXRayLogArgsAttr(Sema &S, Decl *D, const AttributeList &AL) {
+  ParamIdx ArgCount;
 
   if (!checkFunctionOrMethodParameterIndex(S, D, AL, 1, AL.getArgAsExpr(0),
                                            ArgCount,
-                                           true /* AllowImplicitThis*/))
+                                           true /* CanIndexImplicitThis */))
     return;
 
-  // ArgCount isn't a parameter index [0;n), it's a count [1;n] - hence + 1.
-  D->addAttr(::new (S.Context)
-                 XRayLogArgsAttr(AL.getRange(), S.Context, ++ArgCount,
-                                 AL.getAttributeSpellingListIndex()));
+  // ArgCount isn't a parameter index [0;n), it's a count [1;n]
+  D->addAttr(::new (S.Context) XRayLogArgsAttr(
+      AL.getRange(), S.Context, ArgCount.getSourceIndex(),
+      AL.getAttributeSpellingListIndex()));
 }
 
 //===----------------------------------------------------------------------===//