[PATCH] D128440: [WebAssembly] Initial support for reference type funcref in clang

[Paulo Matos via Phabricator via llvm-commits]

pmatos marked an inline comment as done.
pmatos added inline comments.


================
Comment at: clang/include/clang/Basic/Attr.td:4129
+  let Documentation = [WebAssemblyExportNameDocs];
+  let Subjects = SubjectList<[TypedefName], ErrorDiag>;
+}
----------------
aaron.ballman wrote:
> erichkeane wrote:
> > pmatos wrote:
> > > erichkeane wrote:
> > > > pmatos wrote:
> > > > > erichkeane wrote:
> > > > > > Note that it seems this is likely not going to work correctly on template type aliases!  You probably want to make sure that is acceptable to you.
> > > > > Documentation says about Subject:
> > > > > 
> > > > > ```
> > > > >   // The things to which an attribute can appertain
> > > > >   SubjectList Subjects;
> > > > > ```
> > > > > 
> > > > > Given this can be attached to function pointer types, is the subject then just Type ?
> > > > IIRC, the list of subjects are the Decls and Stmt types that are possibly allowed.  I don't thnk it would be just 'Type' there.
> > > > 
> > > > As you have it, it is only allowed on TypedefNameDecl.
> > > > 
> > > > See the SubsetSubject's at the top of this file for some examples on how you could constrain a special matcher to only work on function pointer decls.
> > > Good point @erichkeane . What do you think of FunctionPointer implementation here?
> > That seems acceptable to me.
> That makes sense to me as well, but FWIW, type attributes don't currently have much of any tablegen automation (unlike decl and stmt attributes), so the subject list isn't strictly necessary here. But it's still appreciated because 1) it's documentation, and 2) we want to tablegen more and this helps us with that goal.
@erichkeane Yes, you're right. Something like 

```
using IntIntFuncref = int(*)(int) __funcref;
```

doesn't work. Why is this not accepted? Is it due to the Subjects listed?


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D128440