jdenny updated this revision to Diff 135839.
jdenny retitled this revision from "[Attr] Fix printing of parameter indices in 
attributes" to "[Attr] Fix parameter indexing for attributes".
jdenny edited the summary of this revision.
jdenny added a comment.
Herald added a subscriber: kristof.beyls.

After several attempts at strategies Aaron and I discussed, I ended up going a 
different way.  See the new summary for details.  I believe this version 
addresses all the issues Aaron raised so far.


https://reviews.llvm.org/D43248

Files:
  include/clang/AST/Attr.h
  include/clang/Basic/Attr.td
  lib/AST/ExprConstant.cpp
  lib/CodeGen/CGCall.cpp
  lib/Sema/SemaChecking.cpp
  lib/Sema/SemaDecl.cpp
  lib/Sema/SemaDeclAttr.cpp
  lib/Sema/SemaTemplateInstantiateDecl.cpp
  lib/StaticAnalyzer/Checkers/MallocChecker.cpp
  lib/StaticAnalyzer/Checkers/NonNullParamChecker.cpp
  test/CodeGenCXX/alloc-size.cpp
  test/Misc/ast-dump-attr.cpp
  test/Sema/attr-ownership.cpp
  test/Sema/attr-print.cpp
  test/Sema/error-type-safety.cpp
  utils/TableGen/ClangAttrEmitter.cpp

Index: utils/TableGen/ClangAttrEmitter.cpp
===================================================================
--- utils/TableGen/ClangAttrEmitter.cpp
+++ utils/TableGen/ClangAttrEmitter.cpp
@@ -306,9 +306,6 @@
         return "!get" + getUpperName().str() + "()";
       if (type == "TypeSourceInfo *")
         return "false";
-      // FIXME: Do this declaratively in Attr.td.
-      if (getAttrName() == "AllocSize")
-        return "0 == get" + getUpperName().str() + "()";
       return "false";
     }
 
@@ -749,6 +746,192 @@
     }
   };
 
+  class VariadicParamIdxArgument : public Argument {
+    std::string IdxBeginName, SizeName;
+    bool AllowsThis;
+
+  public:
+    VariadicParamIdxArgument(const Record &Arg, StringRef Attr)
+        : Argument(Arg, Attr), IdxBeginName(getUpperName().str() + "_begin"),
+          SizeName(getUpperName().str() + "_size"),
+          AllowsThis(Arg.getValueAsBit("AllowsThis")) {}
+
+    bool isVariadic() const override { return true; }
+
+    void writeDeclarations(raw_ostream &OS) const override {
+      OS << "  ParamIdxItr " << IdxBeginName << ";\n";
+      OS << "  unsigned " << SizeName << ";\n";
+    }
+
+  public:
+    void writeAccessors(raw_ostream &OS) const override {
+      OS << "\n"
+         << "  static bool " << getLowerName() << "_allowsThis() {"
+         << " return " << AllowsThis << "; }\n";
+      OS << "  unsigned " << getLowerName() << "_size() const {"
+         << " return " << SizeName << "; }\n";
+      OS << "  ParamIdxItr " << getLowerName() << "_begin() const { return "
+         << IdxBeginName << "; }\n"
+         << "  ParamIdxItr " << getLowerName() << "_end() const { return "
+         << IdxBeginName << " + " << SizeName << "; }\n"
+         << "  llvm::iterator_range<ParamIdxItr> " << getLowerName()
+         << "() const { return llvm::make_range(" << getLowerName()
+         << "_begin(), " << getLowerName() << "_end()); }\n";
+    }
+
+    void writeCtorParameters(raw_ostream &OS) const override {
+      OS << "ParamIdxItr " << IdxBeginName << ", "
+         << "unsigned " << SizeName;
+    }
+
+    void writeCloneArgs(raw_ostream &OS) const override {
+      OS << IdxBeginName << ", " << SizeName;
+    }
+
+    void writeTemplateInstantiationArgs(raw_ostream &OS) const override {
+      // This isn't elegant, but we have to go through public methods...
+      OS << "A->" << getLowerName() << "_begin(), "
+         << "A->" << getLowerName() << "_size()";
+    }
+
+    void writeImplicitCtorArgs(raw_ostream &OS) const override {
+      OS << IdxBeginName << ", " << SizeName;
+    }
+
+    void writeCtorInitializers(raw_ostream &OS) const override {
+      OS << IdxBeginName << "(), ";
+      OS << SizeName << "(" << SizeName << ")";
+    }
+
+    void writeCtorDefaultInitializers(raw_ostream &OS) const override {
+      OS << IdxBeginName << "(), " << SizeName << "(0)";
+    }
+
+    void writeCtorBody(raw_ostream &OS) const override {
+      OS << "    unsigned *" << getUpperName()
+         << "_array = new (Ctx, 16) unsigned[" << SizeName << "];\n";
+      OS << "    std::copy((ParamIdxAtSrcItr)" << IdxBeginName
+         << ", (ParamIdxAtSrcItr)" << IdxBeginName << " + " << SizeName << ", "
+         << getUpperName() << "_array);\n";
+      OS << "    this->" << IdxBeginName << " = ParamIdxItr(" << getUpperName()
+         << "_array, " << IdxBeginName << ".hasThis());\n";
+    }
+
+    void writePCHReadDecls(raw_ostream &OS) const override {
+      OS << "    unsigned " << SizeName << " = Record.readInt();\n";
+      OS << "    SmallVector<unsigned, 4> " << getUpperName() << "_vec;\n"
+         << "    " << getUpperName() << "_vec.reserve(" << SizeName << ");\n"
+         << "    for (unsigned i = 0; i != " << SizeName << "; ++i)\n"
+         << "      " << getUpperName() << "_vec.push_back(Record.readInt());\n";
+      OS << "    bool " << getUpperName() << "HasThis = Record.readInt();\n";
+      OS << "    ParamIdxItr " << IdxBeginName << "(" << getUpperName()
+         << "_vec.data(), " << getUpperName() << "HasThis);\n";
+    }
+
+    void writePCHReadArgs(raw_ostream &OS) const override {
+      OS << IdxBeginName << ", " << SizeName;
+    }
+
+    void writePCHWrite(raw_ostream &OS) const override {
+      OS << "    Record.push_back(SA->" << getLowerName() << "_size());\n";
+      OS << "    for (auto Idx : SA->" << getLowerName() << "())\n"
+         << "      Record.push_back(Idx.atSrc());\n";
+      OS << "    Record.push_back(SA->" << getLowerName()
+         << "_begin().hasThis());\n";
+    }
+
+    void writeValue(raw_ostream &OS) const override {
+      OS << "\";\n";
+      OS << "  bool isFirst = true;\n"
+         << "  for (auto Idx : " << getLowerName() << "()) {\n"
+         << "    if (isFirst) isFirst = false;\n"
+         << "    else OS << \", \";\n"
+         << "    OS << Idx.atSrc();\n"
+         << "  }\n";
+      OS << "  OS << \"";
+    }
+
+    void writeDump(raw_ostream &OS) const override {
+      OS << "    for (auto Idx : SA->" << getLowerName() << "())\n";
+      OS << "      OS << \" \" << Idx.atSrc();\n";
+    }
+  };
+
+  class ParamIdxArgument : public Argument {
+    bool AllowsThis;
+    std::string IdxName;
+
+  public:
+    ParamIdxArgument(const Record &Arg, StringRef Attr)
+        : Argument(Arg, Attr), AllowsThis(Arg.getValueAsBit("AllowsThis")),
+          IdxName(getUpperName()) {}
+
+    void writeDeclarations(raw_ostream &OS) const override {
+      OS << "ParamIdx " << IdxName << ";\n";
+    }
+
+    void writeAccessors(raw_ostream &OS) const override {
+      OS << "\n"
+         << "  static bool " << getLowerName() << "_allowsThis() {"
+         << " return " << AllowsThis << "; }\n";
+      OS << "  ParamIdx " << getLowerName() << "() const {"
+         << " return " << IdxName << "; }\n";
+    }
+
+    void writeCtorParameters(raw_ostream &OS) const override {
+      OS << "ParamIdx " << IdxName;
+    }
+
+    void writeCloneArgs(raw_ostream &OS) const override { OS << IdxName; }
+
+    void writeTemplateInstantiationArgs(raw_ostream &OS) const override {
+      OS << "A->" << getLowerName() << "()";
+    }
+
+    void writeImplicitCtorArgs(raw_ostream &OS) const override {
+      OS << IdxName;
+    }
+
+    void writeCtorInitializers(raw_ostream &OS) const override {
+      OS << IdxName << "(" << IdxName << ")";
+    }
+
+    void writeCtorDefaultInitializers(raw_ostream &OS) const override {
+      OS << IdxName << "()";
+    }
+
+    void writePCHReadDecls(raw_ostream &OS) const override {
+      OS << "    unsigned " << IdxName << "Src = Record.readInt();\n";
+      OS << "    bool " << IdxName << "HasThis = Record.readInt();\n";
+    }
+
+    void writePCHReadArgs(raw_ostream &OS) const override {
+      OS << "ParamIdx(" << IdxName << "Src, " << IdxName << "HasThis)";
+    }
+
+    void writePCHWrite(raw_ostream &OS) const override {
+      OS << "    Record.push_back(SA->" << getLowerName()
+         << "().isValid() ? SA->" << getLowerName() << "().atSrc() : 0);\n";
+      OS << "    Record.push_back(SA->" << getLowerName()
+         << "().isValid() ? SA->" << getLowerName()
+         << "().hasThis() : false);\n";
+    }
+
+    std::string getIsOmitted() const override {
+      return "!" + IdxName + ".isValid()";
+    }
+
+    void writeValue(raw_ostream &OS) const override {
+      OS << "\" << " << IdxName << ".atSrc() << \"";
+    }
+
+    void writeDump(raw_ostream &OS) const override {
+      if (isOptional())
+        OS << "    if (SA->" << getLowerName() << "().isValid())\n  ";
+      OS << "    OS << \" \" << SA->" << getLowerName() << "().atSrc();\n";
+    }
+  };
+
   // Unique the enums, but maintain the original declaration ordering.
   std::vector<StringRef>
   uniqueEnumsInOrder(const std::vector<StringRef> &enums) {
@@ -1248,6 +1431,10 @@
     Ptr = llvm::make_unique<VariadicEnumArgument>(Arg, Attr);
   else if (ArgName == "VariadicExprArgument")
     Ptr = llvm::make_unique<VariadicExprArgument>(Arg, Attr);
+  else if (ArgName == "VariadicParamIdxArgument")
+    Ptr = llvm::make_unique<VariadicParamIdxArgument>(Arg, Attr);
+  else if (ArgName == "ParamIdxArgument")
+    Ptr = llvm::make_unique<ParamIdxArgument>(Arg, Attr);
   else if (ArgName == "VersionArgument")
     Ptr = llvm::make_unique<VersionArgument>(Arg, Attr);
 
Index: test/Sema/error-type-safety.cpp
===================================================================
--- test/Sema/error-type-safety.cpp
+++ test/Sema/error-type-safety.cpp
@@ -3,21 +3,50 @@
 #define INT_TAG 42
 
 static const int test_in
-  __attribute__((type_tag_for_datatype(test, int))) = INT_TAG;
+    __attribute__((type_tag_for_datatype(test, int))) = INT_TAG;
 
 // Argument index: 1, Type tag index: 2
 void test_bounds_index(...)
-  __attribute__((argument_with_type_tag(test, 1, 2)));
+    __attribute__((argument_with_type_tag(test, 1, 2)));
+
+// Argument index: 1, Type tag index: 2
+void test_bounds_index_ptr(void *, ...)
+    __attribute__((pointer_with_type_tag(test, 1, 2)));
 
 // Argument index: 3, Type tag index: 1
 void test_bounds_arg_index(...)
-  __attribute__((argument_with_type_tag(test, 3, 1)));
+    __attribute__((argument_with_type_tag(test, 3, 1)));
+
+class C {
+public:
+  // Argument index: 2, Type tag index: 3
+  void test_bounds_index(...)
+      __attribute__((argument_with_type_tag(test, 2, 3)));
+
+  // Argument index: 2, Type tag index: 3
+  void test_bounds_index_ptr(void *, ...)
+      __attribute__((pointer_with_type_tag(test, 2, 3)));
+
+  // Argument index: 4, Type tag index: 2
+  void test_bounds_arg_index(...)
+      __attribute__((argument_with_type_tag(test, 4, 2)));
+};
 
 void test_bounds()
 {
+  C c;
+
   // Test the boundary edges (ensure no off-by-one) with argument indexing.
   test_bounds_index(1, INT_TAG);
+  c.test_bounds_index(1, INT_TAG);
+  test_bounds_index_ptr(0, INT_TAG);
+  c.test_bounds_index_ptr(0, INT_TAG);
+
+  test_bounds_index(1);       // expected-error {{type tag index 2 is greater than the number of arguments specified}}
+  c.test_bounds_index(1);     // expected-error {{type tag index 3 is greater than the number of arguments specified}}
+  test_bounds_index_ptr(0);   // expected-error {{type tag index 2 is greater than the number of arguments specified}}
+  c.test_bounds_index_ptr(0); // expected-error {{type tag index 3 is greater than the number of arguments specified}}
 
-  test_bounds_index(1); // expected-error {{type tag index 2 is greater than the number of arguments specified}}
-  test_bounds_arg_index(INT_TAG, 1); // expected-error {{argument index 3 is greater than the number of arguments specified}}
+  test_bounds_arg_index(INT_TAG, 1);   // expected-error {{argument index 3 is greater than the number of arguments specified}}
+  c.test_bounds_arg_index(INT_TAG, 1); // expected-error {{argument index 4 is greater than the number of arguments specified}}
 }
Index: test/Sema/attr-print.cpp
===================================================================
--- test/Sema/attr-print.cpp
+++ test/Sema/attr-print.cpp
@@ -1,6 +1,67 @@
 // RUN: %clang_cc1 %s -ast-print | FileCheck %s
 
+// CHECK: void xla(int a) __attribute__((xray_log_args(1)));
+void xla(int a) __attribute__((xray_log_args(1)));
+
 // CHECK: void *as2(int, int) __attribute__((alloc_size(1, 2)));
 void *as2(int, int) __attribute__((alloc_size(1, 2)));
 // CHECK: void *as1(void *, int) __attribute__((alloc_size(2)));
 void *as1(void *, int) __attribute__((alloc_size(2)));
+
+// CHECK: void fmt(int, const char *, ...) __attribute__((format(printf, 2, 3)));
+void fmt(int, const char *, ...) __attribute__((format(printf, 2, 3)));
+
+// CHECK: char *fmta(int, const char *) __attribute__((format_arg(2)));
+char *fmta(int, const char *) __attribute__((format_arg(2)));
+
+// CHECK: void nn(int *, int *) __attribute__((nonnull(1, 2)));
+void nn(int *, int *) __attribute__((nonnull(1, 2)));
+
+// CHECK: int *aa(int i) __attribute__((alloc_align(1)));
+int *aa(int i) __attribute__((alloc_align(1)));
+
+// CHECK: void ownt(int *, int *) __attribute__((ownership_takes(foo, 1, 2)));
+void ownt(int *, int *) __attribute__((ownership_takes(foo, 1, 2)));
+// CHECK: void ownh(int *, int *) __attribute__((ownership_holds(foo, 1, 2)));
+void ownh(int *, int *) __attribute__((ownership_holds(foo, 1, 2)));
+// CHECK: void ownr(int) __attribute__((ownership_returns(foo, 1)));
+void ownr(int) __attribute__((ownership_returns(foo, 1)));
+
+// CHECK: void awtt(int, int, ...) __attribute__((argument_with_type_tag(foo, 3, 2)));
+void awtt(int, int, ...) __attribute__((argument_with_type_tag(foo, 3, 2)));
+// CHECK: void pwtt(void *, int) __attribute__((pointer_with_type_tag(foo, 1, 2)));
+void pwtt(void *, int) __attribute__((pointer_with_type_tag(foo, 1, 2)));
+
+class C {
+  // CHECK: void xla(int a) __attribute__((xray_log_args(2)));
+  void xla(int a) __attribute__((xray_log_args(2)));
+
+  // CHECK: void *as2(int, int) __attribute__((alloc_size(2, 3)));
+  void *as2(int, int) __attribute__((alloc_size(2, 3)));
+  // CHECK: void *as1(void *, int) __attribute__((alloc_size(3)));
+  void *as1(void *, int) __attribute__((alloc_size(3)));
+
+  // CHECK: void fmt(int, const char *, ...) __attribute__((format(printf, 3, 4)));
+  void fmt(int, const char *, ...) __attribute__((format(printf, 3, 4)));
+
+  // CHECK: char *fmta(int, const char *) __attribute__((format_arg(3)));
+  char *fmta(int, const char *) __attribute__((format_arg(3)));
+
+  // CHECK: void nn(int *, int *) __attribute__((nonnull(2, 3)));
+  void nn(int *, int *) __attribute__((nonnull(2, 3)));
+
+  // CHECK: int *aa(int i) __attribute__((alloc_align(2)));
+  int *aa(int i) __attribute__((alloc_align(2)));
+
+  // CHECK: void ownt(int *, int *) __attribute__((ownership_takes(foo, 2, 3)));
+  void ownt(int *, int *) __attribute__((ownership_takes(foo, 2, 3)));
+  // CHECK: void ownh(int *, int *) __attribute__((ownership_holds(foo, 2, 3)));
+  void ownh(int *, int *) __attribute__((ownership_holds(foo, 2, 3)));
+  // CHECK: void ownr(int) __attribute__((ownership_returns(foo, 2)));
+  void ownr(int) __attribute__((ownership_returns(foo, 2)));
+
+  // CHECK: void awtt(int, int, ...) __attribute__((argument_with_type_tag(foo, 4, 3)));
+  void awtt(int, int, ...) __attribute__((argument_with_type_tag(foo, 4, 3)));
+  // CHECK: void pwtt(void *, int) __attribute__((pointer_with_type_tag(foo, 2, 3)));
+  void pwtt(void *, int) __attribute__((pointer_with_type_tag(foo, 2, 3)));
+};
Index: test/Sema/attr-ownership.cpp
===================================================================
--- /dev/null
+++ test/Sema/attr-ownership.cpp
@@ -0,0 +1,7 @@
+// RUN: %clang_cc1 %s -verify
+
+class C {
+  void f(int, int)
+      __attribute__((ownership_returns(foo, 2)))  // expected-note {{declared with index 2 here}}
+      __attribute__((ownership_returns(foo, 3))); // expected-error {{'ownership_returns' attribute index does not match; here it is 3}}
+};
Index: test/Misc/ast-dump-attr.cpp
===================================================================
--- test/Misc/ast-dump-attr.cpp
+++ test/Misc/ast-dump-attr.cpp
@@ -68,12 +68,12 @@
 void TestBool(void *, int)
 __attribute__((pointer_with_type_tag(bool1,1,2)));
 // CHECK: FunctionDecl{{.*}}TestBool
-// CHECK:   ArgumentWithTypeTagAttr{{.*}}pointer_with_type_tag bool1 0 1 IsPointer
+// CHECK:   ArgumentWithTypeTagAttr{{.*}}pointer_with_type_tag bool1 1 2 IsPointer
 
 void TestUnsigned(void *, int)
 __attribute__((pointer_with_type_tag(unsigned1,1,2)));
 // CHECK: FunctionDecl{{.*}}TestUnsigned
-// CHECK:   ArgumentWithTypeTagAttr{{.*}} pointer_with_type_tag unsigned1 0 1
+// CHECK:   ArgumentWithTypeTagAttr{{.*}} pointer_with_type_tag unsigned1 1 2
 
 void TestInt(void) __attribute__((constructor(123)));
 // CHECK:      FunctionDecl{{.*}}TestInt
Index: test/CodeGenCXX/alloc-size.cpp
===================================================================
--- test/CodeGenCXX/alloc-size.cpp
+++ test/CodeGenCXX/alloc-size.cpp
@@ -69,4 +69,22 @@
          __builtin_object_size(dependent_calloc<7, 8>(), 0) +
          __builtin_object_size(dependent_calloc2<int, 9>(), 0);
 }
+} // namespace templated_alloc_size
+
+class C {
+public:
+  void *my_malloc(int N) __attribute__((alloc_size(2)));
+  void *my_calloc(int N, int M) __attribute__((alloc_size(2, 3)));
+};
+
+// CHECK-LABEL: define i32 @_Z16callMemberMallocv
+int callMemberMalloc() {
+  // CHECK: ret i32 16
+  return __builtin_object_size(C().my_malloc(16), 0);
+}
+
+// CHECK-LABEL: define i32 @_Z16callMemberCallocv
+int callMemberCalloc() {
+  // CHECK: ret i32 32
+  return __builtin_object_size(C().my_calloc(16, 2), 0);
 }
Index: lib/StaticAnalyzer/Checkers/NonNullParamChecker.cpp
===================================================================
--- lib/StaticAnalyzer/Checkers/NonNullParamChecker.cpp
+++ lib/StaticAnalyzer/Checkers/NonNullParamChecker.cpp
@@ -58,10 +58,10 @@
       AttrNonNull.set(0, NumArgs);
       break;
     }
-    for (unsigned Val : NonNull->args()) {
-      if (Val >= NumArgs)
+    for (ParamIdx Idx : NonNull->args()) {
+      if (Idx.atAST() >= NumArgs)
         continue;
-      AttrNonNull.set(Val);
+      AttrNonNull.set(Idx.atAST());
     }
   }
 
Index: lib/StaticAnalyzer/Checkers/MallocChecker.cpp
===================================================================
--- lib/StaticAnalyzer/Checkers/MallocChecker.cpp
+++ lib/StaticAnalyzer/Checkers/MallocChecker.cpp
@@ -1222,9 +1222,9 @@
   if (Att->getModule() != II_malloc)
     return nullptr;
 
-  OwnershipAttr::args_iterator I = Att->args_begin(), E = Att->args_end();
+  ParamIdxItr I = Att->args_begin(), E = Att->args_end();
   if (I != E) {
-    return MallocMemAux(C, CE, CE->getArg(*I), UndefinedVal(), State);
+    return MallocMemAux(C, CE, CE->getArg(I->atAST()), UndefinedVal(), State);
   }
   return MallocMemAux(C, CE, UnknownVal(), UndefinedVal(), State);
 }
@@ -1322,9 +1322,9 @@
   bool ReleasedAllocated = false;
 
   for (const auto &Arg : Att->args()) {
-    ProgramStateRef StateI = FreeMemAux(C, CE, State, Arg,
-                               Att->getOwnKind() == OwnershipAttr::Holds,
-                               ReleasedAllocated);
+    ProgramStateRef StateI = FreeMemAux(
+        C, CE, State, Arg.atAST(), Att->getOwnKind() == OwnershipAttr::Holds,
+        ReleasedAllocated);
     if (StateI)
       State = StateI;
   }
Index: lib/Sema/SemaTemplateInstantiateDecl.cpp
===================================================================
--- lib/Sema/SemaTemplateInstantiateDecl.cpp
+++ lib/Sema/SemaTemplateInstantiateDecl.cpp
@@ -176,7 +176,7 @@
     Sema &S, const MultiLevelTemplateArgumentList &TemplateArgs,
     const AllocAlignAttr *Align, Decl *New) {
   Expr *Param = IntegerLiteral::Create(
-      S.getASTContext(), llvm::APInt(64, Align->getParamIndex()),
+      S.getASTContext(), llvm::APInt(64, Align->paramIndex().atSrc()),
       S.getASTContext().UnsignedLongLongTy, Align->getLocation());
   S.AddAllocAlignAttr(Align->getLocation(), New, Param,
                       Align->getSpellingListIndex());
Index: lib/Sema/SemaDeclAttr.cpp
===================================================================
--- lib/Sema/SemaDeclAttr.cpp
+++ lib/Sema/SemaDeclAttr.cpp
@@ -309,9 +309,10 @@
 ///
 /// \returns true if IdxExpr is a valid index.
 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) {
+static bool
+checkFunctionOrMethodParameterIndex(Sema &S, const Decl *D, const AttrInfo &AI,
+                                    unsigned AttrArgNum, const Expr *IdxExpr,
+                                    ParamIdx &Idx, bool AllowImplicitThis) {
   assert(isFunctionOrMethodOrBlock(D));
 
   // In C++ the implicit 'this' function parameter also counts.
@@ -331,21 +332,19 @@
     return false;
   }
 
-  Idx = IdxInt.getLimitedValue();
-  if (Idx < 1 || (!IV && Idx > NumParams)) {
+  Idx = ParamIdx(IdxInt.getLimitedValue(UINT_MAX), D);
+  if (Idx.atSrc() < 1 || (!IV && Idx.atSrc() > 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 (Idx.atSrc() == 1) {
       S.Diag(getAttrLoc(AI),
              diag::err_attribute_invalid_implicit_this_argument)
-        << getAttrName(AI) << IdxExpr->getSourceRange();
+          << getAttrName(AI) << IdxExpr->getSourceRange();
       return false;
     }
-    --Idx;
   }
 
   return true;
@@ -773,17 +772,15 @@
 template <typename AttrInfo>
 static bool checkParamIsIntegerType(Sema &S, const FunctionDecl *FD,
                                     const AttrInfo &AI, unsigned AttrArgNo,
-                                    bool AllowDependentType = false) {
+                                    bool AllowImplicitThis) {
   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))
+                                           Idx, AllowImplicitThis))
     return false;
 
-  const ParmVarDecl *Param = FD->getParamDecl(Idx);
-  if (AllowDependentType && Param->getType()->isDependentType())
-    return true;
+  const ParmVarDecl *Param = FD->getParamDecl(Idx.atAST());
   if (!Param->getType()->isIntegerType() && !Param->getType()->isCharType()) {
     SourceLocation SrcLoc = AttrArg->getLocStart();
     S.Diag(SrcLoc, diag::err_attribute_integers_only)
@@ -806,24 +803,27 @@
   }
 
   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;
+  ParamIdx SizeArgNo(SizeArgNoVal, D);
 
-  if (!checkParamIsIntegerType(S, FD, AL, /*AttrArgNo=*/0))
+  if (!checkParamIsIntegerType(S, FD, AL, /*AttrArgNo=*/0,
+                               AllocSizeAttr::elemSizeParam_allowsThis()))
     return;
 
-  // 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;
+    NumberArgNo = ParamIdx(Val, D);
 
-    if (!checkParamIsIntegerType(S, FD, AL, /*AttrArgNo=*/1))
+    if (!checkParamIsIntegerType(S, FD, AL, /*AttrArgNo=*/1,
+                                 AllocSizeAttr::numElemsParam_allowsThis()))
       return;
   }
 
@@ -1427,18 +1427,19 @@
   SmallVector<unsigned, 8> NonNullArgs;
   for (unsigned I = 0; I < AL.getNumArgs(); ++I) {
     Expr *Ex = AL.getArgAsExpr(I);
-    uint64_t Idx;
-    if (!checkFunctionOrMethodParameterIndex(S, D, AL, I + 1, Ex, Idx))
+    ParamIdx Idx;
+    if (!checkFunctionOrMethodParameterIndex(S, D, AL, I + 1, Ex, Idx,
+                                             NonNullAttr::args_allowsThis()))
       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.atAST() < getFunctionOrMethodNumParams(D) &&
+        !attrNonNullArgCheck(S, getFunctionOrMethodParamType(D, Idx.atAST()),
+                             AL, Ex->getSourceRange(),
+                             getFunctionOrMethodParamRange(D, Idx.atAST())))
       continue;
 
-    NonNullArgs.push_back(Idx);
+    NonNullArgs.push_back(Idx.atSrc());
   }
 
   // If no arguments were specified to __attribute__((nonnull)) then all pointer
@@ -1462,9 +1463,10 @@
   unsigned *Start = NonNullArgs.data();
   unsigned Size = NonNullArgs.size();
   llvm::array_pod_sort(Start, Start + Size);
+  ParamIdxItr Begin(Start, D);
   D->addAttr(::new (S.Context)
-             NonNullAttr(AL.getRange(), S.Context, Start, Size,
-                         AL.getAttributeSpellingListIndex()));
+                 NonNullAttr(AL.getRange(), S.Context, Begin, Size,
+                             AL.getAttributeSpellingListIndex()));
 }
 
 static void handleNonNullAttrParameter(Sema &S, ParmVarDecl *D,
@@ -1485,8 +1487,8 @@
     return;
 
   D->addAttr(::new (S.Context)
-             NonNullAttr(AL.getRange(), S.Context, nullptr, 0,
-                         AL.getAttributeSpellingListIndex()));
+                 NonNullAttr(AL.getRange(), S.Context, ParamIdxItr(), 0,
+                             AL.getAttributeSpellingListIndex()));
 }
 
 static void handleReturnsNonNullAttr(Sema &S, Decl *D,
@@ -1587,7 +1589,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() &&
@@ -1597,28 +1599,22 @@
     return;
   }
 
-  uint64_t IndexVal;
+  ParamIdx Idx;
   const auto *FuncDecl = cast<FunctionDecl>(D);
-  if (!checkFunctionOrMethodParameterIndex(*this, FuncDecl, TmpAttr,
-                                           /*AttrArgNo=*/1, ParamExpr,
-                                           IndexVal))
+  if (!checkFunctionOrMethodParameterIndex(
+          *this, FuncDecl, TmpAttr, /*AttrArgNo=*/1, ParamExpr, Idx,
+          AllocAlignAttr::paramIndex_allowsThis()))
     return;
 
-  QualType Ty = getFunctionOrMethodParamType(D, IndexVal);
+  QualType Ty = getFunctionOrMethodParamType(D, Idx.atAST());
   if (!Ty->isDependentType() && !Ty->isIntegralType(Context)) {
     Diag(ParamExpr->getLocStart(), diag::err_attribute_integers_only)
-        << &TmpAttr << FuncDecl->getParamDecl(IndexVal)->getSourceRange();
+        << &TmpAttr << FuncDecl->getParamDecl(Idx.atAST())->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.
@@ -1649,8 +1645,9 @@
 
   // Figure out our Kind.
   OwnershipAttr::OwnershipKind K =
-      OwnershipAttr(AL.getLoc(), S.Context, nullptr, nullptr, 0,
-                    AL.getAttributeSpellingListIndex()).getOwnKind();
+      OwnershipAttr(AL.getLoc(), S.Context, nullptr, ParamIdxItr(), 0,
+                    AL.getAttributeSpellingListIndex())
+          .getOwnKind();
 
   // Check arguments.
   switch (K) {
@@ -1681,12 +1678,13 @@
   SmallVector<unsigned, 8> OwnershipArgs;
   for (unsigned i = 1; i < AL.getNumArgs(); ++i) {
     Expr *Ex = AL.getArgAsExpr(i);
-    uint64_t Idx;
-    if (!checkFunctionOrMethodParameterIndex(S, D, AL, i, Ex, Idx))
+    ParamIdx Idx;
+    if (!checkFunctionOrMethodParameterIndex(S, D, AL, i, Ex, Idx,
+                                             OwnershipAttr::args_allowsThis()))
       return;
 
     // Is the function argument a pointer type?
-    QualType T = getFunctionOrMethodParamType(D, Idx);
+    QualType T = getFunctionOrMethodParamType(D, Idx.atAST());
     int Err = -1;  // No error
     switch (K) {
       case OwnershipAttr::Takes:
@@ -1709,36 +1707,35 @@
     for (const auto *I : D->specific_attrs<OwnershipAttr>()) {
       // Cannot have two ownership attributes of different kinds for the same
       // index.
-      if (I->getOwnKind() != K && I->args_end() !=
-          std::find(I->args_begin(), I->args_end(), Idx)) {
+      if (I->getOwnKind() != K &&
+          I->args_end() != std::find(I->args_begin(), I->args_end(), Idx)) {
         S.Diag(AL.getLoc(), diag::err_attributes_are_not_compatible)
-          << AL.getName() << I;
+            << AL.getName() << I;
         return;
       } 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()->atSrc();
           if (I->args_size())
             S.Diag(AL.getLoc(), diag::note_ownership_returns_index_mismatch)
-                << (unsigned)Idx + 1 << Ex->getSourceRange();
+                << Idx.atSrc() << Ex->getSourceRange();
           return;
         }
       }
     }
-    OwnershipArgs.push_back(Idx);
+    OwnershipArgs.push_back(Idx.atSrc());
   }
 
-  unsigned* start = OwnershipArgs.data();
+  unsigned *start = OwnershipArgs.data();
   unsigned size = OwnershipArgs.size();
   llvm::array_pod_sort(start, start + size);
-
+  ParamIdxItr Begin(start, D);
   D->addAttr(::new (S.Context)
-             OwnershipAttr(AL.getLoc(), S.Context, Module, start, size,
-                           AL.getAttributeSpellingListIndex()));
+                 OwnershipAttr(AL.getLoc(), S.Context, Module, Begin, size,
+                               AL.getAttributeSpellingListIndex()));
 }
 
 static void handleWeakRefAttr(Sema &S, Decl *D, const AttributeList &AL) {
@@ -3109,12 +3106,13 @@
 /// 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;
-  if (!checkFunctionOrMethodParameterIndex(S, D, AL, 1, IdxExpr, Idx))
+  ParamIdx Idx;
+  if (!checkFunctionOrMethodParameterIndex(
+          S, D, AL, 1, IdxExpr, Idx, FormatArgAttr::formatIdx_allowsThis()))
     return;
 
   // Make sure the format string is really a string.
-  QualType Ty = getFunctionOrMethodParamType(D, Idx);
+  QualType Ty = getFunctionOrMethodParamType(D, Idx.atAST());
 
   bool NotNSStringTy = !isNSStringType(Ty, S.Context);
   if (NotNSStringTy &&
@@ -3137,15 +3135,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 {
@@ -4539,22 +4530,24 @@
       << AL.getName() << /* arg num = */ 1 << AANT_ArgumentIdentifier;
     return;
   }
-  
-  uint64_t ArgumentIdx;
-  if (!checkFunctionOrMethodParameterIndex(S, D, AL, 2, AL.getArgAsExpr(1),
-                                           ArgumentIdx))
+
+  ParamIdx ArgumentIdx;
+  if (!checkFunctionOrMethodParameterIndex(
+          S, D, AL, 2, AL.getArgAsExpr(1), ArgumentIdx,
+          ArgumentWithTypeTagAttr::argumentIdx_allowsThis()))
     return;
 
-  uint64_t TypeTagIdx;
-  if (!checkFunctionOrMethodParameterIndex(S, D, AL, 3, AL.getArgAsExpr(2),
-                                           TypeTagIdx))
+  ParamIdx TypeTagIdx;
+  if (!checkFunctionOrMethodParameterIndex(
+          S, D, AL, 3, AL.getArgAsExpr(2), TypeTagIdx,
+          ArgumentWithTypeTagAttr::typeTagIdx_allowsThis()))
     return;
 
   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())
+    if (ArgumentIdx.atAST() >= getFunctionOrMethodNumParams(D) ||
+        !getFunctionOrMethodParamType(D, ArgumentIdx.atAST())->isPointerType())
       S.Diag(AL.getLoc(), diag::err_attribute_pointers_only)
           << AL.getName() << 0;
   }
@@ -4594,18 +4587,17 @@
                                     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*/))
     return;
 
-  // ArgCount isn't a parameter index [0;n), it's a count [1;n] - hence + 1.
+  // 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,
+                 XRayLogArgsAttr(AL.getRange(), S.Context, ArgCount.atSrc(),
                                  AL.getAttributeSpellingListIndex()));
 }
 
Index: lib/Sema/SemaDecl.cpp
===================================================================
--- lib/Sema/SemaDecl.cpp
+++ lib/Sema/SemaDecl.cpp
@@ -13138,7 +13138,7 @@
     // We already have a __builtin___CFStringMakeConstantString,
     // but builds that use -fno-constant-cfstrings don't go through that.
     if (!FD->hasAttr<FormatArgAttr>())
-      FD->addAttr(FormatArgAttr::CreateImplicit(Context, 1,
+      FD->addAttr(FormatArgAttr::CreateImplicit(Context, ParamIdx(1, FD),
                                                 FD->getLocation()));
   }
 }
Index: lib/Sema/SemaChecking.cpp
===================================================================
--- lib/Sema/SemaChecking.cpp
+++ lib/Sema/SemaChecking.cpp
@@ -2619,12 +2619,12 @@
         return;
       }
 
-      for (unsigned Val : NonNull->args()) {
-        if (Val >= Args.size())
+      for (ParamIdx Idx : NonNull->args()) {
+        if (Idx.atAST() >= Args.size())
           continue;
         if (NonNullArgs.empty())
           NonNullArgs.resize(Args.size());
-        NonNullArgs.set(Val);
+        NonNullArgs.set(Idx.atAST());
       }
     }
   }
@@ -5002,12 +5002,7 @@
     const CallExpr *CE = cast<CallExpr>(E);
     if (const NamedDecl *ND = dyn_cast_or_null<NamedDecl>(CE->getCalleeDecl())) {
       if (const FormatArgAttr *FA = ND->getAttr<FormatArgAttr>()) {
-        unsigned ArgIndex = FA->getFormatIdx();
-        if (const CXXMethodDecl *MD = dyn_cast<CXXMethodDecl>(ND))
-          if (MD->isInstance())
-            --ArgIndex;
-        const Expr *Arg = CE->getArg(ArgIndex - 1);
-
+        const Expr *Arg = CE->getArg(FA->formatIdx().atAST());
         return checkFormatStringExpr(S, Arg, Args,
                                      HasVAListArg, format_idx, firstDataArg,
                                      Type, CallType, InFunctionCall,
@@ -5032,8 +5027,7 @@
     const auto *ME = cast<ObjCMessageExpr>(E);
     if (const auto *ND = ME->getMethodDecl()) {
       if (const auto *FA = ND->getAttr<FormatArgAttr>()) {
-        unsigned ArgIndex = FA->getFormatIdx();
-        const Expr *Arg = ME->getArg(ArgIndex - 1);
+        const Expr *Arg = ME->getArg(FA->formatIdx().atAST());
         return checkFormatStringExpr(
             S, Arg, Args, HasVAListArg, format_idx, firstDataArg, Type,
             CallType, InFunctionCall, CheckedVarArgs, UncoveredArg, Offset);
@@ -10086,8 +10080,8 @@
               return;
           }
 
-          for (unsigned ArgNo : NonNull->args()) {
-            if (ArgNo == ParamNo) {
+          for (ParamIdx ArgNo : NonNull->args()) {
+            if (ArgNo.atAST() == ParamNo) {
               ComplainAboutNonnullParamOrCall(NonNull);
               return;
             }
@@ -12242,13 +12236,12 @@
   bool IsPointerAttr = Attr->getIsPointer();
 
   // Retrieve the argument representing the 'type_tag'.
-  if (Attr->getTypeTagIdx() >= ExprArgs.size()) {
-    // Add 1 to display the user's specified value.
+  if (Attr->typeTagIdx().atAST() >= ExprArgs.size()) {
     Diag(CallSiteLoc, diag::err_tag_index_out_of_range)
-        << 0 << Attr->getTypeTagIdx() + 1;
+        << 0 << Attr->typeTagIdx().atSrc();
     return;
   }
-  const Expr *TypeTagExpr = ExprArgs[Attr->getTypeTagIdx()];
+  const Expr *TypeTagExpr = ExprArgs[Attr->typeTagIdx().atAST()];
   bool FoundWrongKind;
   TypeTagData TypeInfo;
   if (!GetMatchingCType(ArgumentKind, TypeTagExpr, Context,
@@ -12262,13 +12255,12 @@
   }
 
   // Retrieve the argument representing the 'arg_idx'.
-  if (Attr->getArgumentIdx() >= ExprArgs.size()) {
-    // Add 1 to display the user's specified value.
+  if (Attr->argumentIdx().atAST() >= ExprArgs.size()) {
     Diag(CallSiteLoc, diag::err_tag_index_out_of_range)
-        << 1 << Attr->getArgumentIdx() + 1;
+        << 1 << Attr->argumentIdx().atSrc();
     return;
   }
-  const Expr *ArgumentExpr = ExprArgs[Attr->getArgumentIdx()];
+  const Expr *ArgumentExpr = ExprArgs[Attr->argumentIdx().atAST()];
   if (IsPointerAttr) {
     // Skip implicit cast of pointer to `void *' (as a function argument).
     if (const ImplicitCastExpr *ICE = dyn_cast<ImplicitCastExpr>(ArgumentExpr))
Index: lib/CodeGen/CGCall.cpp
===================================================================
--- lib/CodeGen/CGCall.cpp
+++ lib/CodeGen/CGCall.cpp
@@ -1846,10 +1846,9 @@
     HasOptnone = TargetDecl->hasAttr<OptimizeNoneAttr>();
     if (auto *AllocSize = TargetDecl->getAttr<AllocSizeAttr>()) {
       Optional<unsigned> NumElemsParam;
-      // alloc_size args are base-1, 0 means not present.
-      if (unsigned N = AllocSize->getNumElemsParam())
-        NumElemsParam = N - 1;
-      FuncAttrs.addAllocSizeAttr(AllocSize->getElemSizeParam() - 1,
+      if (AllocSize->numElemsParam().isValid())
+        NumElemsParam = AllocSize->numElemsParam().atLLVM();
+      FuncAttrs.addAllocSizeAttr(AllocSize->elemSizeParam().atLLVM(),
                                  NumElemsParam);
     }
   }
@@ -4373,7 +4372,7 @@
                               OffsetValue);
     } else if (const auto *AA = TargetDecl->getAttr<AllocAlignAttr>()) {
       llvm::Value *ParamVal =
-          CallArgs[AA->getParamIndex() - 1].RV.getScalarVal();
+          CallArgs[AA->paramIndex().atLLVM()].RV.getScalarVal();
       EmitAlignmentAssumption(Ret.getScalarVal(), ParamVal);
     }
   }
Index: lib/AST/ExprConstant.cpp
===================================================================
--- lib/AST/ExprConstant.cpp
+++ lib/AST/ExprConstant.cpp
@@ -5463,9 +5463,8 @@
                                             llvm::APInt &Result) {
   const AllocSizeAttr *AllocSize = getAllocSizeAttr(Call);
 
-  // alloc_size args are 1-indexed, 0 means not present.
-  assert(AllocSize && AllocSize->getElemSizeParam() != 0);
-  unsigned SizeArgNo = AllocSize->getElemSizeParam() - 1;
+  assert(AllocSize && AllocSize->elemSizeParam().isValid());
+  unsigned SizeArgNo = AllocSize->elemSizeParam().atAST();
   unsigned BitsInSizeT = Ctx.getTypeSize(Ctx.getSizeType());
   if (Call->getNumArgs() <= SizeArgNo)
     return false;
@@ -5483,14 +5482,13 @@
   if (!EvaluateAsSizeT(Call->getArg(SizeArgNo), SizeOfElem))
     return false;
 
-  if (!AllocSize->getNumElemsParam()) {
+  if (!AllocSize->numElemsParam().isValid()) {
     Result = std::move(SizeOfElem);
     return true;
   }
 
   APSInt NumberOfElems;
-  // Argument numbers start at 1
-  unsigned NumArgNo = AllocSize->getNumElemsParam() - 1;
+  unsigned NumArgNo = AllocSize->numElemsParam().atAST();
   if (!EvaluateAsSizeT(Call->getArg(NumArgNo), NumberOfElems))
     return false;
 
Index: include/clang/Basic/Attr.td
===================================================================
--- include/clang/Basic/Attr.td
+++ include/clang/Basic/Attr.td
@@ -166,6 +166,25 @@
 class VariadicExprArgument<string name> : Argument<name, 1>;
 class VariadicStringArgument<string name> : Argument<name, 1>;
 
+// Like VariadicUnsignedArgument except values represent function parameter
+// indices and are accessed via ParamIdxItr.
+//
+// The attribute definition statically declares the value of AllowsThis to
+// indicate whether the C++ implicit this parameter is allowed.  Users that
+// construct attributes from the source code use this information when
+// validating parameter indices.
+class VariadicParamIdxArgument<string name, bit allowsThis>
+    : VariadicUnsignedArgument<name> {
+  bit AllowsThis = allowsThis;
+}
+
+// Like VariadicParamIdxArgument but for a single function parameter index
+// accessed via a ParamIdx.
+class ParamIdxArgument<string name, bit allowsThis, bit opt = 0>
+    : Argument<name, opt> {
+  bit AllowsThis = allowsThis;
+}
+
 // A version of the form major.minor[.subminor].
 class VersionArgument<string name, bit opt = 0> : Argument<name, opt>;
 
@@ -611,6 +630,12 @@
 def XRayLogArgs : InheritableAttr {
   let Spellings = [Clang<"xray_log_args", 1>];
   let Subjects = SubjectList<[Function, ObjCMethod]>;
+  // This argument is a count not an index, so it has the same encoding (base
+  // 1 including C++ implicit this parameter) at the source and LLVM levels of
+  // representation, so ParamIdxArgument is inappropriate.  It is never used
+  // at the AST level of representation, so it never needs to be adjusted not
+  // to include any C++ implicit this parameter.  Thus, we just store it and
+  // use it as an unsigned that never needs adjustment.
   let Args = [UnsignedArgument<"ArgumentCount">];
   let Documentation = [XRayDocs];
 }
@@ -1018,7 +1043,8 @@
 def AllocSize : InheritableAttr {
   let Spellings = [GCC<"alloc_size">];
   let Subjects = SubjectList<[Function]>;
-  let Args = [IntArgument<"ElemSizeParam">, IntArgument<"NumElemsParam", 1>];
+  let Args = [ParamIdxArgument<"ElemSizeParam", /*AllowsThis*/ 0>,
+              ParamIdxArgument<"NumElemsParam", /*AllowsThis*/ 0, /*opt*/ 1>];
   let TemplateDependent = 1;
   let Documentation = [AllocSizeDocs];
 }
@@ -1105,7 +1131,7 @@
 
 def FormatArg : InheritableAttr {
   let Spellings = [GCC<"format_arg">];
-  let Args = [IntArgument<"FormatIdx">];
+  let Args = [ParamIdxArgument<"FormatIdx", /*AllowsThis*/ 0>];
   let Subjects = SubjectList<[ObjCMethod, HasFunctionProto]>;
   let Documentation = [Undocumented];
 }
@@ -1385,16 +1411,15 @@
   let Spellings = [GCC<"nonnull">];
   let Subjects = SubjectList<[ObjCMethod, HasFunctionProto, ParmVar], WarnDiag,
                              "functions, methods, and parameters">;
-  let Args = [VariadicUnsignedArgument<"Args">];
-  let AdditionalMembers =
-[{bool isNonNull(unsigned idx) const {
-    if (!args_size())
-      return true;
-    for (const auto &V : args())
-      if (V == idx)
+  let Args = [VariadicParamIdxArgument<"Args", /*AllowsThis*/ 0>];
+  let AdditionalMembers = [{
+    bool isNonNull(unsigned IdxAtAST) const {
+      ParamIdxAtASTItr Begin = args_begin(), End = args_end();
+      if (Begin == End)
         return true;
-    return false;
-  } }];
+      return End != std::find(Begin, End, IdxAtAST);
+    }
+  }];
   // FIXME: We should merge duplicates into a single nonnull attribute.
   let InheritEvenIfAlreadyPresent = 1;
   let Documentation = [NonNullDocs];
@@ -1452,7 +1477,7 @@
 def AllocAlign : InheritableAttr {
   let Spellings = [GCC<"alloc_align">];
   let Subjects = SubjectList<[HasFunctionProto]>;
-  let Args = [IntArgument<"ParamIndex">];
+  let Args = [ParamIdxArgument<"ParamIndex", /*AllowsThis*/ 0>];
   let Documentation = [AllocAlignDocs];
 }
 
@@ -1661,7 +1686,8 @@
              Returns;
     }
   }];
-  let Args = [IdentifierArgument<"Module">, VariadicUnsignedArgument<"Args">];
+  let Args = [IdentifierArgument<"Module">,
+              VariadicParamIdxArgument<"Args", /*AllowsThis*/ 0>];
   let Subjects = SubjectList<[HasFunctionProto]>;
   let Documentation = [Undocumented];
 }
@@ -2486,8 +2512,8 @@
                    Clang<"pointer_with_type_tag", 1>];
   let Subjects = SubjectList<[HasFunctionProto], ErrorDiag>;
   let Args = [IdentifierArgument<"ArgumentKind">,
-              UnsignedArgument<"ArgumentIdx">,
-              UnsignedArgument<"TypeTagIdx">,
+              ParamIdxArgument<"ArgumentIdx", /*AllowsThis*/0>,
+              ParamIdxArgument<"TypeTagIdx", /*AllowsThis*/0>,
               BoolArgument<"IsPointer", /*opt*/0, /*fake*/1>];
   let Documentation = [ArgumentWithTypeTagDocs, PointerWithTypeTagDocs];
 }
Index: include/clang/AST/Attr.h
===================================================================
--- include/clang/AST/Attr.h
+++ include/clang/AST/Attr.h
@@ -25,6 +25,7 @@
 #include "clang/Basic/SourceLocation.h"
 #include "clang/Basic/VersionTuple.h"
 #include "llvm/ADT/StringSwitch.h"
+#include "llvm/ADT/iterator.h"
 #include "llvm/Support/ErrorHandling.h"
 #include "llvm/Support/raw_ostream.h"
 #include <algorithm>
@@ -192,7 +193,247 @@
   static bool classof(const Attr *A) {
     return A->getKind() >= attr::FirstParameterABIAttr &&
            A->getKind() <= attr::LastParameterABIAttr;
-   }
+  }
+};
+
+/// A single parameter index whose accessors require each use to make explicit
+/// the parameter index encoding needed.
+///
+/// If, instead of a single parameter index, you have an array of parameter
+/// indices all for the same function, avoid using an array of \c ParamIdx.
+/// Instead, consider using an array of unsigned int iterated by a
+/// \c ParamIdxItr.  An array of \c ParamIdx unnecessarily duplicates the
+/// function's metadata across the array, but a \c ParamIdxItr stores that
+/// metadata once.
+class ParamIdx {
+  // Idx is exposed only via accessors that specify specific encodings.
+  unsigned Idx;
+  bool HasThis;
+  bool IsValid;
+
+public:
+  /// Construct an invalid parameter index (\c isValid returns false and
+  /// accessors fail an assert).
+  ParamIdx() : Idx(0), HasThis(false), IsValid(false) {}
+  /// \param Idx is the parameter index as it is normally specified in
+  /// attributes in the source: one-origin including any C++ implicit this
+  /// parameter.
+  ///
+  /// \param D is the declaration containing the parameters.  It is used to
+  /// determine if there is a C++ implicit this parameter.
+  ParamIdx(unsigned Idx, const Decl *D) : Idx(Idx), IsValid(true) {
+    if (const FunctionDecl *FD = dyn_cast<FunctionDecl>(D))
+      HasThis = FD->isCXXInstanceMember();
+    else
+      HasThis = false;
+  }
+  /// \param Idx is the parameter index as it is normally specified in
+  /// attributes in the source: one-origin including any C++ implicit this
+  /// parameter.
+  ///
+  /// \param HasThis specifies whether the function has a C++ implicit this
+  /// parameter.
+  ParamIdx(unsigned Idx, bool HasThis)
+      : Idx(Idx), HasThis(HasThis), IsValid(true) {}
+  ParamIdx &operator=(const ParamIdx &I) {
+    Idx = I.Idx;
+    HasThis = I.HasThis;
+    IsValid = I.IsValid;
+    return *this;
+  }
+  /// Is this parameter index valid?
+  bool isValid() const { return IsValid; }
+  /// Is this identical to another parameter index regardless of encoding?
+  bool operator==(const ParamIdx &I) const {
+    assert(isValid() && "ParamIdx must be valid");
+    return Idx == I.Idx && HasThis == I.HasThis;
+  }
+  /// Is this not identical to another parameter index for some encodings?
+  bool operator!=(const ParamIdx &I) const { return !(*this == I); }
+  /// Is there a C++ implicit this parameter?
+  bool hasThis() const {
+    assert(isValid() && "ParamIdx must be valid");
+    return HasThis;
+  }
+  /// Get the parameter index as it would normally be encoded for attributes at
+  /// the source level of representation: one-origin including any C++ implicit
+  /// this parameter.
+  ///
+  /// This encoding thus makes sense for diagnostics, pretty printing, and
+  /// constructing new attributes from a source-like specification.
+  unsigned atSrc() const {
+    assert(isValid() && "ParamIdx must be valid");
+    return Idx;
+  }
+  /// Get the parameter index as it would normally be encoded at the AST level
+  /// of representation: zero-origin not including any C++ implicit this
+  /// parameter.
+  ///
+  /// This is the encoding primarily used in Sema.  However, in diagnostics,
+  /// Sema uses \c atSrc instead.
+  unsigned atAST() const {
+    assert(isValid() && "ParamIdx must be valid");
+    return Idx - 1 - HasThis;
+  }
+  /// Get the parameter index as it would normally be encoded at the LLVM level
+  /// of representation: zero-origin including any C++ implicit this parameter.
+  ///
+  /// This is the encoding primarily used in CodeGen.
+  unsigned atLLVM() const {
+    assert(isValid() && "ParamIdx must be valid");
+    return Idx - 1;
+  }
+};
+
+/// CRTP base class for \c ParamIdxItr and related parameter index iterator
+/// classes.
+template <typename DerivedT, typename ValueT>
+class ParamIdxItrBase : public std::iterator<std::input_iterator_tag, ValueT,
+                                             ptrdiff_t, ValueT *, ValueT &> {
+  DerivedT &der() { return *static_cast<DerivedT *>(this); }
+  const DerivedT &der() const { return *static_cast<const DerivedT *>(this); }
+
+protected:
+  // \c Ptr is exposed publicly only via indirection operators, which return
+  // a \c ParamIdx that exposes specified encodings rather than the raw
+  // pointer or parameter index.
+  const unsigned *Ptr;
+  bool HasThis;
+  struct ValueProxy {
+    ValueT Idx;
+    explicit ValueProxy(const ValueT &Idx) : Idx(Idx) {}
+    ValueT *operator->() { return &Idx; }
+  };
+  ParamIdxItrBase() : Ptr(nullptr), HasThis(false) {}
+  ParamIdxItrBase(const unsigned *Ptr, const Decl *D)
+      : Ptr(Ptr), HasThis(ParamIdx(0, D).hasThis()) {}
+  ParamIdxItrBase(const unsigned *Ptr, bool HasThis)
+      : Ptr(Ptr), HasThis(HasThis) {}
+  ParamIdxItrBase(const ParamIdxItrBase &I) : Ptr(I.Ptr), HasThis(I.HasThis) {}
+
+public:
+  /// Is there a C++ implicit this parameter?
+  bool hasThis() const { return HasThis; }
+  DerivedT &operator=(const DerivedT &I) {
+    Ptr = I.Ptr;
+    HasThis = I.HasThis;
+    return der();
+  }
+  bool operator==(const DerivedT &I) const {
+    // A specific array of parameter indices in memory is for a specific
+    // function.  Thus, if Ptr is the same, HasThis must be the same.
+    assert(Ptr != I.Ptr || HasThis == I.HasThis);
+    return Ptr == I.Ptr;
+  }
+  bool operator!=(const DerivedT &I) const { return !(*this == I); }
+  bool operator<(const DerivedT &I) const { return Ptr < I.Ptr; }
+  bool operator>(const DerivedT &I) const { return Ptr > I.Ptr; }
+  bool operator<=(const DerivedT &I) const { return Ptr <= I.Ptr; }
+  bool operator>=(const DerivedT &I) const { return Ptr >= I.Ptr; }
+  DerivedT &operator+=(ptrdiff_t N) { Ptr += N; return der(); }
+  DerivedT &operator-=(ptrdiff_t N) { Ptr -= N; return der(); }
+  DerivedT &operator++() { return der() += 1; }
+  DerivedT &operator--() { return der() -= 1; }
+  DerivedT operator++(int) { DerivedT I(der()); ++der(); return I; }
+  DerivedT operator--(int) { DerivedT I(der()); --der(); return I; }
+  DerivedT operator+(ptrdiff_t N) const { DerivedT I(der()); return I += N; }
+  friend DerivedT operator+(ptrdiff_t N, const DerivedT &I) { return I + N; }
+  DerivedT operator-(ptrdiff_t N) const { DerivedT I(der()); return I -= N; }
+  ptrdiff_t operator-(const DerivedT &I) const { return Ptr - I.Ptr; }
+  ValueT operator[](ptrdiff_t N) const { return *(der() + N); }
+  ValueProxy operator->() const { return ValueProxy(*der()); }
+};
+
+class ParamIdxAtSrcItr;
+class ParamIdxAtASTItr;
+class ParamIdxAtLLVMItr;
+
+/// An iterator across an array of parameter indices, all for the same
+/// function.  The iterator exposes the parameter indices via accessors that
+/// require each use to make explicit the parameter index encoding needed.
+///
+/// The array stores the raw parameter indices.  A begin \c ParamIdxItr is
+/// normally used to store the address of that array plus relevant metadata
+/// related to the function.  After the begin \c ParamIdxItr is constructed,
+/// accessing the array directly is normally avoided.  Instead, a copy of the
+/// begin \c ParamIdxItr is normally adjusted and dereferenced to return a
+/// \c ParamIdx, which provides accessor functions that make the needed
+/// encoding of the referenced parameter index explicit at each use.
+///
+/// When it is more convenient to have a parameter index iterator whose
+/// deference operator always returns a specific encoding instead of a
+/// \c ParamIdx, a \c ParamIdxItr can be cast to a \c ParamIdxAtSrcItr,
+/// \c ParamIdxAtASTItr, or \c ParamIdxAtLLVMItr.
+///
+/// These parameter index iterator types do not qualify as C++ forward
+/// iterators (or thus random-access iterators) because (1) the indirection
+/// operator (*) has a value return type, (2) it and the member-access
+/// operator (->) return temporary values, and so (3) equal iterators do not
+/// dereference to references to the same value object.  In other respects,
+/// these iterators are random-access input iterators.
+///
+/// If, instead of an array, you have a single parameter index, consider
+/// instead using just \c ParamIdx, which can be more convenient and is more
+/// memory-efficient for that case.
+class ParamIdxItr : public ParamIdxItrBase<ParamIdxItr, ParamIdx> {
+  // These friends need to access ParamIdxItr's Ptr in their conversion
+  // constructors, but Ptr should not be publicly exposed.
+  friend ParamIdxAtSrcItr;
+  friend ParamIdxAtASTItr;
+  friend ParamIdxAtLLVMItr;
+
+public:
+  /// Construct an invalid parameter index iterator.
+  ParamIdxItr() : ParamIdxItrBase() {}
+  /// \param Ptr points to the array of parameter indices encoded as they are
+  /// normally specified in attributes in the source: one-origin including any
+  /// C++ implicit this parameter.  The same \c Ptr must not be stored in
+  /// multiple \c ParamIdxItr iterators for the sake of different functions
+  /// (and thus potentially different values of \c HasThis).
+  ///
+  /// \param D is the declaration containing the parameters.  It is used to
+  /// determine if there is a C++ implicit this parameter.
+  ParamIdxItr(const unsigned *Ptr, const Decl *D) : ParamIdxItrBase(Ptr, D) {}
+  /// \param Ptr points to an array of parameter indices stored as they are
+  /// normally specified in attributes in the source: one-origin including any
+  /// C++ implicit this parameter.  The same \c Ptr must not be stored in
+  /// multiple \c ParamIdxItr iterators for the sake of different functions
+  /// (and thus potentially different values of \c HasThis).
+  ///
+  /// \param HasThis specifies whether the function has a C++ implicit this
+  /// parameter.
+  ParamIdxItr(const unsigned *Ptr, bool HasThis)
+      : ParamIdxItrBase(Ptr, HasThis) {}
+  ParamIdxItr(const ParamIdxItr &I) : ParamIdxItrBase(I) {}
+  ParamIdx operator*() const { return ParamIdx(*Ptr, HasThis); }
+};
+
+/// See documentation for \c ParamIdxItr and for the \c atSrc accessor of
+/// \c ParamIdx.
+class ParamIdxAtSrcItr : public ParamIdxItrBase<ParamIdxAtSrcItr, unsigned> {
+public:
+  ParamIdxAtSrcItr(const ParamIdxItr &I) : ParamIdxItrBase(I.Ptr, I.HasThis) {}
+  ParamIdxAtSrcItr(const ParamIdxAtSrcItr &I) : ParamIdxItrBase(I) {}
+  unsigned operator*() const { return ParamIdx(*Ptr, HasThis).atSrc(); }
+};
+
+/// See documentation for \c ParamIdxItr and for the \c atAST accessor of
+/// \c ParamIdx.
+class ParamIdxAtASTItr : public ParamIdxItrBase<ParamIdxAtASTItr, unsigned> {
+public:
+  ParamIdxAtASTItr(const ParamIdxItr &I) : ParamIdxItrBase(I.Ptr, I.HasThis) {}
+  ParamIdxAtASTItr(const ParamIdxAtASTItr &I) : ParamIdxItrBase(I) {}
+  unsigned operator*() const { return ParamIdx(*Ptr, HasThis).atAST(); }
+};
+
+/// See documentation for \c ParamIdxItr and for the \c atAST accessor of
+/// \c ParamIdx.
+class ParamIdxAtLLVMItr : public ParamIdxItrBase<ParamIdxAtLLVMItr, unsigned> {
+public:
+  ParamIdxAtLLVMItr(const ParamIdxItr &I)
+      : ParamIdxItrBase(I.Ptr, I.HasThis) {}
+  ParamIdxAtLLVMItr(const ParamIdxAtLLVMItr &I) : ParamIdxItrBase(I) {}
+  unsigned operator*() const { return ParamIdx(*Ptr, HasThis).atAST(); }
 };
 
 #include "clang/AST/Attrs.inc"
_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

Reply via email to