[PATCH] D12839: Extend MoveConstructorInitCheck to also flag constructor arguments passed by value and can be moved assigned to fields.

[Alexander Kornienko via cfe-commits]

alexfh added a comment.

In http://reviews.llvm.org/D12839#254384, @flx wrote:

> I changed the check to also produce a fix that wraps the argument in std::move().
>
> When I modified the test include -isystem %S/Inputs/Headers it broke and only produces warnings but no fixes anymore. Is there a subtle difference in how tests with includes need to be invoked?


Did you add the files you're including (`j.h`, `s.h`)? If not, this will result in compilation errors and clang-tidy by default won't apply fixes, if the code doesn't compile. In any case, they should be present in the patch as well.


================
Comment at: clang-tidy/misc/MoveConstructorInitCheck.cpp:110
@@ +109,3 @@
+          Result.SourceManager->getFileID(InitArg->getLocStart()), "utility",
+          true)) {
+    DiagOut << *IncludeFixit;
----------------
`/*IsAngled=*/true)) {`

================
Comment at: clang-tidy/modernize/PassByValueCheck.cpp:121
@@ -120,4 +120,3 @@
     : ClangTidyCheck(Name, Context),
-      IncludeStyle(Options.get("IncludeStyle", "llvm") == "llvm" ?
-                   IncludeSorter::IS_LLVM : IncludeSorter::IS_Google) {}
+      IncludeStyle(IncludeSorter::toIncludeStyle(Options.get("IncludeStyle", "llvm"))) {}
 
----------------
clang-format?

================
Comment at: clang-tidy/modernize/ReplaceAutoPtrCheck.cpp:191
@@ -190,5 +190,3 @@
     : ClangTidyCheck(Name, Context),
-      IncludeStyle(Options.get("IncludeStyle", "llvm") == "llvm"
-                       ? IncludeSorter::IS_LLVM
-                       : IncludeSorter::IS_Google) {}
+      IncludeStyle(IncludeSorter::toIncludeStyle(Options.get("IncludeStyle", "llvm"))) {}
 
----------------
clang-format?

================
Comment at: clang-tidy/utils/IncludeSorter.h:29
@@ +28,3 @@
+  // Converts "llvm" to IS_LLVM, otherwise returns IS_Google.
+  static IncludeStyle toIncludeStyle(const std::string &Value);
+
----------------
nit: The current name gives no hints at what is being converted to IncludeStyle, but `parseIncludeStyle` would make it clear that the source is a string.


http://reviews.llvm.org/D12839