steffenlarsen added inline comments.
================ Comment at: clang/include/clang/Basic/Attr.td:545 + // Set to true if this attribute accepts parameter pack expansion expressions. + bit AcceptsExprPack = 0; // Lists language options, one of which is required to be true for the ---------------- aaron.ballman wrote: > You should probably add a release note about this new feature. I have added a release note about this under "Internal API Changes". Let me know if you think the wording needs changes. ================ Comment at: clang/include/clang/Basic/Attr.td:789 let PragmaAttributeSupport = 1; + let AcceptsExprPack = 1; let Documentation = [Undocumented]; ---------------- aaron.ballman wrote: > You should definitely add a release note about this new behavior for annotate. I have added a release note about this under "Attribute Changes in Clang". As with the other note, let me know if wording needs changes. ================ Comment at: clang/lib/Sema/SemaDeclAttr.cpp:8153 + + bool AttrHasVariadicArg = AL.hasVariadicArg(); + unsigned AttrNumArgs = AL.getNumArgMembers(); ---------------- erichkeane wrote: > steffenlarsen wrote: > > erichkeane wrote: > > > This still doesn't work if the VariadicExprArgument isn't last, right? > > > Do we ensure that is the case in clang-attr-emitter? > > Good point. I don't think there's a check as there are select few > > attributes that use multiple variadic (`OMPDeclareSimdDecl` and > > `OMPDeclareVariant` are the only ones, I think.) > > > > Since I don't think it's safe to check for all, should I make a check > > similar to the one for type/identifier arguments in attributes marked > > `AcceptsExprPack`? Would that suffice? > I'm fine rejecting a case that has anything besides expression-arguments(and > ones create-able from expression arguments) and > limited-to-only-1-must-be-last variadic-expr-list in ClangAttrEmitter. > > I believe we discussed that at one point, but I didn't see it here. I have added a check to ClangAttrEmitter next to the check ensuring that the relevant attributes don't have identifier or type arguments. CHANGES SINCE LAST ACTION https://reviews.llvm.org/D114439/new/ https://reviews.llvm.org/D114439 _______________________________________________ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
