aaron.ballman added inline comments.
================
Comment at: clang-tidy/misc/MoveConstructorInitCheck.cpp:40
@@ +39,3 @@
+ for (const auto *Ctor : CopyCtor->getParent()->ctors()) {
+ if (Ctor->isMoveConstructor() &&
+ Ctor->getAccess() <= AS_protected &&
----------------
alexfh wrote:
> clang-format?
I thought the current formatting was an improvement over what clang-format
produces (for those of us with debuggers that aren't as good at subexpression
highlighting). I'm fine either way, though.
================
Comment at: clang-tidy/misc/MoveConstructorInitCheck.cpp:46
@@ +45,3 @@
+ //
+ // FIXME: Determine whether the move constructor is a viable candidate
+ // for the ctor-initializer, perhaps provide a fixit that suggests
----------------
alexfh wrote:
> This seems to be rather important to do from the beginning. Otherwise the
> check may be too noisy. BTW, did you run it over LLVM and Clang sources?
> Would be useful for some smoke testing.
In order to do that, I would need access to more parts of Sema. The check, as
it currently stands, is fairly reasonable from what I can tell. The false
positive rate appears to be low. I ran it over Clang and LLVM and it did point
out one debatably-true-positive (which we've since resolved) with no
false-positives. In testing other code bases, the diagnostic was not chatty,
but perhaps they did not make heavy use of move semantics.
================
Comment at: test/clang-tidy/misc-move-constructor-init.cpp:64
@@ +63,3 @@
+struct J : I {
+ // CHECK-NOT: warning:
+ J(J &&RHS) : I(RHS) {} // ok
----------------
alexfh wrote:
> I'd suggest using FileCheck -implicit-check-not='{{warning|error}}:' instead
> of stuffing the code with `// CHECK-NOT: warning:`. It will make the test
> more consistent with the other tests that use the clang_tidy_test.sh script.
Can do (though I am explicitly not using clang_tidy_test.sh because I am
working on Windows and all those tests are currently disabled due to REQUIRES:
shell :-()
http://reviews.llvm.org/D11784
_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
