On Tue, Jan 5, 2016 at 9:18 AM, Bartosz Kosiorek <gan...@poczta.onet.pl> wrote:
>
> Hi Ben.
>
> 2016-01-04 17:39 GMT+01:00 Ben Boeckel <ben.boec...@kitware.com>:
>>
>> The
>> big remaining problem is passing char* as an argument where functions do
>> std::string(arg) right away. I fixed all of those which did explicit
>> std::string conversions (via assignment or otherwise), but those which
>> are conditional should get std::string overloads to avoid a
>> malloc/copy/free penalty. There is a *lot* of work here though. The
>> branch I had been working on (which is now woefully out-of-date) is 100
>> commits long.

This improvement should be semi-automated with clang-tidy. We have
many functions that take `const char*` and are called by passing a
`string.c_str()`. The function signature should be changed to `const
std::string&` manually, but changing all the call sites should be done
automatically (ideally by kwrobot, periodically, on multiple
platforms).

Here is how to run clang-tidy on projects built with CMake:

$ cd <build dir>
$ cmake -DCMAKE_EXPORT_COMPILE_COMMANDS=ON <source dir>
$ run-clang-tidy.py -p <build dir> -fix
-checks='-*,readability-redundant-string-cstr' <source dir>/Source/

The run-clang-tidy.py script is here:
https://llvm.org/svn/llvm-project/clang-tools-extra/trunk/clang-tidy/tool/run-clang-tidy.py
The `-fix` argument tells clang-tidy to fix the violations inplace.
Without this argument the diagnostics are just printed.

I wanted to demonstrage the ability of the `redundant-string-cstr`
checker, but apparently there are no violations found.
I have attached a patch that was generated by the
`container-size-empty` checker.
Please don't apply it. Such changes should be performed by kwrobot.

You can output all possible checks with:

$ clang-tidy -p <build dir> -checks='*' -list-checks <source dir>

> That's great news.
> What is the branch name/link to these improvement?
> Is it possible to push these improvements partially?
>
> Maybe CMake community could continue working on that improvement?
> It will be great to create ticket, in which propose solution will be
> described.
>
> BTW: Do you know why the Xcode and MS Visual Studio is slower than CMake one
> ?
>
> Here is the benchmarks for my internal project:
>
> Xcode generation:
>    real 6m31.733s
>    user 4m51.862s
>    sys 0m20.268s
>
> Make generation:
>    real 4m45.089s
>    user 2m56.117s
>    sys 0m17.481s
>
> Ninja generation:
>   real 2m48.585s
>   user 2m30.712s
>   sys 0m6.313s

Generators for Xcode and Visual Studio have to generate more files.
From 46a5639a27bab0b227b457e1224c702dd143af33 Mon Sep 17 00:00:00 2001
From: Daniel Pfeifer <dan...@pfeifer-mail.de>
Date: Tue, 5 Jan 2016 13:28:16 +0100
Subject: [PATCH] demonstrate clang-tidy

---
 Source/CTest/cmCTestRunTest.cxx     | 4 ++--
 Source/CTest/cmCTestTestHandler.cxx | 2 +-
 2 files changed, 3 insertions(+), 3 deletions(-)

diff --git a/Source/CTest/cmCTestRunTest.cxx b/Source/CTest/cmCTestRunTest.cxx
index d108592..87e9f62 100644
--- a/Source/CTest/cmCTestRunTest.cxx
+++ b/Source/CTest/cmCTestRunTest.cxx
@@ -207,7 +207,7 @@ bool cmCTestRunTest::EndTest(size_t completed, size_t total, bool started)
     {
     bool success =
       !forceFail &&  (retVal == 0 ||
-      this->TestProperties->RequiredRegularExpressions.size());
+      !this->TestProperties->RequiredRegularExpressions.empty());
     if(this->TestProperties->SkipReturnCode >= 0
       && this->TestProperties->SkipReturnCode == retVal)
       {
@@ -584,7 +584,7 @@ void cmCTestRunTest::ComputeArguments()
              << std::endl);
 
   // Print any test-specific env vars in verbose mode
-  if (this->TestProperties->Environment.size())
+  if (!this->TestProperties->Environment.empty())
     {
     cmCTestLog(this->CTest, HANDLER_VERBOSE_OUTPUT, this->Index << ": "
                << "Environment variables: " << std::endl);
diff --git a/Source/CTest/cmCTestTestHandler.cxx b/Source/CTest/cmCTestTestHandler.cxx
index a8f983f..df91ac5 100644
--- a/Source/CTest/cmCTestTestHandler.cxx
+++ b/Source/CTest/cmCTestTestHandler.cxx
@@ -1318,7 +1318,7 @@ void cmCTestTestHandler::AttachFiles(cmXMLWriter& xml,
                                      cmCTestTestResult* result)
 {
   if(result->Status != cmCTestTestHandler::COMPLETED
-     && result->Properties->AttachOnFail.size())
+     && !result->Properties->AttachOnFail.empty())
     {
     result->Properties->AttachedFiles.insert(
       result->Properties->AttachedFiles.end(),
-- 
2.6.4

-- 

Powered by www.kitware.com

Please keep messages on-topic and check the CMake FAQ at: 
http://www.cmake.org/Wiki/CMake_FAQ

Kitware offers various services to support the CMake community. For more 
information on each offering, please visit:

CMake Support: http://cmake.org/cmake/help/support.html
CMake Consulting: http://cmake.org/cmake/help/consulting.html
CMake Training Courses: http://cmake.org/cmake/help/training.html

Visit other Kitware open-source projects at 
http://www.kitware.com/opensource/opensource.html

Follow this link to subscribe/unsubscribe:
http://public.kitware.com/mailman/listinfo/cmake-developers

Reply via email to