On Thu, Aug 25, 2022 at 01:00:19PM +0200, Ævar Arnfjörð Bjarmason wrote: > > One, it feels like we're abusing the deprecated attribute here. The > > Definitely, but structurally it seems like a better pick. I.e. isn't the > only problem with it the "deprecated" and its interaction with > -Wno-deprecated. > > If the exact same feature existed as a "insert-custom-warning", which > would work exactly "deprecated" without the default warning "prefix" > would you think this would fit perfectly? Yeah, I agree that would remove my complaint about overloading the meaning. I don't think that exists, though. > This is mildly annoying, but I don't really think it's a practical > issue. We're talking about running this without > -Wno-deprecated-declarations in CI, and by default. > > For unused parameters it's enough that we're catching them somewhere, or > in common compilation settings, we don't need to catch them > *everywhere*, do we? No, but the farther away you go from the edit-compile-run cycle, the more painful warnings become. Catching them immediately and fully has real value, as it means the cost of correcting them is lower. So all things being equal, I think we should prefer universal solutions when they're available (and for example compiler errors over say, coccinelle or other analysis tools). (And yes, I know all things sadly aren't equal; see below...) > IOW is anyone writing patches where they're testing with > -Wno-deprecated-declarations *and* adding unused parameters *and* won't > test without -Wno-deprecated-declarations before submitting them, *and* > nobody else will catch it? Probably not. I don't actually build with -Wno-deprecated-declarations routinely. But my fear is that some platform may be stuck there for a while (because an overzealous libc marks something). But that's kind of hypothetical, so we may have to just accept it and cross that bridge if we come to it. > > And finally, I actually prefer the parentheses of: > > > > static int register_ref(const char *refname, const struct object_id *oid, > > int UNUSED(flags), void *UNUSED(cb_data)) > > ...and now to the real reason for the follow-up. You/Junio were CC'd, > but this is breaking coccinelle, see: > https://lore.kernel.org/git/220825.86ilmg4mil.gmgdl@xxxxxxxxxxxxxxxxxxx/ Ugh. Yeah, that is really unfortunate. I much prefer the parenthesized syntax, but if we can't find a way to unconfuse third-party parsing, then switching is probably the least-bad solution. -Peff
