Yeah, I think that'd mostly work. But be careful about this case:

template<typename T>
void f(T *p __attribute__((nonnull))) {
  p(0); // #1
}
void g() { f<void(int *q)>(0); } // #2

Here again, GCC warns on #1 and Clang warns on #2. If we want to keep
'nonnull' as the spelling for this new thing, I think we should keep
warning on #2. (But I do wonder if we should come up with a new name for
this, rather than picking something that collides with an existing GCC
attribute, even if only in corner cases.)

On Mon Feb 17 2014 at 4:21:49 PM, Jordan Rose <[email protected]> wrote:

> +Ted.
>
> I'm guessing we should stick with GCC's interpretation when the parameter
> is a function or block pointer, but maybe have a warning, and a fix-it for
> both possible interpretations.
>
> Jordan
>
>
> On Feb 17, 2014, at 15:52, Richard Smith <[email protected]> wrote:
>
> This still seems to be GCC-incompatible. Per the GCC documentation,
> "__attribute__((nonnull))" on a pointer-to-function parameter means that
> all of *that* function's parameters can't be null. For instance:
>
> void f(int (*p)(int *a, int *b) __attribute__((nonnull))) {
>   if (p)
>     p(0, 0); // #1
> }
> void g() { f(0); } // #2
>
> GCC warns on line #1, Clang warns on line #2.
>
> On Tue Feb 11 2014 at 9:40:01 AM, Aaron Ballman <[email protected]>
> wrote:
>
> On Tue, Feb 11, 2014 at 12:37 PM, Jordan Rose <[email protected]>
> wrote:
> >
> > On Feb 11, 2014, at 9:36 , Aaron Ballman <[email protected]> wrote:
> >
> > +    if (D->getFunctionType()) {
> > +      handleNonNullAttr(S, D, Attr);
> > +    } else {
> > +      S.Diag(Attr.getLoc(), diag::warn_attribute_nonnull_parm_no_args)
> > +        << D->getSourceRange();
> > +    }
> >
> >
> > Minor nit about the style -- shouldn't be using the curly braces here.
> >
> >
> > Uh...hm. I tried it without the curly braces, but it looked very strange
> > with the << dangling there. Should I add a dummy comment to justify it?
>
> I'm not too keen on dummy comments. If you think it's ugly without the
> braces, I'm fine with leaving them. :-)
>
> ~Aaron
> _______________________________________________
> cfe-commits mailing list
> [email protected]
> http://lists.cs.uiuc.edu/mailman/listinfo/cfe-commits
>
>
>
>
_______________________________________________
cfe-commits mailing list
[email protected]
http://lists.cs.uiuc.edu/mailman/listinfo/cfe-commits

Reply via email to