steffenlarsen updated this revision to Diff 392340.
steffenlarsen added a comment.
The new revision does the following:
1. Revisits the diagnostics. Now parameter packs will cause the same error on
attributes that do not support parameter packs in arguments, while attributes
that support it will cause an error if parameter packs are used in the
incorrect argument, specifying which argument should be the earliest for it.
2. Adds additional tests to ensure that variadic arguments supporting packs
also allow pack expansion intermingled with other arguments.
3. Adds assertions to attribute generation to check that attributes only ever
support packs in the last variadic argument and that there are no other
variadic arguments if they do.
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/lib/Parse/ParseDecl.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
@@ -258,8 +258,17 @@
[[]] return;
}
-template<typename...Ts> void variadic() {
- void bar [[noreturn...]] (); // expected-error {{attribute 'noreturn' cannot be used as an attribute pack}}
+template <int... Is> void variadic() {
+ 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 pack expansion in arguments}}
+ void boz [[clang::annotate(Is...)]] (); // expected-error {{pack expansion in 'annotate' is restricted to argument 2 or later}}
+ void bor [[clang::annotate("A", "V" ...)]] (); // expected-error {{pack expansion does not contain any unexpanded parameter packs}}
+ void boo [[unknown::foo(Is...)]] (); // expected-warning {{unknown attribute 'foo' ignored}}
+ void faz [[clang::annotate("B", (Is + ...))]] (); // expected-warning {{pack fold expression is a C++17 extension}}
+ void far [[clang::annotate("C", Is...)]] ();
+ void foz [[clang::annotate("D", 1, 2, 3, Is...)]] ();
+ void fiz [[clang::annotate("E", Is..., 1, 2, 3)]] ();
+ void fir [[clang::annotate("F", 1, Is..., 2, 3)]] ();
}
// Expression tests
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/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
@@ -425,6 +444,31 @@
ExprResult ArgExpr(
Actions.CorrectDelayedTyposInExpr(ParseAssignmentExpression()));
+
+ if (Tok.is(tok::ellipsis)) {
+ // 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;
+ }
+
+ // Pack expansion is only allowed in the variadic argument at the end.
+ const size_t attrNumArgs = attributeNumberOfArguments(*AttrName);
+ if (ArgExprs.size() + 1 < attrNumArgs) {
+ Diag(Tok.getLocation(),
+ diag::err_attribute_parm_pack_last_argument_only)
+ << AttrName << attrNumArgs;
+ SkipUntil(tok::r_paren, StopAtSemi);
+ return 0;
+ }
+
+ ArgExpr = Actions.ActOnPackExpansion(ArgExpr.get(), ConsumeToken());
+ }
+
if (ArgExpr.isInvalid()) {
SkipUntil(tok::r_paren, StopAtSemi);
return 0;
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 pack expansion in arguments">;
+def err_attribute_parm_pack_last_argument_only : Error<
+ "pack expansion in %0 is restricted to argument %1 or later">;
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
