On Wed, Aug 28, 2024 at 10:48 AM Jeff King <peff@xxxxxxxx> wrote: > 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. > Signed-off-by: Jeff King <peff@xxxxxxxx> > --- > 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.
