vikramRH wrote:
> > > > Gentle ping @AaronBallman , @philnik777 , @fpetrogalli :)
> > >
> > >
> > > Ah, sorry -- because the PR is marked as a Draft, I figured it wasn't
> > > ready for review yet.
> > > I think I'd rather this was expressed differently; we already don't put
> > > attribute information in the prototype anyway (`noexcept` as an example),
> > > so I'd prefer to continue down that road and put the address space
> > > information into the `Attributes` field. e.g.,
> > > ```
> > > def BuiltinCPUIs : Builtin {
> > > let Spellings = ["__builtin_cpu_is"];
> > > let Attributes = [NoThrow, Const, AddressSpace<2>];
> > > let Prototype = "bool(char const*)";
> > > }
> > > ```
> > >
> > >
> > >
> > >
> > >
> > >
> > >
> > >
> > >
> > >
> > >
> > > I think that makes it more clean in terms of specifying the attribute,
> > > and it also means we can name the address spaces in `BuiltinsBase.td` if
> > > we would like, which is even easier for folks to understand when reading
> > > `Builtins.td`
> > > WDYT?
> >
> >
> > Thanks for the reply @AaronBallman . The reason this is still a draft is
> > that I wanted it to be an initial proposal to get some inputs and a
> > consensus on the final design. and about it being part of the "Attributes"
> > field, one major issue is that the address space information should be per
> > argument including the return type. "Attributes" field currently expresses
> > attributes to the function. If attribute in the prototype is not desired,
> > probably a new field that lets us specify per argument attributes makes
> > sense ?
>
> Oh! I hadn't realized this was needed on a per-parameter basis. Oof that
> makes this more awkward. I'd still love to avoid writing this as part of the
> signature; I think we could use the existing `IndexedAttribute` to specify
> which argument the attribute applies to. e.g.,
>
> ```
> class AddressSpace<int Idx, int AddrSpaceNum> : IndexedAttribute<"something",
> Idx> {
> int SpaceNum = AddrSpaceNum;
> }
> ```
Makes sense, I will give this a try and update the PR
https://github.com/llvm/llvm-project/pull/108497
_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
