[clang-tidy] False positive for cppcoreguidelines-rvalue-reference-param-not-moved when parameter has [[maybe_unused]]

[nilssonk]

The following seemingly valid code

#include <tuple>
#include <utility>

void conditional_use([[maybe_unused]] int && x) {
#ifdef USE_X
    int y = std::move(x);
    std::ignore = y;
#endif
}

int main() {
    conditional_use(5);
}

results in the error

repro.cpp:4:46: error: rvalue reference parameter 'x' is never moved from inside the function body [cppcoreguidelines-rvalue-reference-param-not-moved,-warnings-as-errors]
    4 | void conditional_use([[maybe_unused]] int && x) {

Encountered on

$ lsb_release -a
No LSB modules are available.
Distributor ID: Ubuntu
Description:    Ubuntu 22.04.3 LTS
Release:        22.04
Codename:       jammy

$ clang-tidy --version
Ubuntu LLVM version 17.0.2
  Optimized build.

installed with the script from https://apt.llvm.org/llvm.sh .

[AMS21]

I see the problem, but I'm not sure how we should generally handle these cases and if [[maybe_unused]] should really make a difference in these cases.

// No warning here
void f([[maybe_unused]] int&& x)
{}

// But give a warning here? Seems a bit inconsistent
void g(int&& x)
{}

// This should defiantly give a warning in my opinion
void f([[maybe_unused]] int&& x)
{
    int copy = x;
}

So I would argue to either leave the behavior as it is or simply ignore all empty functions for this check. However, there might be a better solution here that I don't see. Some additional input would be appreciated :)

[nilssonk]

What I would expect is that when [[maybe_unused]] is present, then this check is performed only if the parameter in question is used. I do not know if this is tricky to implement or not, but seeing as clang-tidy can detect unused variables in general, shouldn't it be possible to integrate with that?

Also generally speaking, the function can be non-empty, but it may be a noop w.r.t. the parameter marked as [[maybe_unused]]. E.g.

int conditional_use([[maybe_unused]] int && x, int y) {
    int z;

#ifdef USE_X
    z = std::move(x);
    z += y;
#else
    z = y;
#endif

    return z;
}

[AMS21]

Yeah I think that would sense.
The actual checking should be fairly easy with ParmVarDecls methods isUsed() and hasAttr<UnusedAttr>().
Still not sure if this should be behind an option or not. But will try to take a shot at this (hopefully) sometime this week.