git/list[1] front-page[2] threads[3] people[4] search[5] about
 

Re: [PATCH 7/6] CodingGuidelines: mention -Wunused-parameter and UNUSED

From
Eric Sunshine <sunshine@sunshineco.com>
Date
Aug 28, 2024, 19:53 UTC
Message-ID
<CAPig+cSBPxtv50KknJVbucAa9RGxQxP66XHpjCjECWdSGPRHjw@mail.gmail.com>
In-Reply-To
<20240828144814.GB4020916@coredump.intra.peff.net>
On Wed, Aug 28, 2024 at 10:48 AM Jeff King <peff@peff.net> wrote:
Show 28 quoted lines
> On Wed, Aug 28, 2024 at 01:56:13AM -0400, Eric Sunshine wrote:
> > What is the expectation regarding newcomers to the project or even
> > people who have not been following this topic and its cousins?
> > Documentation/CodingGuidelines recommends enabling DEVELOPER mode,
> > which is good, but this change means that such people may now be hit
> > with a compiler complaint which they don't necessarily know how to
> > deal with in the legitimate case #3 (described above). Should
> > CodingGuidelines be updated to mention "UNUSED" and the circumstances
> > under which it should be used?
>
> Yeah, I agree some guidance is in order. How about this on top? I didn't
> go into the decision tree of when you should remove the parameter versus
> using it versus annotating it. I think in general that the first two are
> pretty obvious when they are appropriate, and we just need to focus on
> the annotating option.
>
> -- >8 --
> Subject: [PATCH] CodingGuidelines: mention -Wunused-parameter and UNUSED
>
> Now that -Wunused-parameter is on by default for DEVELOPER=1 builds,
> people may trigger it, blocking their build. When it's a mistake for the
> parameter to exist, the path forward is obvious: remove it. But
> sometimes you need to suppress the warning, and the "UNUSED" mechanism
> for that is specific to our project, so people may not know about it.
>
> Let's put some advice in CodingGuidelines, including an example warning
> message. That should help people who grep for the warning text after
> seeing it from the compiler.
Makes sense.
Show 10 quoted lines
> Signed-off-by: Jeff King <peff@peff.net>
> ---
> diff --git a/Documentation/CodingGuidelines b/Documentation/CodingGuidelines
> @@ -258,6 +258,13 @@ For C programs:
> + - When using DEVELOPER=1 mode, you may see warnings from the compiler
> +   like "error: unused parameter 'foo' [-Werror=unused-parameter]",
> +   which indicates that a function ignores its argument. If the unused
> +   parameter can't be removed (e.g., because the function is used as a
> +   callback and has to match a certain interface), you can annotate the
> +   individual parameters with the UNUSED keyword, like "int foo UNUSED".

Perfect. This fully addresses the question expressed by my review comment. Thank you.

Previous: Jeff KingNext: Junio C Hamano
Message 11 of 13 in “unused parameters: the final countdown”
  1. 0/6 unused parameters: the final countdownJeff King, Aug 28, 2024
  2. 1/6 gc: mark unused config parameter in virtual functionsJeff King, Aug 28, 2024
  3. 2/6 t-reftable-readwrite: mark unused parameter in callback functionJeff King, Aug 28, 2024
  4. 3/6 compat: disable -Wunused-parameter in 3rd-party codeJeff King, Aug 28, 2024
  5. 4/6 compat: disable -Wunused-parameter in win32/headless.cJeff King, Aug 28, 2024
  6. 5/6 compat: mark unused parameters in win32/mingw functionsJeff King, Aug 28, 2024
  7. 6/6 config.mak.dev: enable -Wunused-parameter by defaultJeff King, Aug 28, 2024
  8. Eric SunshineAug 28, 2024
  9. Patrick SteinhardtAug 28, 2024
  10. 7/6 CodingGuidelines: mention -Wunused-parameter and UNUSEDJeff King, Aug 28, 2024
  11. Eric SunshineAug 28, 2024
  12. Junio C HamanoAug 28, 2024
  13. Jeff KingAug 28, 2024

Read the whole thread, see it on lore, or plain text.

$ cat FOOTERMessages come from the public archive at lore.kernel.org/git, fetched every hour. The front page is chosen and written each morning by an AI editor and can be wrong; the threads themselves are the record. About and API. For agents: an MCP server at https://gitlist.dev/mcp, and any thread, story or person page as Markdown by adding .md to its URL (or sending Accept: text/markdown). Details in /llms.txt.