https://gcc.gnu.org/bugzilla/show_bug.cgi?id=79961

--- Comment #11 from Pedro Alves <palves at redhat dot com> ---
> All the interesting calls here are undefined.

I meant that the one pointed out is undefined even without the nonnull
attribute.  I.e., it's not a use case that justifies supporting nonnull(1) on
non-members.  The others are only undefined due to the nonnull attribute.
Actually, I'd think that G++ should warn/error for that "((A*)0)->f();" case as
if the user that specified nonnull(1), even if the user hadn't.  I.e.,
implicitly add the nonnull(1) on all non-static methods.

> 
> The point of the example is to highlight that the nonnull attribute on the
> typedef
> 
>   typedef void F (void*) __attribute__ ((__nonnull__ (1)));
> 
> is interpreted differently depending on how the typedef is used.  While in
> most cases the number refers to the first void* argument, when it's used to
> declare a member function it refers to the implicit this argument.  If
> attribute((nonnull(1))) is to be rejected on non-static member function
> declarations as you suggest this seems like a case worth keeping in mind.

Yes, but that's not specific to the implicit this argument.  That's true for
_all_ arguments.  Change the prototype to:

  typedef void F (int, void*) __attribute__ ((__nonnull__ (2)));

and now when F is used as type of a class method, if the nonnull argument
number is reevaluated, nonnull(2) applies to the "int", which would be an
error, because that's not a pointer parameter.  Interestingly, seems like
neither G++ nor clang error in that case:

$ cat nonnull2.cc
typedef void F (int, void*) __attribute__ ((__nonnull__ (2)));

struct A
{
  F foo;
};

void foo ()
{
  A a;

  ((A *) 0)->foo (0, &a);
  ((A *) 0)->foo (1, 0);
  ((A *) 0)->foo (0, 0);
  a.foo (0, &a);
  a.foo (0, 0);
}
$ /opt/gcc/bin/g++ -O2 -c -Wall -Wextra -Wpedantic nonnull2.cc 
(nothing)
$ clang++ -O2 -c -Wall -Wextra -Wpedantic nonnull2.cc 
(nothing)

So I'm not sure what's the logic that dictates when and how are nonnull
argument numbers reevaluated in the method typedef case.  Looks like the
nonnull attribute is discarded above.  But in your case with nonnull(1), it
isn't.  ??!

Note that adding back the nonnull attribute like this:

struct A
{
- F foo;
+ F __attribute__ ((__nonnull__ (3))) foo;
};

makes both G++ and clang complain again as expected:

nonnull2.cc:13:23: warning: null argument where non-null required (argument 3)
[-Wnonnull]
   ((A *) 0)->foo (1, 0);
                       ^
nonnull2.cc:14:23: warning: null argument where non-null required (argument 3)
[-Wnonnull]
   ((A *) 0)->foo (0, 0);
                       ^
nonnull2.cc:16:14: warning: null argument where non-null required (argument 3)
[-Wnonnull]
   a.foo (0, 0);
              ^

> 
> The call to A::g in the test case is meant to demonstrate that simply
> printing "null argument where non-null required (argument 1)" in the
> diagnostic may not be sufficient to identify which argument is being
> referred to.  

I agree.

> The fact that the caret doesn't point at the argument only
> makes it that much more difficult (and underscores the importance of
> referring to the argument more clearly, e.g., by printing "the this pointer"
> instead of "argument 1" or something like that).

Definitely agreed.  For the other arguments, underlining the parameter in
question, as in clang's output shown in comment 7 makes it much simpler to
grok, IMO:

nonnull.cc:5:51: warning: '__nonnull__' attribute only applies to pointer
arguments [-Wignored-attributes]
  void foo (int, const char *arg) __attribute__ ((__nonnull__ (2)));
            ~~~            

> Here's a slightly different example that should make that clear:

Understood and agreed.

Reply via email to