[C++] Top-level const and volatile should not be present for function parameters

[cjdb]

Describe the bug
void f(int), void f(int const), and void f(int volatile) are all the declarations for the same overload A C++ best-practice is to not const-qualify proper function declarations, and only function definitions. This is because one can declare f as void f(int const), and then define it as void f(int). You can verify this using this Compiler Explorer example.
Note that this does not apply to references. void f(int&) and void f(int const&) are different overloads, as demonstrated by this second Compiler Explorer example.

Top-level const-qualifier is redundant at best, but it's potentially misleading, since a reader may interpret this to mean that x won't change, which isn't guaranteed by a proper declaration, since the definition can change this up.

Functions with inline linkage are typically defined in header files, which means that the Doxygen will be like so:

/// \returns x * x
int square(int const x, int volatile y)
{
  return x * x;
}

For the parameters, Doxygen generates the following.

<param>
  <type>int const</type>
  <declname>x</declname>
</param>
<param>
  <type>int volatile</type>
  <declname>y</declname>
</param>

Expected behavior
The above XML should be

<param>
  <type>int</type>
  <declname>x</declname>
</param>
<param>
  <type>int</type>
  <declname>y</declname>
</param>

To Reproduce

1. Set up a new Doxygen project and enable XML output.
2. Add square to the C++ file being processed.
3. Run Doxygen.

Version

$ doxygen --version
1.9.6

$ lsb_release -a
LSB Version:    n/a
Distributor ID: Arch
Description:    Arch Linux
Release:        rolling
Codename:       n/a

Stack trace
n/a

[albert-github]

Some thoughts:

• The user has specified the argument as int const or int volatile and this should thus be shown as well.

• The fact that a user defines the argument as int const and declares it as int is an inconsistency in the user code.

• The question that arises is: are arguments with type int const and int volatile allowed according to the C++ standard or are it extensions of the compilers?
  Note:
  when using the square example I can get some warnings:

  gcc -c  -Wpedantic --std=c++23 aa.cpp
  aa.cpp:2:38: warning: 'volatile'-qualified parameter is deprecated [-Wvolatile]
      2 | int square(int const x, int volatile y)
        |                         ~~~~~~~~~~~~~^
  

  So it looks like it is allowed according to the standard, leading to the question:

  • is there a reference / definition in the standard that the int const should be considered as int

[cjdb]

• The user has specified the argument as int const or int volatile and this should thus be shown as well.
• The fact that a user defines the argument as int const and declares it as int is an inconsistency in the user code.

I disagree. It doesn't create an inconsistency in user code: it's a feature of the language because declarations can be removed from definitions. When creating a proper declaration, there is no benefit to mentioning that a parameter is top-level cv-qualified because from the user's perspective, any argument passed in won't be modified. Whether or not it's modified in the definition is immaterial because the user will never experience that.

The question that arises is: are arguments with type int const and int volatile allowed according to the C++ standard or are it extensions of the compilers?
...
So it looks like it is allowed according to the standard

If you're interested in following up on this in the grammar, here's the first/follow set.

1. Function declaration start token
2. parameter-declaration-clause
3. parameter-declaration-list
4. parameter-declaration
5. declaratior
6. parameters-and-qualifiers
7. cv-qualifier-seq
8. cv-qualifier

is there a reference / definition in the standard that the int const should be considered as int

[dcl.fct]/p5 provides the standard wording for this section.

[albert-github]

The fact that a user defines the argument as int const and declares it as int is an inconsistency in the user code.

I think we have here a small misunderstanding what I meant is that the user uses 2 styles which by the compiler / language standard are mapped to the same function. For the reader it might be initially confusing whether or not e.g f(int x) and f(int const x) represent the same function or are different overloads and the user might also be confused when he cannot find his ''definition" in the documentation as a word disappeared.

Regarding the links, it would be better to refer here to the paragraphs / codes (like [dcl.fct]) in the draft pdf like N4950.pdf or to a specific version of the standard (links might change).

[doxygen]

@albert-github I do understand the bug report.

If you have a declaration

void foo(int x);

this is exactly the same as

void foo(const int x);

since x is a value type, the const is redundant.

However consider the following valid definition of either declaration:

void foo(int x) 
{ 
   x = 42; // this is ok, overwrites x
}

then the above is fine, but the same example with const is an error, not in the declaration, but in the implementation:

void foo(const int x)
{
  x = 42; // compiler error: x is const and cannot change.
}

So the const in the definition is part of the implementation, not of the user facing API, so one can argue if it should appear in the documentation or not. See also https://stackoverflow.com/questions/117293/use-of-const-for-function-parameters for more discussion on this topic.

Note that things can get a bit difficult for doxygen if you start to hide reference types behind typedefs or macros, e.g.

#if PASS_BY_VALUE
#define MyType int
#else
#define MyType int *
#endif

void foo(const MyType x); // is this const redundant or not? it depends...

[cjdb]

Note that things can get a bit difficult for doxygen if you start to hide reference types behind typedefs or macros

Clang's parser is able to detect type aliases, and there's even a Clang Tidy check.

Macros might be a bit more tricky. It might be possible in the above case, but this would be borderline impossible.

#if PASS_BY_VALUE
using MyType = int;
#else
using MyType = int*;
#endif

As much as I would prefer it to be not present by default, perhaps there should be an opt-in for this?

[albert-github]

@doxygen
That is very subtle. In the declaration which is for the user the const is redundant / not necessary but the behavior of the definition there is a difference between the version with and without the const.
I think that when using const in the declaration it is signaled to the end user that there is no, internal, change possible of the argument in the definition (though it would still be possible to use a local and thus changeable variable in the initial value of the argument).
In fact when the definition is locally changing the value of the non-const argument I think this is laziness of the implementer (not using a local variable) and should have been prohibited by the standard (now the compiler probably creates a local variable which otherwise would have been done by the user).
This remains a complicated and confusing business.

[cjdb]

@albert-github

I think that when using const in the declaration it is signaled to the end user that there is no, internal, change possible of the argument in the definition (though it would still be possible to use a local and thus changeable variable in the initial value of the argument).

It might mean what you've said. It might not. Perhaps the author could have originally intended this, then changed the definition so that the parameter is no longer const: now the documented declaration is stale. There's a good reason that tooling and best practices discourage top-level cv-qualifiers, and Doxygen be following said best practices: it eliminates this ambiguity.

Having said that, none of this addresses that the user does not benefit from knowing that a parameter is cv-qualified at the top level. My users have no need to know that I've chosen to const-qualify the parameters of std::is_eq. As it stands, one cannot do the following

/// Returns ``cmp == 0``.
constexpr bool is_eq(partial_ordering const cmp) noexcept
{
	return cmp == 0;
}

without is_eq being documented as is_eq(partial_ordering const cmp). That is not desirable, and as far as I'm aware, the only options I have to omit that const-qualifier are to:

1. create a forward declaration just for the purposes of documentation;
2. de-qualify cmp in the definition;
3. use some sort of macro to hide top-level cv-qualifiers from Doxygen;
4. run a post-processing tool to detect top-level const in the XML and remove it.

None of these are satisfactory solutions to the problem.

In fact when the definition is locally changing the value of the non-const argument I think this is laziness of the implementer (not using a local variable) and should have been prohibited by the standard

It's not lazy to pick one good name for something and then use that for the duration of the scope.

(now the compiler probably creates a local variable which otherwise would have been done by the user).

Not sure what you're talking about here. Compilers don't create locals when they don't need to, and optimised builds will remove local variables if they have the opportunity. Here's a demonstration.

[albert-github]

In fact when the definition is locally changing the value of the non-const argument I think this is laziness of the implementer (not using a local variable) and should have been prohibited by the standard

It's not lazy to pick one good name for something and then use that for the duration of the scope.

a good local variable could be local<originalNmae>

(now the compiler probably creates a local variable which otherwise would have been done by the user).

Not sure what you're talking about here. Compilers don't create locals when they don't need to, and optimised builds will remove local variables if they have the opportunity. Here's a demonstration.

What I meant to say with "probably creates a local variable" that it conceptually creates, not shown to the user, something like a local / scratch variable that is discarded when the routine / block is finished. How the compiler does this is immaterial, it can be on the stack, in a register etc. The compiler somewhere has to store temporarily the information.