Re: [PATCH 4/13] Coarray shared memory library

[Tobias Burnus]

Jerry D wrote:

     Fortran: Fix signatures of coarray API and caf_single.

The teams argument to some functions was marked as unused in the header.

     With upcoming caf_shmem this is incorrect, given the mark is repeated in
     caf_single.

First: I am completely happy with the patch. But not with that wording.

There seems to be some misunderstanding of this attribute: 'unused' does not
meant that it is guaranteed to be unused but that it might be unused. That's
used here to silence -Wunused-parameter (→ unused dummy arguments) warnings.

Cf. 
https://gcc.gnu.org/onlinedocs/gcc/Common-Variable-Attributes.html#index-unused-variable-attribute

Thus, it is not 'incorrect' to keep the 'unused'.

Still, it is cleaner to remove it from the header - and keep it just in
the function definition inside caf/single.c - thus, I like the change.

When focusing on 'incorrect', it should rather mention the printf with va_list
bug fix!

     libgfortran/ChangeLog:

* caf/libcaf.h (_gfortran_caf_failed_images): Team attribute is

             used now in some libs.
             (_gfortran_caf_image_status): Same.
             (_gfortran_caf_stopped_images): Same.
             * caf/single.c (caf_internal_error): Use correct printf function
             to handle va_list.

For the *.h changes, I think something like "Remove 'unused' attribute
from team argument" would be clearer. (The bullet points could also be
combined ["(fn1, fn2, fn3):"])

As mentioned, the patch itself LGTM - but consider some tweaking of the
ChangeLog - and the wording before the changelog.

As the commit title doesn't show up in the email thread but is buried in
the attachment: Maybe tweak the subject as well:
   "Fix signatures of coarray API and caf_single."

There is fortunately no API issue but just a cleanup (remove 'unused' from *.h)
and a bug fix inside caf_single.

Tobias