Bug 68230 - Unused function parameters not reported by -Wunused-parameter when only used recursively (add -Wparameter-only-used-recursively instead?)
Summary: Unused function parameters not reported by -Wunused-parameter when only used ...
Status: NEW
Alias: None
Product: gcc
Classification: Unclassified
Component: c++ (show other bugs)
Version: 5.2.0
: P3 enhancement
Target Milestone: ---
Assignee: Not yet assigned to anyone
URL:
Keywords: diagnostic
Depends on:
Blocks: new-warning Wunused
  Show dependency treegraph
 
Reported: 2015-11-06 01:11 UTC by Joshua T, Fisher
Modified: 2019-09-19 02:52 UTC (History)
5 users (show)

See Also:
Host:
Target:
Build:
Known to work:
Known to fail:
Last reconfirmed: 2017-09-28 00:00:00


Attachments

Note You need to log in before you can comment on or make changes to this bug.
Description Joshua T, Fisher 2015-11-06 01:11:35 UTC
The code below does not produce a warning when -Wunused-parameter is turned on. It would be fantastic if it did, as fundamentally, the variable notUsed is still not actually being used.

#include <iostream>
using namespace std;

int Recursive(int num, const char *notUsed);

int main() {
  const char *notUsedLocal = "Not used";

  cout << "Test: " << Recursive(1, notUsedLocal) << endl;
  return 0;
}

int Recursive(int num, const char *notUsed) {
  if (num > 5)
    return num;
  else
    return Recursive(++num, notUsed);
}
Comment 1 Eric Gallager 2017-09-28 12:41:18 UTC
Confirmed that there's currently no warning.
Comment 2 Jonathan Wakely 2017-09-28 13:00:28 UTC
I'm not sure there *should* be a warning here.

I find -Wunused-parameter most useful to know when I can remove the name of the parameter, but you can't do that here (because it still needs a name to pass it to the recursive call).

It's rare that my reaction to a -Wunused-parameter warning is to change the function to remove the parameter entirely. YMMV.
Comment 3 Eric Gallager 2017-09-28 14:38:08 UTC
(In reply to Jonathan Wakely from comment #2)
> I'm not sure there *should* be a warning here.
> 
> I find -Wunused-parameter most useful to know when I can remove the name of
> the parameter, but you can't do that here (because it still needs a name to
> pass it to the recursive call).
> 
> It's rare that my reaction to a -Wunused-parameter warning is to change the
> function to remove the parameter entirely. YMMV.

There's also attribute unused and pragma unused, but yeah, it'd feel weird using either of those here, too... maybe this should be closed as WONTFIX instead...
Comment 4 Jonathan Wakely 2017-09-28 15:09:38 UTC
Those aren't portable, so aren't a good general solution.
Comment 5 Joshua T, Fisher 2017-09-28 17:51:36 UTC
(In reply to Jonathan Wakely from comment #2)
> It's rare that my reaction to a -Wunused-parameter warning is to change the
> function to remove the parameter entirely. YMMV.

Totally reasonable, I too rarely change the signature and use either attributes, removing the parameter name, or the void cast trick. However I have seen bugs pop up a couple times from important parameters being included, but not used, see:

https://github.com/nvpro-pipeline/VkHLF/commit/b6646c4773e8aef49c40e8684eca1382bf2e9d50

and my blog where I first encountered this in a closed source codebase:

http://www.nullterminatedstrings.com/c++/recursive-warning

Perhaps this doesn't qualify as a bug in the current -Wunused-parameter warning implementation, but would it be reasonable to add it as a separate warning?

Thanks for looking into this by the way folks!
Comment 6 Joshua T, Fisher 2017-09-28 21:00:02 UTC
(In reply to Joshua T, Fisher from comment #5)
> https://github.com/nvpro-pipeline/VkHLF/commit/b6646c4773e8aef49c40e8684eca1382bf2e9d50

Apologies this one doesn't apply. Just took another look and I just misremembered, this is just a normal -Wunused-parameter, I don't think they had it turned on.
Comment 7 Jonathan Wakely 2017-09-28 23:57:15 UTC
(In reply to Joshua T, Fisher from comment #5)
> and my blog where I first encountered this in a closed source codebase:
> 
> http://www.nullterminatedstrings.com/c++/recursive-warning

I'm a bit more persuaded having read this. If it points out you forgot to use it (for anything except the recursion) that could help find real bugs.
Comment 8 Joshua T, Fisher 2017-10-03 00:47:42 UTC
(In reply to Jonathan Wakely from comment #7)
> (In reply to Joshua T, Fisher from comment #5)
> > and my blog where I first encountered this in a closed source codebase:
> > 
> > http://www.nullterminatedstrings.com/c++/recursive-warning
> 
> I'm a bit more persuaded having read this. If it points out you forgot to
> use it (for anything except the recursion) that could help find real bugs.

That's what I was hoping! I feel like it's definitely something that could hit some real code bases. I find it unlikely I'm the only person ever to run into this.
Comment 9 Eric Gallager 2018-10-03 02:24:31 UTC
(In reply to Joshua T, Fisher from comment #8)
> (In reply to Jonathan Wakely from comment #7)
> > (In reply to Joshua T, Fisher from comment #5)
> > > and my blog where I first encountered this in a closed source codebase:
> > > 
> > > http://www.nullterminatedstrings.com/c++/recursive-warning
> > 
> > I'm a bit more persuaded having read this. If it points out you forgot to
> > use it (for anything except the recursion) that could help find real bugs.
> 
> That's what I was hoping! I feel like it's definitely something that could
> hit some real code bases. I find it unlikely I'm the only person ever to run
> into this.

Maybe it'd be possible to put it under a separate flag (e.g. -Wparameter-only-used-recursively) so that it'd be easier to focus on just that kind of warning individually? There are some codebases that turn off -Wunused-parameter specifically that still might want this warning.
Comment 10 Eric Gallager 2019-09-19 02:52:32 UTC
Martin Jambor's IPA-SRA rewrite might be relevant here; it sounded like the new IPA-SRA will remove parameters that are unused like this, but I didn't quite catch if it'd also warn about them...