steffenlarsen updated this revision to Diff 399334.
steffenlarsen added a comment.
I have made the changes suggested in my previous comment. This makes
significantly more changes to the parsing of attribute arguments as the old
path was needed for attributes that allow both expressions and types. It also
required some new controlling arguments for `ParseExpressionList`.
Because these changes also allow intializer lists in attribute arguments I have
added a test case showing that `clang::annotate` parses these but will not
accept them.
@erichkeane & @aaron.ballman - Is this in line with what you were thinking?
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,8 +259,18 @@
[[]] 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 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
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 << attrNumArgs;
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
@@ -1815,7 +1815,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 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
