aaron.ballman added a comment.
I think all that's missing are some C++ tests and some nits.
================
Comment at: include/clang/Basic/Attr.td:1072
def AllocSize : InheritableAttr {
let Spellings = [GCC<"alloc_size">];
+ let Subjects = SubjectList<[HasFunctionProto]>;
----------------
arichardson wrote:
> aaron.ballman wrote:
> > arichardson wrote:
> > > aaron.ballman wrote:
> > > > arichardson wrote:
> > > > > aaron.ballman wrote:
> > > > > > Does GCC support writing `alloc_size` on function pointers?
> > > > > Yes it does and it seems to be used by some projects. I first
> > > > > discovered this while compiling libxml2:
> > > > > https://github.com/GNOME/libxml2/blob/35e83488505d501864826125cfe6a7950d6cba78/include/libxml/xmlmemory.h#L66
> > > > Parsed and ignored is different than supported. For instance, I can't
> > > > seem to get GCC to produce different behavior here:
> > > > https://godbolt.org/z/MI5k_m
> > > >
> > > > Am I missing something?
> > > Ah yes, it does seem like it is ignored. I assumed it would work for GCC
> > > since it handles, e.g. the format attribute on function pointers.
> > >
> > > However, I would like it if we could support it on function pointers even
> > > if GCC just ignores is.
> > > However, I would like it if we could support it on function pointers even
> > > if GCC just ignores is.
> >
> > We can, but the question is: under what spelling(s)? I think we can support
> > this under `__attribute__((alloc_size(N)))` without issue. If GCC is
> > looking to implement this same behavior, then I think we can also support
> > it under `[[gnu::alloc_size(N)]]`.
> >
> > If GCC doesn't have plans to add this functionality any time soon, we'd be
> > stepping on their implementation namespace by implementing this
> > functionality under the gnu vendor namespace and users would then be
> > confused as to whose bug it is. In that case, I'd recommend we add
> > `[[clang::alloc_size(N)]]` that behaves identically to
> > `[[gnu::alloc_size(N)]]' except that it can be applied to function pointer
> > types, and not change the behavior of `[[gnu::alloc_size(N)]]`. This can be
> > implemented as a single semantic attribute by looking at the semantic
> > spelling of the attribute and issuing a compatibility warning if you see
> > `[[gnu::alloc_size(N)]]` being applied to a function pointer type, and can
> > even recommend a fix-it to switch to `[[clang::alloc_size(N)]]` in that
> > case. (See uses of `getSemanticSpelling()` elsewhere in the project for
> > examples of how to test which spelling a given sematic attribute was
> > spelled with.)
> It seems like GCC will also implement this behaviour so using
> `[[gnu::alloc_size()]]`
> (https://gcc.gnu.org/bugzilla/show_bug.cgi?id=88372#c1) should be fine.
Fantastic, thank you for coordinating this! I think using the `GCC` spelling is
the right way to go then.
Repository:
rC Clang
CHANGES SINCE LAST ACTION
https://reviews.llvm.org/D55212/new/
https://reviews.llvm.org/D55212
_______________________________________________
cfe-commits mailing list
[email protected]
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
