Fix 14136: False positive: constParameterReference when using void cast

[pfultz2]

No description provided.

[sonarqubecloud]

Quality Gate passed

Issues
1 New issue
0 Accepted issues

Measures
0 Security Hotspots
0.0% Coverage on New Code
0.0% Duplication on New Code

See analysis details on SonarQube Cloud

[pfultz2]

@chrchr-github @danmar @orbitcowboy Can this be merged?

[chrchr-github]

https://trac.cppcheck.net/ticket/13754 basically asks for the opposite of what this PR does, so we should be clear about how we handle (void) going forward,

[firewave]

https://trac.cppcheck.net/ticket/13754 basically asks for the opposite of what this PR does, so we should be clear about how we handle (void) going forward,

That was about local variables.

In both cases it is problematic for our "no configuration" approach because this heavily depends on coding guidelines (albeit unnamed parameters might also interfere with documentation generators).

In some cases other tools might even behave different based on the annotation used (see #7810 (comment)).

Given that we had incomplete handling of those in other places we should double-check them (not the scope of this - might take a look in a few days if nobody else does).

[pfultz2]

We already skip warning when using the maybe_unused attribute, and when we void cast a pointer, so we should do that same for void casts for non pointer to make it consistent. Fixing the FP should be preferred over fixing a FN, we can try to work on a configuration to enable the FN in the future.

[firewave]

We already skip warning when using the maybe_unused attribute, and when we void cast a pointer, so we should do that same for void casts for non pointer to make it consistent.

The important thing for me is to be consistent (which we usually are not) and to stay consistent (we flip-flop sometimes).

Fixing the FP should be preferred over fixing a FN, we can try to work on a configuration to enable the FN in the future.

We should try not to add configuration.

[pfultz2]

The important thing for me is to be consistent (which we usually are not) and to stay consistent (we flip-flop sometimes).

It does make it more consistent, even though that is not a stated goal for cppcheck. And it also reaches our stated goal of "If the code is written as it is by design, then our goal is to not show any false positives".

We should try not to add configuration.

That is fine too, I guess I misunderstood your "it is problematic for our "no configuration" approach" comment.

Either way, this fixes false positives on valid code and so it should be merged in. We can discuss how we want to solve the FNs for this case in the future.

[danmar]

this looks good to me.

[danmar]

Either way, this fixes false positives on valid code and so it should be merged in. We can discuss how we want to solve the FNs for this case in the future.

I agree. And as you say it's also consistent with how we treat pointers.