arichardson marked an inline comment as done.
arichardson added inline comments.
================
Comment at: include/clang/Basic/Attr.td:1072
def AllocSize : InheritableAttr {
let Spellings = [GCC<"alloc_size">];
+ let Subjects = SubjectList<[HasFunctionProto]>;
----------------
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.
Repository:
rC Clang
CHANGES SINCE LAST ACTION
https://reviews.llvm.org/D55212/new/
https://reviews.llvm.org/D55212
_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
