Re: [PATCH] D14145: modernize-use-default supports copy constructor and copy-assignment operator.

2015-11-01 Thread Manuel Klimek via cfe-commits
klimek accepted this revision. klimek added a comment. This revision is now accepted and ready to land. lg http://reviews.llvm.org/D14145 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-comm

Re: [PATCH] D14145: modernize-use-default supports copy constructor and copy-assignment operator.

2015-10-29 Thread Angel Garcia via cfe-commits
angelgarcia updated this revision to Diff 38730. angelgarcia marked an inline comment as done. angelgarcia added a comment. Remove debugging code. http://reviews.llvm.org/D14145 Files: clang-tidy/modernize/UseDefaultCheck.cpp test/clang-tidy/modernize-use-default-copy.cpp test/clang-tidy/

Re: [PATCH] D14145: modernize-use-default supports copy constructor and copy-assignment operator.

2015-10-29 Thread Angel Garcia via cfe-commits
angelgarcia updated this revision to Diff 38728. angelgarcia added a comment. Put more logic into the matchers. Btw, do you know how can I get the exception specification of a function? http://reviews.llvm.org/D14145 Files: clang-tidy/modernize/UseDefaultCheck.cpp test/clang-tidy/modernize

Re: [PATCH] D14145: modernize-use-default supports copy constructor and copy-assignment operator.

2015-10-28 Thread Alexander Kornienko via cfe-commits
alexfh added inline comments. Comment at: test/clang-tidy/modernize-use-default.cpp:139 @@ +138,3 @@ + +struct O { + O() { Can we choose a different name for it, that does not resemble a digit? Comment at: test/clang-tidy/modernize-use-default.c

Re: [PATCH] D14145: modernize-use-default supports copy constructor and copy-assignment operator.

2015-10-28 Thread Manuel Klimek via cfe-commits
klimek added a comment. In http://reviews.llvm.org/D14145#277222, @angelgarcia wrote: > You can initialize an indirect base if it's virtual. Cool, those cases would be useful to have as comment, for example: // Make sure there are no additional initializations going on (for example, of indirec

Re: [PATCH] D14145: modernize-use-default supports copy constructor and copy-assignment operator.

2015-10-28 Thread Angel Garcia via cfe-commits
angelgarcia added a comment. You can initialize an indirect base if it's virtual. http://reviews.llvm.org/D14145 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

Re: [PATCH] D14145: modernize-use-default supports copy constructor and copy-assignment operator.

2015-10-28 Thread Manuel Klimek via cfe-commits
klimek added a comment. In http://reviews.llvm.org/D14145#277202, @angelgarcia wrote: > > In which cases can this be false? (I assume if not all are copied?) > > > This can be false is there is we do anything else than initializing the > bases and members. So the previous code is there to ensure

Re: [PATCH] D14145: modernize-use-default supports copy constructor and copy-assignment operator.

2015-10-28 Thread Angel Garcia via cfe-commits
angelgarcia added a comment. > In which cases can this be false? (I assume if not all are copied?) This can be false is there is we do anything else than initializing the bases and members. So the previous code is there to ensure that we do that and this is here to ensure that we *only* do that.

Re: [PATCH] D14145: modernize-use-default supports copy constructor and copy-assignment operator.

2015-10-28 Thread Manuel Klimek via cfe-commits
klimek added inline comments. Comment at: clang-tidy/modernize/UseDefaultCheck.cpp:83-91 @@ +82,11 @@ + const CXXMethodDecl *Method) { + if (Method->getNumParams() != 1) +return false; + QualType ArgType = Method->getParamDecl(0)->getType(); + +

Re: [PATCH] D14145: modernize-use-default supports copy constructor and copy-assignment operator.

2015-10-28 Thread Angel Garcia via cfe-commits
angelgarcia updated this revision to Diff 38664. angelgarcia added a comment. Refactor most of the code to use AST matchers. Note that now this change requires http://reviews.llvm.org/D14152 to be applied first. http://reviews.llvm.org/D14145 Files: clang-tidy/modernize/UseDefaultCheck.cpp

Re: [PATCH] D14145: modernize-use-default supports copy constructor and copy-assignment operator.

2015-10-28 Thread Manuel Klimek via cfe-commits
Thanks! On Wed, Oct 28, 2015 at 4:51 AM Angel Garcia wrote: > angelgarcia added a comment. > > OK, thanks! I will try to refactor some of the parts into AST matchers > then. It will probably take some time, though. > > > http://reviews.llvm.org/D14145 > > > >

Re: [PATCH] D14145: modernize-use-default supports copy constructor and copy-assignment operator.

2015-10-28 Thread Angel Garcia via cfe-commits
angelgarcia added a comment. OK, thanks! I will try to refactor some of the parts into AST matchers then. It will probably take some time, though. http://reviews.llvm.org/D14145 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.ll

Re: [PATCH] D14145: modernize-use-default supports copy constructor and copy-assignment operator.

2015-10-28 Thread Manuel Klimek via cfe-commits
klimek added a comment. Generally, I feel like a lot of the code would be written better as ast matchers (by adding new matchers if necessary). You can use tooling::ast_matchers::matches if you want to do a match inside a callback. Comment at: clang-tidy/modernize/UseDefaultCh

Re: [PATCH] D14145: modernize-use-default supports copy constructor and copy-assignment operator.

2015-10-28 Thread Angel Garcia via cfe-commits
angelgarcia updated this revision to Diff 38648. angelgarcia added a comment. Small tweak. http://reviews.llvm.org/D14145 Files: clang-tidy/modernize/UseDefaultCheck.cpp test/clang-tidy/modernize-use-default-copy.cpp test/clang-tidy/modernize-use-default.cpp Index: test/clang-tidy/modern

[PATCH] D14145: modernize-use-default supports copy constructor and copy-assignment operator.

2015-10-28 Thread Angel Garcia via cfe-commits
angelgarcia created this revision. angelgarcia added a reviewer: klimek. angelgarcia added subscribers: alexfh, cfe-commits. the check will now warn when the user provided definitions of this functions is equivalent to the explicitly defaulted ones. http://reviews.llvm.org/D14145 Files: clang