aaron.ballman added inline comments.
================ Comment at: clang/lib/Sema/SemaDeclAttr.cpp:4203 + if (AllArgs.size() && AllArgs[0]->isValueDependent()) { + auto *Attr = AnnotateAttr::CreateWithDelayedArgs( + S.getASTContext(), AllArgs.data(), AllArgs.size(), AL); ---------------- erichkeane wrote: > aaron.ballman wrote: > > erichkeane wrote: > > > I would like @aaron.ballman to comment on this, but I think we probably > > > want this case to be covered in the top of `HandleDeclAttr`, which would > > > mean in the 'not all values filled' case, we skip the > > > 'handleAnnotateAttr'. > > > > > > WDYT Aaron? The downside is that this function couldn't check a > > > 'partially filled in' attribute, but it would make us that much closer to > > > this flag being a very simple thing to opt into. > > Do you mean `ProcessDeclAttribute()`? I don't think we should have > > attribute-specific logic in there, but are you thinking of something more > > general than that (I'm not seeing how the suggestion makes the flag easier > > to opt into)? > Ah, yes, thats what I mean. The question for ME is whether there should be a > generic "this supports variadic pack, so check to see if all the named > non-expr arguments are fill-in-able. If not, do the 'delayed' version. > > This would mean that HandleAnnotateAttr NEVER sees the > "CreateWithDelayedArgs" case. Thanks for clarifying -- yes, I think that would be preferable if it works out in a clean, generic way. I'd be fine with tablegen emitting something else (if necessary) to help generalize it. 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