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

Reply via email to