[PATCH] D12839: Extend MoveConstructorInitCheck to also flag constructor arguments passed by value and can be moved assigned to fields.
Alexander Kornienko via cfe-commits
cfe-commits at lists.llvm.org
Mon Sep 28 07:14:06 PDT 2015
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
More information about the cfe-commits
mailing list