Tool to fix calls to scoped_ptr<T>(NULL) to use the default ctor instead

Tool to fix calls to scoped_ptr<T>(NULL) to use the default ctor instead

BUG=173286
R=thakis@chromium.org

Committed: https://src.chromium.org/viewvc/chrome?view=rev&revision=205621

[dcheng, 2013-06-08 15:39:17]

Time to finish the job...

[Nico, 2013-06-11 19:58:12]

Nice!

https://codereview.chromium.org/16606002/diff/2001/tools/clang/rewrite_scoped...
File tools/clang/rewrite_scoped_ptr_ctor_null/tests/test-expected.cc (right):

https://codereview.chromium.org/16606002/diff/2001/tools/clang/rewrite_scoped...
tools/clang/rewrite_scoped_ptr_ctor_null/tests/test-expected.cc:5: #include
"base/memory/scoped_ptr.h"
Hm. One day, someone will move scoped_ptr to namespace base and the test will
stop running. Is it possible to make this not depend on other files?

Then the py change wouldn't be necessary either I think.

https://codereview.chromium.org/16606002/diff/2001/tools/clang/rewrite_scoped...
File tools/clang/rewrite_scoped_ptr_ctor_null/tests/test-original.cc (right):

https://codereview.chromium.org/16606002/diff/2001/tools/clang/rewrite_scoped...
tools/clang/rewrite_scoped_ptr_ctor_null/tests/test-original.cc:13:
scoped_ptr<int>* a = new scoped_ptr<int>(NULL),
Do you want to check `scoped_ptr<int>( (NULL) )` somewhere too?

https://codereview.chromium.org/16606002/diff/2001/tools/clang/scripts/test_t...
File tools/clang/scripts/test_tool.py (right):

https://codereview.chromium.org/16606002/diff/2001/tools/clang/scripts/test_t...
tools/clang/scripts/test_tool.py:18: def _GenerateCompileCommands(files,
include_paths):
fyi: ninja 1.3.4 (now in depot_tools) now has a -t compdb tool for writing
compile dbs. Not sure if that could help.

[dcheng, 2013-06-11 20:07:41]

https://codereview.chromium.org/16606002/diff/2001/tools/clang/rewrite_scoped...
File tools/clang/rewrite_scoped_ptr_ctor_null/tests/test-expected.cc (right):

https://codereview.chromium.org/16606002/diff/2001/tools/clang/rewrite_scoped...
tools/clang/rewrite_scoped_ptr_ctor_null/tests/test-expected.cc:5: #include
"base/memory/scoped_ptr.h"
On 2013/06/11 19:58:13, Nico wrote:
> Hm. One day, someone will move scoped_ptr to namespace base and the test will
> stop running. Is it possible to make this not depend on other files?
> 
> Then the py change wouldn't be necessary either I think.

Originally, that's what I did--I just had a minimal definition of scoped_ptr in
this file. But the problem is AST matching is somewhat fragile, especially when
temporaries are involved. In the end, I figured it'd be more robust to just have
it depend on scoped_ptr directly.

Also, this is essentially a one-off tool; once all existing violations are
fixed, I plan to make this usage a compile error. After that, I might remove
this tool (or just leave it, so people have an example to follow).

https://codereview.chromium.org/16606002/diff/2001/tools/clang/rewrite_scoped...
File tools/clang/rewrite_scoped_ptr_ctor_null/tests/test-original.cc (right):

https://codereview.chromium.org/16606002/diff/2001/tools/clang/rewrite_scoped...
tools/clang/rewrite_scoped_ptr_ctor_null/tests/test-original.cc:13:
scoped_ptr<int>* a = new scoped_ptr<int>(NULL),
On 2013/06/11 19:58:13, Nico wrote:
> Do you want to check `scoped_ptr<int>( (NULL) )` somewhere too?

I think that's covered by lines 30-33?

https://codereview.chromium.org/16606002/diff/2001/tools/clang/scripts/test_t...
File tools/clang/scripts/test_tool.py (right):

https://codereview.chromium.org/16606002/diff/2001/tools/clang/scripts/test_t...
tools/clang/scripts/test_tool.py:18: def _GenerateCompileCommands(files,
include_paths):
On 2013/06/11 19:58:13, Nico wrote:
> fyi: ninja 1.3.4 (now in depot_tools) now has a -t compdb tool for writing
> compile dbs. Not sure if that could help.

The problem is these test files don't end up in a gyp anywhere.

[Nico, 2013-06-11 20:18:28]

lgtm then :-)


On Tue, Jun 11, 2013 at 1:07 PM, <dcheng@chromium.org> wrote:

>
> https://codereview.chromium.**org/16606002/diff/2001/tools/**
>
clang/rewrite_scoped_ptr_ctor_**null/tests/test-expected.cc<https://codereview.chromium.org/16606002/diff/2001/tools/clang/rewrite_scoped_ptr_ctor_null/tests/test-expected.cc>
> File tools/clang/rewrite_scoped_**ptr_ctor_null/tests/test-**expected.cc
> (right):
>
> https://codereview.chromium.**org/16606002/diff/2001/tools/**
>
clang/rewrite_scoped_ptr_ctor_**null/tests/test-expected.cc#**newcode5<https://codereview.chromium.org/16606002/diff/2001/tools/clang/rewrite_scoped_ptr_ctor_null/tests/test-expected.cc#newcode5>
> tools/clang/rewrite_scoped_**ptr_ctor_null/tests/test-**expected.cc:5:
> #include "base/memory/scoped_ptr.h"
> On 2013/06/11 19:58:13, Nico wrote:
>
>> Hm. One day, someone will move scoped_ptr to namespace base and the
>>
> test will
>
>> stop running. Is it possible to make this not depend on other files?
>>
>
>  Then the py change wouldn't be necessary either I think.
>>
>
> Originally, that's what I did--I just had a minimal definition of
> scoped_ptr in this file. But the problem is AST matching is somewhat
> fragile, especially when temporaries are involved. In the end, I figured
> it'd be more robust to just have it depend on scoped_ptr directly.
>
> Also, this is essentially a one-off tool; once all existing violations
> are fixed, I plan to make this usage a compile error. After that, I
> might remove this tool (or just leave it, so people have an example to
> follow).
>
>
> https://codereview.chromium.**org/16606002/diff/2001/tools/**
>
clang/rewrite_scoped_ptr_ctor_**null/tests/test-original.cc<https://codereview.chromium.org/16606002/diff/2001/tools/clang/rewrite_scoped_ptr_ctor_null/tests/test-original.cc>
> File tools/clang/rewrite_scoped_**ptr_ctor_null/tests/test-**original.cc
> (right):
>
> https://codereview.chromium.**org/16606002/diff/2001/tools/**
>
clang/rewrite_scoped_ptr_ctor_**null/tests/test-original.cc#**newcode13<https://codereview.chromium.org/16606002/diff/2001/tools/clang/rewrite_scoped_ptr_ctor_null/tests/test-original.cc#newcode13>
> tools/clang/rewrite_scoped_**ptr_ctor_null/tests/test-**original.cc:13:
> scoped_ptr<int>* a = new scoped_ptr<int>(NULL),
> On 2013/06/11 19:58:13, Nico wrote:
>
>> Do you want to check `scoped_ptr<int>( (NULL) )` somewhere too?
>>
>
> I think that's covered by lines 30-33?
>
>
> https://codereview.chromium.**org/16606002/diff/2001/tools/**
>
clang/scripts/test_tool.py<https://codereview.chromium.org/16606002/diff/2001/tools/clang/scripts/test_tool.py>
> File tools/clang/scripts/test_tool.**py (right):
>
> https://codereview.chromium.**org/16606002/diff/2001/tools/**
>
clang/scripts/test_tool.py#**newcode18<https://codereview.chromium.org/16606002/diff/2001/tools/clang/scripts/test_tool.py#newcode18>
> tools/clang/scripts/test_tool.**py:18: def _GenerateCompileCommands(**
> files,
> include_paths):
> On 2013/06/11 19:58:13, Nico wrote:
>
>> fyi: ninja 1.3.4 (now in depot_tools) now has a -t compdb tool for
>>
> writing
>
>> compile dbs. Not sure if that could help.
>>
>
> The problem is these test files don't end up in a gyp anywhere.
>
>
https://codereview.chromium.**org/16606002/<https://codereview.chromium.org/1...
>

[dcheng, 2013-06-11 20:30:54]

Committed patchset #2 manually as r205621 (presubmit successful).