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

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