Hi Paul,
> Because draft C2x requires using [[maybe_unused]] before a parameter
> instead of after, the recent C2x-related changes to Gnulib caused me to
> move all uses of _GL_UNUSED_PARAMETER
Agreed. We should follow standards. The new standard picked a different
position for the marker than what we used to have.
> _GL_ATTRIBUTE_MAYBE_UNUSED is too long, so I propose we rename
> it to something shorter.
In implementation code, it is more convenient to include "attribute.h" and
then use MAYBE_UNUSED. The names with _GL_ prefix are, IMO, only for
places where including "attribute.h" is not desired, namely in public
header files. And in public header files we don't have many function
definitions that ignore some arguments. It's basically only
lib/se-context.in.h
lib/se-label.in.h
lib/se-selinux.in.h
IMO that's not enough justification for introducing a new name for
_GL_ATTRIBUTE_MAYBE_UNUSED.
> Also, draft C2x lets one write the above function without naming the
> parameters, as follows:
>
> SE_SELINUX_INLINE int
> fsetfilecon (int, char const *)
> { errno = ENOTSUP; return -1; }
>
> This is nicer than [[maybe_unused]], because it says the arguments are
> *definitely* unused instead of merely *maybe* unused, and that allows a
> bit more checking of the code.
I disagree on this one. For a human reader who wants to understand the code,
the parameter name is more important than its type. Therefore, for
maintainability your example should better read
SE_SELINUX_INLINE int
fsetfilecon (fd, context)
{ errno = ENOTSUP; return -1; }
In other words, it is a misfeature in the standard. They have designed
this language feature from the perspective what the compiler needs in
order to generate machine code, not from the perspective of the programmer
and maintainability.
So, we should better not use this misfeature.
> So, how about the following ideas:
>
> * Rename _GL_ATTRIBUTE_MAYBE_UNUSED to _GL_maybe_unused. Similarly for
> _GL_deprecated, _GL_fallthrough, and _GL_nodiscard.
Ohhhhhh. This breaks a decades-long convention. We have the convention
in GNU that macro names are upper case, to distinguish them from function
and variable names. This is good convention.
I have personally used mixed-case macro names in GNU clisp, and I can
assert (in hindsight) that it did *not* contribute to maintainability.
> As C2x becomes more
> popular, it'll be easy to read _GL_deprecated as shorthand for
> "[[deprecated]] if supported, empty otherwise".
Whether the expansion is mainly lowercase or not, is irrelevant.
Breaking said convention is not good.
> * Remove all uses of _GL_UNUSED after arguments in Gnulib, replacing
> them with _GL_maybe_unused before arguments. This will support non-GCC
> C2x compilers better. Deprecate _GL_UNUSED.
Hmm. This _GL_UNUSED macro is used a lot in .c code, simply because it
it short (and the "maybe" in _GL_ATTRIBUTE_MAYBE_UNUSED is more of a
compiler technicality). I'm OK with moving all _GL_UNUSED from after
the parameter declaration to before the parameter declaration. Then
we can continue to have
#define _GL_UNUSED _GL_ATTRIBUTE_MAYBE_UNUSED
> * Define a macro _GL_UNUSED_ARG(TYPE, NAME) that expands to 'TYPE' in
> draft C2x, and to '_GL_maybe_unused TYPE NAME' otherwise. That way, one
> can write:
>
> SE_SELINUX_INLINE int
> fsetfilecon (_GL_UNUSED_ARG (int, fd),
> _GL_UNUSED_ARG (char const *, con))
> { errno = ENOTSUP; return -1; }
Such macros with type parameters have two problems:
* Maintainability problem: The reader needs to understand the macro
first, before they can understand the declaration.
Whereas when the declaration reads
SE_SELINUX_INLINE int
fsetfilecon (_GL_UNUSED int fd, _GL_UNUSED char const *con)
the reader understands 70% of the syntax and can treat _GL_UNUSED
as "some token I don't need to know about".
* It doesn't apply to function types that are not typedefs. How would
you write
int foo (_GL_UNUSED int (*p) (const void *) )
?
> * Define a macro _GL_UNUSED_ARGNAME(NAME) that expands to NAME if not
> draft C2x, empty otherwise. Programs can use this macro for complicated
> argument types where _GL_UNUSED_ARG does not suffice, e.g.,
> '_GL_maybe_unused int (*_GL_UNUSED_ARGNAME (p)) (int)'.
I object for two reasons:
- it's a macro with type parameter (see above),
- its purpose is just to make use of a misfeature in the language (see
above as well).
> * Remove the snippet/unused-parameter module as it's not used now.
Indeed, this module is unused in gnulib. It may be used in packages that
use gnulib; therefore I vote for marking it 'obsolete' and remove it only
in a year or two.
Bruno
