JonasToth 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:
> 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
Repository:
rC Clang
https://reviews.llvm.org/D52120
_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
