Skip to content

Remove unused parameter names in StatusArg.h#8976

Open
annafilimonova1 wants to merge 1 commit into
FirebirdSQL:masterfrom
annafilimonova1:fix_unused_params
Open

Remove unused parameter names in StatusArg.h#8976
annafilimonova1 wants to merge 1 commit into
FirebirdSQL:masterfrom
annafilimonova1:fix_unused_params

Conversation

@annafilimonova1

Copy link
Copy Markdown

Part of closed #8938

@hvlad

hvlad commented Apr 2, 2026

Copy link
Copy Markdown
Member

I have no idea how parameter name in declaration could be [un]used.
And why it should be removed.
Honestly, I prefer to name all params in declarations - it greatly helps intellisense to produce better hints.

@hvlad

hvlad commented Apr 2, 2026

Copy link
Copy Markdown
Member

I have no idea how parameter name in declaration could be [un]used.

Oops, it is not only declarations, it contains empty bodies.
Anyway, my opinion is not changed.

@asfernandes

Copy link
Copy Markdown
Member

I have no idea how parameter name in declaration could be [un]used.

Oops, it is not only declarations, it contains empty bodies. Anyway, my opinion is not changed.

Agreed.

@aafemt

aafemt commented Apr 2, 2026

Copy link
Copy Markdown
Contributor

Unfortunately compilers disagree and produce warning for such cases. That's what [[maybe_unused]] is for.

@hvlad

hvlad commented Apr 2, 2026

Copy link
Copy Markdown
Member

Unfortunately compilers disagree and produce warning for such cases.

What compiler and which warnings ? I see none.

@aafemt

aafemt commented Apr 2, 2026

Copy link
Copy Markdown
Contributor

What compiler and which warnings ?

class Foo
{
public:
  void bar(int x) {  }
};

g++ -Wall -Wextra:

warn.cpp: In member function 'void Foo::bar(int)':
warn.cpp:4:16: warning: unused parameter 'x' [-Wunused-parameter]
   void bar(int x) {  }
            ~~~~^

You don't see them just because Firebird build system suppress too many diagnostic messages.

@hvlad

hvlad commented Apr 2, 2026

Copy link
Copy Markdown
Member

Your case is very far from Firebird code above.
Make Foo::bar virtual and add derived class with overrided bar which really uses parameter.
Useless warnings should not distract so mush attention as in this case.

@aafemt

aafemt commented Apr 2, 2026

Copy link
Copy Markdown
Contributor

Make Foo::bar virtual and add derived class with overrided bar which really uses parameter.

Result is exactly the same:

class Foo
{
public:
  virtual void bar(int x) {  }
};

class Bar: public Foo
{
  void bar(int x) override { throw x; }
};
warn.cpp: In member function 'virtual void Foo::bar(int)':
warn.cpp:4:24: warning: unused parameter 'x' [-Wunused-parameter]
   virtual void bar(int x) {  }
                    ~~~~^

Warning-less build would be much more useful than nested namespaces...

@hvlad

hvlad commented Apr 2, 2026

Copy link
Copy Markdown
Member

Result is exactly the same:

Exactly. This is why this warning in this place is completely useless and wrong.

@AlexPeshkoff

AlexPeshkoff commented Apr 3, 2026 via email

Copy link
Copy Markdown
Member

@aafemt

aafemt commented Apr 3, 2026

Copy link
Copy Markdown
Contributor

That's why [[maybe_unused]] exists: to confirm that coder intentionally didn't use the parameter and not just forget to write some important code.

@hvlad

hvlad commented Apr 3, 2026

Copy link
Copy Markdown
Member

In short: not in this case

@aafemt

aafemt commented Apr 3, 2026

Copy link
Copy Markdown
Contributor

JFYI: this attribute is not inheritable.

@hvlad

hvlad commented Apr 3, 2026

Copy link
Copy Markdown
Member

JFYI: this attribute is not inheritable.

wow! this change everything (nope)

I have no idea why do you wrote it.

@aafemt

aafemt commented Apr 3, 2026

Copy link
Copy Markdown
Contributor

When function's parameter isn't mentioned in function's body, compilers (and code analyzers) issue a warning. There are four ways to handle that:

  1. Close your eyes (yours).
  2. Instruct them to shut up (Firebird's).
  3. Remove parameter name (pre-C++17).
  4. Use [[maybe_unused]] attribute (post-C++17).

First two options are bad because warnings are your friends helping to discover bugs in code. Last two options are used case-per-case and show readers that unused parameter is the writer's intention, not mistake.

@hvlad

hvlad commented Apr 3, 2026

Copy link
Copy Markdown
Member

You wrong again. Read this, for example.

I'm insist that:

  • parameter names should not be missed in functions declarations
  • [[maybe_unused]] should not be used as a garbage in functions declarations making it impossible to read.

In this case, when declaration of virtual function in very base class is combined with its empty definition, one may avoid scary stupid useless warnings by:

  • decoupling declaration from definition, or
  • make declaration pure virtual (when/if possible, of course).

But I didn't consider it as necessary at all.

@aafemt

aafemt commented Apr 3, 2026

Copy link
Copy Markdown
Contributor
  • make declaration pure virtual (when/if possible, of course).

Finally you said something that make sense.

@dyemanov

dyemanov commented Apr 3, 2026

Copy link
Copy Markdown
Member

Originally parameters were named. Names were removed by Claudio to eliminate warnings (which implies he had seen them). Then two new methods were added, again with named parameters. This PR eliminates the new warnings again (and it follows the existing practice in this file). I'm open to any arguments except those reverting us back to unnecessary warnings.

Personally, I don't like [[maybe_unused]], even if standard - but that's just me. I don't like missing names either and prefer to use commented names instead. Options suggested by Vlad are the best ones IMHO, but they require some refactoring. The question is whether we insist it to be done by students or just apply the PR and defer the refactoring to some other day.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

6 participants