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

[Shuai Wang via Phabricator via cfe-commits]

shuaiwang marked 2 inline comments as done.
shuaiwang added inline comments.


================
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()));
+
----------------
JonasToth wrote:
> JonasToth wrote:
> > 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.
> Related: https://clang.llvm.org/extra/clang-tidy/checks/performance-move-const-arg.html
Added these tests:
* std::move copy only type doesn't mutate
* std::move move only type does mutate
* std::move copiable & movable type does mutate
* std::move const copiable & movable type doesn't mutate
Didn't test array because array can't be assigned (so copy/move isn't relevant) and passing array to function can only be done via reference (passing by value results in array to pointer decay) so no copy/move there as well.


Repository:
  rC Clang

https://reviews.llvm.org/D52120