[PATCH] D52120: [analyzer] Treat std::{move, forward} as casts in ExprMutationAnalyzer.

[Jonas Toth via Phabricator via cfe-commits]

JonasToth added a comment.

Thank you very much for the good work from my side as well!



================
Comment at: unittests/Analysis/ExprMutationAnalyzerTest.cpp:387
       match(withEnclosingCompound(declRefTo("x")), AST->getASTContext());
-  EXPECT_THAT(mutatedBy(Results, AST.get()), ElementsAre("std::move(x)"));
+  EXPECT_FALSE(isMutated(Results, AST.get()));
+
----------------
I am thinking about the correctness of the change.

If we have this case:
```
std::string s1 = "Hello World!";
std::string s2 = std::move(s1);
```
`s1` gets mutated, but it looks like it would not be considered as mutation?

On the other hand
```
int i1 = 42;
int i2 = std::move(i1);
```
should resolve in a copy `i1` and therefor not be a mutation.

Could you please add tests demonstrating this difference and the correct diagnostic detection for that.

Potentially interesting:
- Builtin types should be Copy-Only?, builtin arrays as expensive to move types for the completeness please as well
- Types with deleted move operations, should resolve in copy too
- Move-Only types like unique_ptr
- Types with both move and copy constructor (vector-like)
- and something with everything defaulted

These thoughts are just thinking and writing, I just wondered that moving a variable with std::move is not considered as mutation here, even though there are cases were it is actually a mutation.


Repository:
  rC Clang

https://reviews.llvm.org/D52120