Re: [PATCH] D13166: Create modernize-make-unique check.

2015-09-29 Thread Angel Garcia via cfe-commits
angelgarcia updated this revision to Diff 35949. angelgarcia added a comment. Add a comment. http://reviews.llvm.org/D13166 Files: clang-tidy/modernize/CMakeLists.txt clang-tidy/modernize/MakeUniqueCheck.cpp clang-tidy/modernize/MakeUniqueCheck.h clang-tidy/modernize/ModernizeTidyModule

Re: [PATCH] D13166: Create modernize-make-unique check.

2015-09-29 Thread Manuel Klimek via cfe-commits
klimek accepted this revision. klimek added a comment. This revision is now accepted and ready to land. apart from the comment, LG Comment at: clang-tidy/modernize/MakeUniqueCheck.cpp:67-70 @@ +66,6 @@ + SourceLocation ConstructCallEnd; + if (LAngle == StringRef::npos) { +

Re: [PATCH] D13166: Create modernize-make-unique check.

2015-09-28 Thread Angel Garcia via cfe-commits
angelgarcia updated this revision to Diff 35873. angelgarcia added a comment. Also replace on aliases. http://reviews.llvm.org/D13166 Files: clang-tidy/modernize/CMakeLists.txt clang-tidy/modernize/MakeUniqueCheck.cpp clang-tidy/modernize/MakeUniqueCheck.h clang-tidy/modernize/Modernize

Re: [PATCH] D13166: Create modernize-make-unique check.

2015-09-28 Thread Manuel Klimek via cfe-commits
klimek added a comment. In http://reviews.llvm.org/D13166#254730, @angelgarcia wrote: > This raises a question. Do we want to do replacements when we use an alias > for std::unique_ptr? That fact that something is an unique_ptr might be an > implementation detail that should not be exposed, but

Re: [PATCH] D13166: Create modernize-make-unique check.

2015-09-28 Thread Angel Garcia via cfe-commits
angelgarcia added a comment. This raises a question. Do we want to do replacements when we use an alias for std::unique_ptr? That fact that something is an unique_ptr might be an implementation detail that should not be exposed, but it could also happen that the alias is there only for brevity. Wh

Re: [PATCH] D13166: Create modernize-make-unique check.

2015-09-28 Thread Manuel Klimek via cfe-commits
klimek added inline comments. Comment at: clang-tidy/modernize/MakeUniqueCheck.cpp:75-76 @@ +74,4 @@ + std::string Identifier = removeWhitespace(ExprStr.substr(0, LAngle)); + if (Identifier != "unique_ptr" && Identifier != "std::unique_ptr") +return; + SourceLocation Constr

Re: [PATCH] D13166: Create modernize-make-unique check.

2015-09-28 Thread Angel Garcia via cfe-commits
angelgarcia updated this revision to Diff 35864. angelgarcia added a comment. I think this is a bit better than before. http://reviews.llvm.org/D13166 Files: clang-tidy/modernize/CMakeLists.txt clang-tidy/modernize/MakeUniqueCheck.cpp clang-tidy/modernize/MakeUniqueCheck.h clang-tidy/mo

Re: [PATCH] D13166: Create modernize-make-unique check.

2015-09-28 Thread Angel Garcia via cfe-commits
angelgarcia added a comment. How can Lexer::getSourceText give us the range? My problem precisely is that I couldn't find any way to obtain the range of the constructor call without the template arguments. http://reviews.llvm.org/D13166 ___ cfe-comm

Re: [PATCH] D13166: Create modernize-make-unique check.

2015-09-28 Thread Manuel Klimek via cfe-commits
klimek added a comment. In http://reviews.llvm.org/D13166#254520, @angelgarcia wrote: > Two tests in which 'getTokenLength' returns 0. Thanks for the tests - question is: I would have expected us to use something like Lexer::getSourceText, which should give us the full range in the first test

Re: [PATCH] D13166: Create modernize-make-unique check.

2015-09-28 Thread Angel Garcia via cfe-commits
angelgarcia updated this revision to Diff 35841. angelgarcia added a comment. Two tests in which 'getTokenLength' returns 0. http://reviews.llvm.org/D13166 Files: clang-tidy/modernize/CMakeLists.txt clang-tidy/modernize/MakeUniqueCheck.cpp clang-tidy/modernize/MakeUniqueCheck.h clang-ti

Re: [PATCH] D13166: Create modernize-make-unique check.

2015-09-28 Thread Manuel Klimek via cfe-commits
klimek added inline comments. Comment at: clang-tidy/modernize/MakeUniqueCheck.cpp:26-28 @@ +25,5 @@ +/// \brief Returns the length of the token that goes since the beggining of the +/// constructor call until the '<' of the template. This token should either be +/// 'unique_ptr'

Re: [PATCH] D13166: Create modernize-make-unique check.

2015-09-28 Thread Angel Garcia via cfe-commits
angelgarcia updated this revision to Diff 35839. angelgarcia marked an inline comment as done. angelgarcia added a comment. Remove 'hasCanonicalType'. http://reviews.llvm.org/D13166 Files: clang-tidy/modernize/CMakeLists.txt clang-tidy/modernize/MakeUniqueCheck.cpp clang-tidy/modernize/Ma

Re: [PATCH] D13166: Create modernize-make-unique check.

2015-09-28 Thread Angel Garcia via cfe-commits
angelgarcia added inline comments. Comment at: clang-tidy/modernize/MakeUniqueCheck.cpp:25-27 @@ +24,5 @@ + +/// \brief Returns the length of the token that goes since the beggining of the +/// constructor call until the '<' of the template. This token should either be +/// 'uniqu

Re: [PATCH] D13166: Create modernize-make-unique check.

2015-09-25 Thread Manuel Klimek via cfe-commits
klimek added a comment. This is definitely a useful check to have in modernize. Comment at: clang-tidy/modernize/MakeUniqueCheck.cpp:25-27 @@ +24,5 @@ + +/// \brief Returns the length of the token that goes since the beggining of the +/// constructor call until the '<' of the te

[PATCH] D13166: Create modernize-make-unique check.

2015-09-25 Thread Angel Garcia via cfe-commits
angelgarcia created this revision. angelgarcia added a reviewer: alexfh. angelgarcia added subscribers: klimek, cfe-commits. create a check that replaces 'std::unique_ptr(new type(args...))' with 'std::make_unique(args...)'. It was on the list of "Ideas for new Tools". It needs to be tested more