ymandel added inline comments.

================
Comment at: clang/lib/AST/ASTTypeTraits.cpp:138
+    return ASTNodeKind(NKI_##A##Attr);
+#include "clang/Basic/AttrList.inc"
+  }
----------------
aaron.ballman wrote:
> Oye, this brings up an interesting point. Plugin-based attributes currently 
> cannot create their own semantic attribute, but will often instead reuse an 
> existing semantic attribute like `annotate`. This means code like 
> `[[clang::plugin_attr]] int x;` may or may not be possible to match. Further, 
> some builtin attributes have no semantic attribute associated with them 
> whatsoever: 
> https://github.com/llvm/llvm-project/blob/master/clang/include/clang/Basic/Attr.td#L2740
> 
> I think the `switch` statement logic here is correct in these weird cases and 
> we won't hit the `llvm_unreachable`. For attributes with no AST 
> representation, there's no `Attr` object that could be passed in the first 
> place. Unknown attributes similarly won't get here because there's no way to 
> get an AST node for them. Plugin-based attributes are still going to be 
> similarly surprising, but... I don't know that we can solve that here given 
> there's no way to create a plugin-based semantic attribute yet.
> 
> Pining @ymandel to raise awareness of these sorts of issues that stencil may 
> run into. For the AST matchers, I think it's reasonable for us to say "if 
> there's no AST node, we can't match on it", but IIRC, stencil was looking to 
> stay a bit closer to the user's source code rather than be strongly tied to 
> the AST.
@aaron.ballman Thanks for pinging me. However, `Stencil` is limited to AST 
nodes, for better or worse. They make it somewhat easier to _generate_ source 
plainly, but they are fundamentaly an abstraction over the AST.  I think that 
the only way we'll get beyond the AST is Syntax Trees.

Still, nice to see this patch! I've been meaning to do the same for 
LambaCapture for a while and this will be a handy guide to what needs to be 
changed.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D89743

_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

Reply via email to