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); }
Confirmed that there's currently no warning.
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.
(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...
Those aren't portable, so aren't a good general solution.
(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!
(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.
(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.
(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.
(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.
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...
bug 32562 is related (that's for functions, this is for parameters)
(In reply to Eric Gallager from comment #10) > 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... IPA-SRA can remove parameters that are only used to be passed to another function in a chain of recursive calls, it does not even have to be a direct recursion (but it can). It does not warn but it could. One problem is however that IPA-SRA only modifies and analyzes "local" functions, so in non-LTO mode it only works if all of the recursive functions were static or in an anonymous C++ namespace. With LTO, anything not exported outside the result executable/shared object would do but you'd only get your warnings at link-time. Please also note that with this particular testcase, by the time IPA passes see the function the recursion is converted to tail-call and since IPA-CP (running just before IPA-SRA) can also remove totally unused parameters, IPA-SRA does not do anything in this case even with -fwhole-program.