[clang-tidy] False positive bugprone-use-after-move with std::tie

[dmpolukhin]

Minimal reproducer https://godbolt.org/z/PoboWj4PM:

#include <tuple>
#include <utility>
#include <string>

std::pair<std::string, std::string> foo(std::string a, std::string b) {
  return std::make_pair(std::move(a), std::move(b));
}

void bar(std::string a, std::string b) {
  std::tie(a, b) = foo(std::move(a), std::move(b));
  std::tie(a, b) = foo(std::move(a), std::move(b));
}

/**
test.cpp:11:34: warning: 'a' used after it was moved [bugprone-use-after-move]
   11 |   std::tie(a, b) = foo(std::move(a), std::move(b));
      |                                  ^
test.cpp:10:24: note: move occurred here
   10 |   std::tie(a, b) = foo(std::move(a), std::move(b));
      |                        ^
test.cpp:11:48: warning: 'b' used after it was moved [bugprone-use-after-move]
   11 |   std::tie(a, b) = foo(std::move(a), std::move(b));
      |                                                ^
test.cpp:10:38: note: move occurred here
   10 |   std::tie(a, b) = foo(std::move(a), std::move(b));
      |                                      ^
**/

[llvmbot]

@llvm/issue-subscribers-clang-tidy

Author: Dmitry Polukhin (dmpolukhin)

Minimal reproducer https://godbolt.org/z/PoboWj4PM: ```c++ #include <tuple> #include <utility> #include <string>

std::pair<std::string, std::string> foo(std::string a, std::string b) {
return std::make_pair(std::move(a), std::move(b));
}

void bar(std::string a, std::string b) {
std::tie(a, b) = foo(std::move(a), std::move(b));
std::tie(a, b) = foo(std::move(a), std::move(b));
}

/**
test.cpp:11:34: warning: 'a' used after it was moved [bugprone-use-after-move]
11 | std::tie(a, b) = foo(std::move(a), std::move(b));
| ^
test.cpp:10:24: note: move occurred here
10 | std::tie(a, b) = foo(std::move(a), std::move(b));
| ^
test.cpp:11:48: warning: 'b' used after it was moved [bugprone-use-after-move]
11 | std::tie(a, b) = foo(std::move(a), std::move(b));
| ^
test.cpp:10:38: note: move occurred here
10 | std::tie(a, b) = foo(std::move(a), std::move(b));
| ^
**/

</details>

[chrchr-github]

Reduced:

#include <string>
#include <iostream>

std::string foo(std::string a) {
  a += "a";
  return a;
}

std::string bar(std::string a) {
  std::string& r1 = a;
  r1 = foo(std::move(a));
  std::string& r2 = a;
  r2 = foo(std::move(a));
  return std::move(a);
}

int main() {
    std::string a = bar("1");
    std::cout << a << "\n";
}

<source>:12:21: warning: 'a' used after it was moved [bugprone-use-after-move]
   12 |   std::string& r2 = a;
      |                     ^
<source>:11:12: note: move occurred here]
   11 |   r1 = foo(std::move(a));
      |            ^
<source>:14:20: warning: 'a' used after it was moved [bugprone-use-after-move]
   14 |   return std::move(a);
      |                    ^
<source>:13:12: note: move occurred here]
   13 |   r2 = foo(std::move(a));
      |            ^

https://godbolt.org/z/Wvz7KEqo1

[vbvictor]

In reduced version, i guess that warning is generated intentionally. As doc says

Any call of std::move on a variable is considered to cause a move of that variable, even if the result of std::move is not passed to an rvalue reference parameter.
This means that the check will flag a use-after-move even on a type that does not define a move constructor or move assignment operator. This is intentional.

So I guess the check has a simple pattern: if variable used in any way after std::move - it is flagged by the check

[dmpolukhin]

In reduced version, i guess that warning is generated intentionally. As doc says

Any call of std::move on a variable is considered to cause a move of that variable, even if the result of std::move is not passed to an rvalue reference parameter.
This means that the check will flag a use-after-move even on a type that does not define a move constructor or move assignment operator. This is intentional.

So I guess the check has a simple pattern: if variable used in any way after std::move - it is flagged by the check

I agree reduction is not correct. Original code is false positive because assign to std::tie should mark variable as non-moved.

[vbvictor]

In the original code it probably should count as reinitialization and should not produce warning, but my guess we only look for direct assignments to the variable (not counting std::tie and etc)

[chrchr-github]

Isn't the reduced code functionally equivalent, only using a reference directly instead of a tuple of references?

[vbvictor]

Yes, now to me it looks almost the same. However, std::tie case may be easier to implement because we can match std::tie calls and "magically" know that the variable a will be reassigned. With references some more work should be done.