steffenlarsen updated this revision to Diff 401580.
steffenlarsen added a comment.

Applied suggestions.


CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D114439/new/

https://reviews.llvm.org/D114439

Files:
  clang/include/clang/Basic/Attr.td
  clang/include/clang/Basic/DiagnosticParseKinds.td
  clang/include/clang/Parse/Parser.h
  clang/lib/Parse/ParseDecl.cpp
  clang/lib/Parse/ParseExpr.cpp
  clang/lib/Sema/SemaTemplateInstantiateDecl.cpp
  clang/test/Parser/cxx0x-attributes.cpp
  clang/test/SemaTemplate/attributes.cpp
  clang/utils/TableGen/ClangAttrEmitter.cpp

Index: clang/utils/TableGen/ClangAttrEmitter.cpp
===================================================================
--- clang/utils/TableGen/ClangAttrEmitter.cpp
+++ clang/utils/TableGen/ClangAttrEmitter.cpp
@@ -1164,10 +1164,12 @@
   };
 
   class VariadicExprArgument : public VariadicArgument {
+    bool AllowPack = false;
+
   public:
     VariadicExprArgument(const Record &Arg, StringRef Attr)
-      : VariadicArgument(Arg, Attr, "Expr *")
-    {}
+        : VariadicArgument(Arg, Attr, "Expr *"),
+          AllowPack(Arg.getValueAsBit("AllowPack")) {}
 
     void writeASTVisitorTraversal(raw_ostream &OS) const override {
       OS << "  {\n";
@@ -2264,6 +2266,47 @@
   OS << "#endif // CLANG_ATTR_THIS_ISA_IDENTIFIER_ARG_LIST\n\n";
 }
 
+static void emitClangAttrNumArgs(RecordKeeper &Records, raw_ostream &OS) {
+  OS << "#if defined(CLANG_ATTR_NUM_ARGS)\n";
+  std::vector<Record *> Attrs = Records.getAllDerivedDefinitions("Attr");
+  for (const auto *A : Attrs) {
+    std::vector<Record *> Args = A->getValueAsListOfDefs("Args");
+    forEachUniqueSpelling(*A, [&](const FlattenedSpelling &S) {
+      OS << ".Case(\"" << S.name() << "\", " << Args.size() << ")\n";
+    });
+  }
+  OS << "#endif // CLANG_ATTR_NUM_ARGS\n\n";
+}
+
+static bool argSupportsParmPack(const Record *Arg) {
+  return !Arg->getSuperClasses().empty() &&
+         llvm::StringSwitch<bool>(
+             Arg->getSuperClasses().back().first->getName())
+             .Case("VariadicExprArgument", true)
+             .Default(false) &&
+         Arg->getValueAsBit("AllowPack");
+}
+
+static void emitClangAttrLastArgParmPackSupport(RecordKeeper &Records,
+                                                raw_ostream &OS) {
+  OS << "#if defined(CLANG_ATTR_LAST_ARG_PARM_PACK_SUPPORT)\n";
+  std::vector<Record *> Attrs = Records.getAllDerivedDefinitions("Attr");
+  for (const auto *A : Attrs) {
+    std::vector<Record *> Args = A->getValueAsListOfDefs("Args");
+    bool LastArgSupportsParmPack =
+        !Args.empty() && argSupportsParmPack(Args.back());
+    forEachUniqueSpelling(*A, [&](const FlattenedSpelling &S) {
+      OS << ".Case(\"" << S.name() << "\", " << LastArgSupportsParmPack
+         << ")\n";
+    });
+  }
+  OS << "#endif // CLANG_ATTR_LAST_ARG_PARM_PACK_SUPPORT\n\n";
+}
+
+static bool isArgVariadic(const Record &R, StringRef AttrName) {
+  return createArgument(R, AttrName)->isVariadic();
+}
+
 static void emitAttributes(RecordKeeper &Records, raw_ostream &OS,
                            bool Header) {
   std::vector<Record*> Attrs = Records.getAllDerivedDefinitions("Attr");
@@ -2320,6 +2363,18 @@
     std::vector<std::unique_ptr<Argument>> Args;
     Args.reserve(ArgRecords.size());
 
+    if (!ArgRecords.empty()) {
+      const bool LastArgSupportsPack = argSupportsParmPack(ArgRecords.back());
+      for (size_t I = 0; I < ArgRecords.size() - 1; ++I) {
+        assert((!LastArgSupportsPack ||
+                !isArgVariadic(*ArgRecords[I], R.getName())) &&
+               "Attributes supporting packs can only have the last "
+               "argument as variadic");
+        assert(!argSupportsParmPack(ArgRecords[I]) &&
+               "Only the last argument can allow packs");
+      }
+    }
+
     bool HasOptArg = false;
     bool HasFakeArg = false;
     for (const auto *ArgRecord : ArgRecords) {
@@ -3382,10 +3437,6 @@
   }
 }
 
-static bool isArgVariadic(const Record &R, StringRef AttrName) {
-  return createArgument(R, AttrName)->isVariadic();
-}
-
 static void emitArgInfo(const Record &R, raw_ostream &OS) {
   // This function will count the number of arguments specified for the
   // attribute and emit the number of required arguments followed by the
@@ -4219,6 +4270,8 @@
   emitClangAttrIdentifierArgList(Records, OS);
   emitClangAttrVariadicIdentifierArgList(Records, OS);
   emitClangAttrThisIsaIdentifierArgList(Records, OS);
+  emitClangAttrNumArgs(Records, OS);
+  emitClangAttrLastArgParmPackSupport(Records, OS);
   emitClangAttrTypeArgList(Records, OS);
   emitClangAttrLateParsedList(Records, OS);
 }
Index: clang/test/SemaTemplate/attributes.cpp
===================================================================
--- clang/test/SemaTemplate/attributes.cpp
+++ clang/test/SemaTemplate/attributes.cpp
@@ -64,6 +64,23 @@
 template<typename T> [[clang::annotate("ANNOTATE_FOO"), clang::annotate("ANNOTATE_BAR")]] void HasAnnotations();
 void UseAnnotations() { HasAnnotations<int>(); }
 
+// CHECK:      FunctionTemplateDecl {{.*}} HasPackAnnotations
+// CHECK:        AnnotateAttr {{.*}} "ANNOTATE_BAZ"
+// CHECK:      FunctionDecl {{.*}} HasPackAnnotations
+// CHECK:        TemplateArgument{{.*}} pack
+// CHECK-NEXT:     TemplateArgument{{.*}} integral 1
+// CHECK-NEXT:     TemplateArgument{{.*}} integral 2
+// CHECK-NEXT:     TemplateArgument{{.*}} integral 3
+// CHECK:        AnnotateAttr {{.*}} "ANNOTATE_BAZ"
+// CHECK:          ConstantExpr {{.*}} 'int'
+// CHECK-NEXT:       value: Int 1
+// CHECK:          ConstantExpr {{.*}} 'int'
+// CHECK-NEXT:       value: Int 2
+// CHECK:          ConstantExpr {{.*}} 'int'
+// CHECK-NEXT:       value: Int 3
+template <int... Is> [[clang::annotate("ANNOTATE_BAZ", Is...)]] void HasPackAnnotations();
+void UsePackAnnotations() { HasPackAnnotations<1, 2, 3>(); }
+
 namespace preferred_name {
   int x [[clang::preferred_name("frank")]]; // expected-error {{expected a type}}
   int y [[clang::preferred_name(int)]]; // expected-warning {{'preferred_name' attribute only applies to class templates}}
Index: clang/test/Parser/cxx0x-attributes.cpp
===================================================================
--- clang/test/Parser/cxx0x-attributes.cpp
+++ clang/test/Parser/cxx0x-attributes.cpp
@@ -259,10 +259,24 @@
   [[]] return;
 }
 
-template<typename...Ts> void variadic() {
+template <typename... Ts> void variadic() {
   void bar [[noreturn...]] (); // expected-error {{attribute 'noreturn' cannot be used as an attribute pack}}
 }
 
+template <int... Is> void variadic_nttp() {
+  void bar [[noreturn...]] ();                        // expected-error {{attribute 'noreturn' cannot be used as an attribute pack}}
+  void baz [[clang::no_sanitize(Is...)]] ();          // expected-error {{attribute 'no_sanitize' does not support argument pack expansion}}
+  void boz [[clang::annotate(Is...)]] ();             // expected-error {{pack expansion in 'annotate' may only be applied to the last argument}}
+  void bor [[clang::annotate("A", "V" ...)]] ();      // expected-error {{pack expansion does not contain any unexpanded parameter packs}}
+  void bir [[clang::annotate("B", {1, 2, 3, 4})]] (); // expected-error {{'annotate' attribute requires parameter 1 to be a constant expression}} expected-note {{subexpression not valid in a constant expression}}
+  void boo [[unknown::foo(Is...)]] ();                // expected-warning {{unknown attribute 'foo' ignored}}
+  void faz [[clang::annotate("C", (Is + ...))]] ();   // expected-warning {{pack fold expression is a C++17 extension}}
+  void far [[clang::annotate("D", Is...)]] ();
+  void foz [[clang::annotate("E", 1, 2, 3, Is...)]] ();
+  void fiz [[clang::annotate("F", Is..., 1, 2, 3)]] ();
+  void fir [[clang::annotate("G", 1, Is..., 2, 3)]] ();
+}
+
 // Expression tests
 void bar () {
   // FIXME: GCC accepts [[gnu::noreturn]] on a lambda, even though it appertains
Index: clang/lib/Sema/SemaTemplateInstantiateDecl.cpp
===================================================================
--- clang/lib/Sema/SemaTemplateInstantiateDecl.cpp
+++ clang/lib/Sema/SemaTemplateInstantiateDecl.cpp
@@ -189,13 +189,9 @@
   EnterExpressionEvaluationContext Unevaluated(
       S, Sema::ExpressionEvaluationContext::ConstantEvaluated);
   SmallVector<Expr *, 4> Args;
-  Args.reserve(Attr->args_size());
-  for (auto *E : Attr->args()) {
-    ExprResult Result = S.SubstExpr(E, TemplateArgs);
-    if (!Result.isUsable())
-      return;
-    Args.push_back(Result.get());
-  }
+  if (S.SubstExprs(ArrayRef<Expr *>(Attr->args_begin(), Attr->args_end()),
+                   /*IsCall=*/false, TemplateArgs, Args))
+    return;
   S.AddAnnotationAttr(New, *Attr, Attr->getAnnotation(), Args);
 }
 
Index: clang/lib/Parse/ParseExpr.cpp
===================================================================
--- clang/lib/Parse/ParseExpr.cpp
+++ clang/lib/Parse/ParseExpr.cpp
@@ -3356,7 +3356,9 @@
 /// \endverbatim
 bool Parser::ParseExpressionList(SmallVectorImpl<Expr *> &Exprs,
                                  SmallVectorImpl<SourceLocation> &CommaLocs,
-                                 llvm::function_ref<void()> ExpressionStarts) {
+                                 llvm::function_ref<void()> ExpressionStarts,
+                                 bool FailImmediatelyOnInvalidExpr,
+                                 bool EarlyTypoCorrection) {
   bool SawError = false;
   while (true) {
     if (ExpressionStarts)
@@ -3369,6 +3371,9 @@
     } else
       Expr = ParseAssignmentExpression();
 
+    if (EarlyTypoCorrection)
+      Expr = Actions.CorrectDelayedTyposInExpr(Expr);
+
     if (Tok.is(tok::ellipsis))
       Expr = Actions.ActOnPackExpansion(Expr.get(), ConsumeToken());
     else if (Tok.is(tok::code_completion)) {
@@ -3382,8 +3387,10 @@
       break;
     }
     if (Expr.isInvalid()) {
-      SkipUntil(tok::comma, tok::r_paren, StopBeforeMatch);
       SawError = true;
+      if (FailImmediatelyOnInvalidExpr)
+        break;
+      SkipUntil(tok::comma, tok::r_paren, StopBeforeMatch);
     } else {
       Exprs.push_back(Expr.get());
     }
Index: clang/lib/Parse/ParseDecl.cpp
===================================================================
--- clang/lib/Parse/ParseDecl.cpp
+++ clang/lib/Parse/ParseDecl.cpp
@@ -300,6 +300,25 @@
 #undef CLANG_ATTR_THIS_ISA_IDENTIFIER_ARG_LIST
 }
 
+/// Determine how many arguments an attribute has. Variadic arguments counts as
+/// a single argument.
+static size_t attributeNumberOfArguments(const IdentifierInfo &II) {
+#define CLANG_ATTR_NUM_ARGS
+  return llvm::StringSwitch<size_t>(normalizeAttrName(II.getName()))
+#include "clang/Parse/AttrParserStringSwitches.inc"
+      .Default(0);
+#undef CLANG_ATTR_NUM_ARGS
+}
+
+/// Determine if the last attribute argument supports pack expansion.
+static bool attributeLastArgParmPackSupport(const IdentifierInfo &II) {
+#define CLANG_ATTR_LAST_ARG_PARM_PACK_SUPPORT
+  return llvm::StringSwitch<bool>(normalizeAttrName(II.getName()))
+#include "clang/Parse/AttrParserStringSwitches.inc"
+      .Default(false);
+#undef CLANG_ATTR_LAST_ARG_PARM_PACK_SUPPORT
+}
+
 /// Determine whether the given attribute parses a type argument.
 static bool attributeIsTypeArgAttr(const IdentifierInfo &II) {
 #define CLANG_ATTR_TYPE_ARG_LIST
@@ -366,6 +385,8 @@
 
   bool ChangeKWThisToIdent = attributeTreatsKeywordThisAsIdentifier(*AttrName);
   bool AttributeIsTypeArgAttr = attributeIsTypeArgAttr(*AttrName);
+  bool AttributeHasVariadicIdentifierArg =
+      attributeHasVariadicIdentifierArg(*AttrName);
 
   // Interpret "kw_this" as an identifier if the attributed requests it.
   if (ChangeKWThisToIdent && Tok.is(tok::kw_this))
@@ -374,8 +395,8 @@
   ArgsVector ArgExprs;
   if (Tok.is(tok::identifier)) {
     // If this attribute wants an 'identifier' argument, make it so.
-    bool IsIdentifierArg = attributeHasIdentifierArg(*AttrName) ||
-                           attributeHasVariadicIdentifierArg(*AttrName);
+    bool IsIdentifierArg = AttributeHasVariadicIdentifierArg ||
+                           attributeHasIdentifierArg(*AttrName);
     ParsedAttr::Kind AttrKind =
         ParsedAttr::getParsedKind(AttrName, ScopeName, Syntax);
 
@@ -397,42 +418,94 @@
     if (!ArgExprs.empty())
       ConsumeToken();
 
-    // Parse the non-empty comma-separated list of expressions.
-    do {
-      // Interpret "kw_this" as an identifier if the attributed requests it.
-      if (ChangeKWThisToIdent && Tok.is(tok::kw_this))
-        Tok.setKind(tok::identifier);
+    if (AttributeIsTypeArgAttr) {
+      // FIXME: Multiple type arguments are not implemented.
+      TypeResult T = ParseTypeName();
+      if (T.isInvalid()) {
+        SkipUntil(tok::r_paren, StopAtSemi);
+        return 0;
+      }
+      if (T.isUsable())
+        TheParsedType = T.get();
+    } else if (AttributeHasVariadicIdentifierArg) {
+      // Parse variadic identifier arg. This can either consume identifiers or
+      // expressions.
+      // FIXME: Variadic identifier args do not currently support parameter
+      //        packs.
+      do {
+        // Interpret "kw_this" as an identifier if the attributed requests it.
+        if (ChangeKWThisToIdent && Tok.is(tok::kw_this))
+          Tok.setKind(tok::identifier);
+
+        ExprResult ArgExpr;
+        if (Tok.is(tok::identifier)) {
+          ArgExprs.push_back(ParseIdentifierLoc());
+        } else {
+          bool Uneval = attributeParsedArgsUnevaluated(*AttrName);
+          EnterExpressionEvaluationContext Unevaluated(
+              Actions,
+              Uneval ? Sema::ExpressionEvaluationContext::Unevaluated
+                     : Sema::ExpressionEvaluationContext::ConstantEvaluated);
 
-      ExprResult ArgExpr;
-      if (AttributeIsTypeArgAttr) {
-        TypeResult T = ParseTypeName();
-        if (T.isInvalid()) {
+          ExprResult ArgExpr(
+              Actions.CorrectDelayedTyposInExpr(ParseAssignmentExpression()));
+
+          if (ArgExpr.isInvalid()) {
+            SkipUntil(tok::r_paren, StopAtSemi);
+            return 0;
+          }
+          ArgExprs.push_back(ArgExpr.get());
+        }
+        // Eat the comma, move to the next argument
+      } while (TryConsumeToken(tok::comma));
+    } else {
+      bool Uneval = attributeParsedArgsUnevaluated(*AttrName);
+      EnterExpressionEvaluationContext Unevaluated(
+          Actions, Uneval
+                       ? Sema::ExpressionEvaluationContext::Unevaluated
+                       : Sema::ExpressionEvaluationContext::ConstantEvaluated);
+
+      // General case. Parse all available expressions.
+      CommaLocsTy CommaLocs;
+      ExprVector ParsedExprs;
+      if (ParseExpressionList(ParsedExprs, CommaLocs,
+                              llvm::function_ref<void()>(),
+                              /*FailImmediatelyOnInvalidExpr=*/true,
+                              /*EarlyTypoCorrection=*/true)) {
+        SkipUntil(tok::r_paren, StopAtSemi);
+        return 0;
+      }
+
+      // Pack expansion must currently be explicitly supported by an attribute.
+      // Additionally, this is only currently allowed in a variadic argument at
+      // the end of the attribute.
+      for (size_t I = 0; I < ParsedExprs.size(); ++I) {
+        if (!isa<PackExpansionExpr>(ParsedExprs[I]))
+          continue;
+
+        // Attribute must explicitly support parameter pack in its last
+        // argument.
+        if (!attributeLastArgParmPackSupport(*AttrName)) {
+          Diag(Tok.getLocation(),
+               diag::err_attribute_argument_parm_pack_not_supported)
+              << AttrName;
           SkipUntil(tok::r_paren, StopAtSemi);
           return 0;
         }
-        if (T.isUsable())
-          TheParsedType = T.get();
-        break; // FIXME: Multiple type arguments are not implemented.
-      } else if (Tok.is(tok::identifier) &&
-                 attributeHasVariadicIdentifierArg(*AttrName)) {
-        ArgExprs.push_back(ParseIdentifierLoc());
-      } else {
-        bool Uneval = attributeParsedArgsUnevaluated(*AttrName);
-        EnterExpressionEvaluationContext Unevaluated(
-            Actions,
-            Uneval ? Sema::ExpressionEvaluationContext::Unevaluated
-                   : Sema::ExpressionEvaluationContext::ConstantEvaluated);
-
-        ExprResult ArgExpr(
-            Actions.CorrectDelayedTyposInExpr(ParseAssignmentExpression()));
-        if (ArgExpr.isInvalid()) {
+
+        // Pack expansion is only allowed in the variadic argument at the end.
+        const size_t AttrNumArgs = attributeNumberOfArguments(*AttrName);
+        if (ArgExprs.size() + I + 1 < AttrNumArgs) {
+          Diag(Tok.getLocation(),
+               diag::err_attribute_parm_pack_last_argument_only)
+              << AttrName;
           SkipUntil(tok::r_paren, StopAtSemi);
           return 0;
         }
-        ArgExprs.push_back(ArgExpr.get());
       }
-      // Eat the comma, move to the next argument
-    } while (TryConsumeToken(tok::comma));
+
+      ArgExprs.insert(ArgExprs.end(), ParsedExprs.begin(), ParsedExprs.end());
+    }
   }
 
   SourceLocation RParen = Tok.getLocation();
Index: clang/include/clang/Parse/Parser.h
===================================================================
--- clang/include/clang/Parse/Parser.h
+++ clang/include/clang/Parse/Parser.h
@@ -1814,7 +1814,9 @@
   bool ParseExpressionList(SmallVectorImpl<Expr *> &Exprs,
                            SmallVectorImpl<SourceLocation> &CommaLocs,
                            llvm::function_ref<void()> ExpressionStarts =
-                               llvm::function_ref<void()>());
+                               llvm::function_ref<void()>(),
+                           bool FailImmediatelyOnInvalidExpr = false,
+                           bool EarlyTypoCorrection = false);
 
   /// ParseSimpleExpressionList - A simple comma-separated list of expressions,
   /// used for misc language extensions.
Index: clang/include/clang/Basic/DiagnosticParseKinds.td
===================================================================
--- clang/include/clang/Basic/DiagnosticParseKinds.td
+++ clang/include/clang/Basic/DiagnosticParseKinds.td
@@ -719,6 +719,10 @@
 def err_l_square_l_square_not_attribute : Error<
   "C++11 only allows consecutive left square brackets when "
   "introducing an attribute">;
+def err_attribute_argument_parm_pack_not_supported : Error<
+  "attribute %0 does not support argument pack expansion">;
+def err_attribute_parm_pack_last_argument_only : Error<
+  "pack expansion in %0 may only be applied to the last argument">;
 def err_ms_declspec_type : Error<
   "__declspec attributes must be an identifier or string literal">;
 def err_ms_property_no_getter_or_putter : Error<
Index: clang/include/clang/Basic/Attr.td
===================================================================
--- clang/include/clang/Basic/Attr.td
+++ clang/include/clang/Basic/Attr.td
@@ -205,7 +205,12 @@
 class TypeArgument<string name, bit opt = 0> : Argument<name, opt>;
 class UnsignedArgument<string name, bit opt = 0> : Argument<name, opt>;
 class VariadicUnsignedArgument<string name> : Argument<name, 1>;
-class VariadicExprArgument<string name> : Argument<name, 1>;
+class VariadicExprArgument<string name, bit packs = 0> : Argument<name, 1> {
+  // Set to true if variadic expression argument supports parameter pack
+  // expansion. Setting this to true is only allowed if the argument is the last
+  // argument of the attribute.
+  bit AllowPack = packs;
+}
 class VariadicStringArgument<string name> : Argument<name, 1>;
 class VariadicIdentifierArgument<string name> : Argument<name, 1>;
 
@@ -770,7 +775,8 @@
 
 def Annotate : InheritableParamAttr {
   let Spellings = [Clang<"annotate">];
-  let Args = [StringArgument<"Annotation">, VariadicExprArgument<"Args">];
+  let Args = [StringArgument<"Annotation">,
+              VariadicExprArgument<"Args", /*AllowPack*/1>];
   // Ensure that the annotate attribute can be used with
   // '#pragma clang attribute' even though it has no subject list.
   let AdditionalMembers = [{
_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

Reply via email to