aaron.ballman updated this revision to Diff 32697.
aaron.ballman added a comment.
In http://reviews.llvm.org/D11784#228764, @alexfh wrote:
In http://reviews.llvm.org/D11784#227939, @aaron.ballman wrote:
Addressed review comments. I re-ran the updated patch against LLVM and
Clang, and
alexfh accepted this revision.
alexfh added a comment.
This revision is now accepted and ready to land.
With the triviality implementation, I now get zero false positives (and zero
true positives) in the Clang and LLVM source base.
Awesome! Thanks for working on this!
LGTM
alexfh added a comment.
Sorry for the long delay.
Comment at: clang-tidy/misc/MoveConstructorInitCheck.cpp:40
@@ +39,3 @@
+ for (const auto *Ctor : CopyCtor-getParent()-ctors()) {
+if (Ctor-isMoveConstructor()
+Ctor-getAccess() = AS_protected
aaron.ballman updated this revision to Diff 32559.
aaron.ballman added a comment.
Addressed review comments. I re-ran the updated patch against LLVM and Clang,
and there were some more false positives that I would like to address if
possible. It seems my previous run against the source base was
aaron.ballman marked 5 inline comments as done.
aaron.ballman added a comment.
http://reviews.llvm.org/D11784
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
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:
aaron.ballman updated this revision to Diff 32175.
aaron.ballman added a comment.
Updated the patch to handle member initializers as well as base class
initializers. This changes the name of things from base to init as well.
http://reviews.llvm.org/D11784
Files:
aaron.ballman added a comment.
In http://reviews.llvm.org/D11784#224430, @alexfh wrote:
In http://reviews.llvm.org/D11784#224421, @aaron.ballman wrote:
In http://reviews.llvm.org/D11784#224386, @alexfh wrote:
One thing I am not certain of in this patch is how to test it. I have
alexfh added a comment.
In http://reviews.llvm.org/D11784#224433, @aaron.ballman wrote:
In http://reviews.llvm.org/D11784#224430, @alexfh wrote:
In http://reviews.llvm.org/D11784#224421, @aaron.ballman wrote:
In http://reviews.llvm.org/D11784#224386, @alexfh wrote:
Ah,
alexfh added a comment.
In http://reviews.llvm.org/D11784#224421, @aaron.ballman wrote:
In http://reviews.llvm.org/D11784#224386, @alexfh wrote:
One thing I am not certain of in this patch is how to test it. I have
some rudimentary tests, but am unable to test the note: diagnostics
alexfh added a comment.
In http://reviews.llvm.org/D11784#220654, @aaron.ballman wrote:
Ping?
Sorry for the delay. I'm on vacation currently. Will be back tomorrow.
- Alex
http://reviews.llvm.org/D11784
___
cfe-commits mailing list
aaron.ballman added a comment.
Ping?
FWIW, this patch almost caught a bug in LLVM. ;-) DependenceAnalysis.h has a
class: FullDependence which would suffer from this problem if the Dependence
base class did not accidentally suppress creation of the move constructor by
defaulting only the copy
12 matches
Mail list logo
