[Attr] Fix parameter indexing for several attributes

The patch fixes a number of bugs related to parameter indexing in
attributes:

* Parameter indices in some attributes (argument_with_type_tag,
  pointer_with_type_tag, nonnull, ownership_takes, ownership_holds,
  and ownership_returns) are specified in source as one-origin
  including any C++ implicit this parameter, were stored as
  zero-origin excluding any this parameter, and were erroneously
  printing (-ast-print) and confusingly dumping (-ast-dump) as the
  stored values.

* For alloc_size, the C++ implicit this parameter was not subtracted
  correctly in Sema, leading to assert failures or to silent failures
  of __builtin_object_size to compute a value.

* For argument_with_type_tag, pointer_with_type_tag, and
  ownership_returns, the C++ implicit this parameter was not added
  back to parameter indices in some diagnostics.

This patch fixes the above bugs and aims to prevent similar bugs in
the future by introducing careful mechanisms for handling parameter
indices in attributes.  ParamIdx stores a parameter index and is
designed to hide the stored encoding while providing accessors that
require each use (such as printing) to make explicit the encoding that
is needed.  Attribute declarations declare parameter index arguments
as [Variadic]ParamIdxArgument, which are exposed as ParamIdx[*].  This
patch rewrites all attribute arguments that are processed by
checkFunctionOrMethodParameterIndex in SemaDeclAttr.cpp to be declared
as [Variadic]ParamIdxArgument.  The only exception is xray_log_args's
argument, which is encoded as a count not an index.

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

git-svn-id: https://llvm.org/svn/llvm-project/cfe/trunk@326602 91177308-0d34-0410-b5e6-96231b3b80d8

1 files changed, 136 insertions, 3 deletions

diff --git a/utils/TableGen/ClangAttrEmitter.cpp b/utils/TableGen/ClangAttrEmitter.cpp
index 5c786f5341..263dcae3b3 100644
--- a/utils/TableGen/ClangAttrEmitter.cpp
+++ b/utils/TableGen/ClangAttrEmitter.cpp
@@ -302,9 +302,6 @@ namespace {
     std::string getIsOmitted() const override {
       if (type == "IdentifierInfo *")
         return "!get" + getUpperName().str() + "()";
-      // FIXME: Do this declaratively in Attr.td.
-      if (getAttrName() == "AllocSize")
-        return "0 == get" + getUpperName().str() + "()";
       return "false";
     }
 
@@ -748,6 +745,138 @@ namespace {
     }
   };
 
+  class VariadicParamIdxArgument : public VariadicArgument {
+  public:
+    VariadicParamIdxArgument(const Record &Arg, StringRef Attr)
+        : VariadicArgument(Arg, Attr, "ParamIdx") {}
+
+  public:
+    void writeCtorBody(raw_ostream &OS) const override {
+      VariadicArgument::writeCtorBody(OS);
+      OS << "    #ifndef NDEBUG\n"
+         << "    if (" << getLowerName() << "_size()) {\n"
+         << "      bool HasThis = " << getLowerName()
+         << "_begin()->hasThis();\n"
+         << "      for (const auto Idx : " << getLowerName() << "()) {\n"
+         << "        assert(Idx.isValid() && \"ParamIdx must be valid\");\n"
+         << "        assert(HasThis == Idx.hasThis() && "
+         << "\"HasThis must be consistent\");\n"
+         << "      }\n"
+         << "    }\n"
+         << "    #endif\n";
+    }
+
+    void writePCHReadDecls(raw_ostream &OS) const override {
+      OS << "    unsigned " << getUpperName() << "Size = Record.readInt();\n";
+      OS << "    bool " << getUpperName() << "HasThis = " << getUpperName()
+         << "Size ? Record.readInt() : false;\n";
+      OS << "    SmallVector<ParamIdx, 4> " << getUpperName() << ";\n"
+         << "    " << getUpperName() << ".reserve(" << getUpperName()
+         << "Size);\n"
+         << "    for (unsigned i = 0; i != " << getUpperName()
+         << "Size; ++i) {\n"
+         << "      " << getUpperName()
+         << ".push_back(ParamIdx(Record.readInt(), " << getUpperName()
+         << "HasThis));\n"
+         << "    }\n";
+    }
+
+    void writePCHReadArgs(raw_ostream &OS) const override {
+      OS << getUpperName() << ".data(), " << getUpperName() << "Size";
+    }
+
+    void writePCHWrite(raw_ostream &OS) const override {
+      OS << "    Record.push_back(SA->" << getLowerName() << "_size());\n";
+      OS << "    if (SA->" << getLowerName() << "_size())\n"
+         << "      Record.push_back(SA->" << getLowerName()
+         << "_begin()->hasThis());\n";
+      OS << "    for (auto Idx : SA->" << getLowerName() << "())\n"
+         << "      Record.push_back(Idx.getSourceIndex());\n";
+    }
+
+    void writeValueImpl(raw_ostream &OS) const override {
+      OS << "    OS << Val.getSourceIndex();\n";
+    }
+
+    void writeDump(raw_ostream &OS) const override {
+      OS << "    for (auto Idx : SA->" << getLowerName() << "())\n";
+      OS << "      OS << \" \" << Idx.getSourceIndex();\n";
+    }
+  };
+
+  class ParamIdxArgument : public Argument {
+    std::string IdxName;
+
+  public:
+    ParamIdxArgument(const Record &Arg, StringRef Attr)
+        : Argument(Arg, Attr), IdxName(getUpperName()) {}
+
+    void writeDeclarations(raw_ostream &OS) const override {
+      OS << "ParamIdx " << IdxName << ";\n";
+    }
+
+    void writeAccessors(raw_ostream &OS) const override {
+      OS << "\n"
+         << "  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()
+         << "().getSourceIndex() : 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 << ".getSourceIndex() << \"";
+    }
+
+    void writeDump(raw_ostream &OS) const override {
+      if (isOptional())
+        OS << "    if (SA->" << getLowerName() << "().isValid())\n  ";
+      OS << "    OS << \" \" << SA->" << getLowerName()
+         << "().getSourceIndex();\n";
+    }
+  };
+
   // Unique the enums, but maintain the original declaration ordering.
   std::vector<StringRef>
   uniqueEnumsInOrder(const std::vector<StringRef> &enums) {
@@ -1247,6 +1376,10 @@ createArgument(const Record &Arg, StringRef Attr,
     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);