[clang-tools-extra] r217162 - Plug a unit test memory leak.

[Benjamin Kramer]

On Thu, Sep 4, 2014 at 7:05 PM, David Blaikie <dblaikie at gmail.com> wrote:
>
>
>
> On Thu, Sep 4, 2014 at 8:15 AM, Benjamin Kramer <benny.kra at googlemail.com>
> wrote:
>>
>> Author: d0k
>> Date: Thu Sep  4 10:15:27 2014
>> New Revision: 217162
>>
>> URL: http://llvm.org/viewvc/llvm-project?rev=217162&view=rev
>> Log:
>> Plug a unit test memory leak.
>>
>> MatchFinder.addMatcher doesn't take ownership.
>>
>> Modified:
>>     clang-tools-extra/trunk/unittests/clang-modernize/TransformTest.cpp
>>
>> Modified:
>> clang-tools-extra/trunk/unittests/clang-modernize/TransformTest.cpp
>> URL:
>> http://llvm.org/viewvc/llvm-project/clang-tools-extra/trunk/unittests/clang-modernize/TransformTest.cpp?rev=217162&r1=217161&r2=217162&view=diff
>>
>> ==============================================================================
>> --- clang-tools-extra/trunk/unittests/clang-modernize/TransformTest.cpp
>> (original)
>> +++ clang-tools-extra/trunk/unittests/clang-modernize/TransformTest.cpp
>> Thu Sep  4 10:15:27 2014
>> @@ -271,7 +271,8 @@ TEST(Transform, isFileModifiable) {
>>
>>    DummyTransform T("dummy", Options);
>>    MatchFinder Finder;
>> -  Finder.addMatcher(varDecl().bind("decl"), new ModifiableCallback(T));
>> +  ModifiableCallback Callback(T);
>> +  Finder.addMatcher(varDecl().bind("decl"), &Callback);
>
>
> Could/should we modify the addMatcher functions to take the second parameter
> by reference instead of pointer to reduce the risk of these leaks being
> written?
>
> (I actually started making this change & eventually realized it's a bit of a
> rat's-nest of Google-esque "pass non-const references by pointer" which is
> annoying)

addMatcher stores the pointer so we have to be careful about lifetime.
Changing it is probably not worth the churn :/

- Ben

>>
>>    Tool.run(tooling::newFrontendActionFactory(&Finder).get());
>>  }
>>
>>
>>
>> _______________________________________________
>> cfe-commits mailing list
>> cfe-commits at cs.uiuc.edu
>> http://lists.cs.uiuc.edu/mailman/listinfo/cfe-commits
>
>