[PATCH] D89743: Support Attr in DynTypedNode and ASTMatchers.

[Yitzhak Mandelbaum via Phabricator via cfe-commits]

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