[PATCH] D21507: Changes after running check modernize-use-emplace (D20964)

2017-02-02 Thread Alexander Kornienko via Phabricator via cfe-commits
alexfh requested changes to this revision. alexfh added a comment. This revision now requires changes to proceed. If you're still considering to submit this patch, could you rebase it (or maybe re-generate instead?) and split into easier to digest parts? A couple of things I noticed: 1.

Re: [PATCH] D21507: Changes after running check modernize-use-emplace (D20964)

2016-06-28 Thread Piotr Padlewski via cfe-commits
Prazek added a comment. In http://reviews.llvm.org/D21507#465733, @sanjoy wrote: > One other thing to point out -- have you verified that the changes in > `unittests/` are benign (i.e. you're not semantically changing the tests)? Yes I do. Repository: rL LLVM

Re: [PATCH] D21507: Changes after running check modernize-use-emplace (D20964)

2016-06-23 Thread Sanjoy Das via cfe-commits
sanjoy added a comment. One other thing to point out -- have you verified that the changes in `unittests/` are benign (i.e. you're not semantically changing the tests)? Repository: rL LLVM http://reviews.llvm.org/D21507 ___ cfe-commits mailing

Re: [PATCH] D21507: Changes after running check modernize-use-emplace (D20964)

2016-06-23 Thread Vedant Kumar via cfe-commits
vsk added a comment. In http://reviews.llvm.org/D21507#465444, @Prazek wrote: > In http://reviews.llvm.org/D21507#464791, @vsk wrote: > > > Neat! It would help to upload a git-clang-format'd. Fwiw I only managed to > > look over the changes in lib/{ARCMigrate,AST,Analysis}. > > > > Have you run

Re: [PATCH] D21507: Changes after running check modernize-use-emplace (D20964)

2016-06-23 Thread Piotr Padlewski via cfe-commits
Prazek added a comment. In http://reviews.llvm.org/D21507#464791, @vsk wrote: > Neat! It would help to upload a git-clang-format'd. Fwiw I only managed to > look over the changes in lib/{ARCMigrate,AST,Analysis}. > > Have you run check-all and the full test-suite? Yep, didn't have any

Re: [PATCH] D21507: Changes after running check modernize-use-emplace (D20964)

2016-06-22 Thread Vedant Kumar via cfe-commits
vsk added a subscriber: vsk. vsk added a comment. Neat! It would help to upload a git-clang-format'd. Fwiw I only managed to look over the changes in lib/{ARCMigrate,AST,Analysis}. Have you run check-all and the full test-suite? Repository: rL LLVM http://reviews.llvm.org/D21507

Re: [PATCH] D21507: Changes after running check modernize-use-emplace (D20964)

2016-06-20 Thread Piotr Padlewski via cfe-commits
Prazek added a comment. In http://reviews.llvm.org/D21507#462210, @jlebar wrote: > There seem to be many nontrivial whitespace errors introduced by this patch. > For example, > > -Attrs.push_back(HTMLStartTagComment::Attribute(Ident.getLocation(), > -

Re: [PATCH] D21507: Changes after running check modernize-use-emplace (D20964)

2016-06-20 Thread Justin Lebar via cfe-commits
jlebar added a subscriber: jlebar. jlebar added a comment. There seem to be many nontrivial whitespace errors introduced by this patch. For example, -Attrs.push_back(HTMLStartTagComment::Attribute(Ident.getLocation(), -