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