[PATCH] D114439: [Annotation] Allow parameter pack expansions and initializer lists in annotate attribute

Erich Keane via Phabricator via cfe-commits cfe-commits at lists.llvm.org
Thu Feb 3 06:09:45 PST 2022


erichkeane 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);
----------------
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.


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

https://reviews.llvm.org/D114439



More information about the cfe-commits mailing list