[PATCH] Fix GraphTraits for "const CallGraphNode *" and "const CallGraph *"

Rafael EspĂ­ndola rafael.espindola at gmail.com
Tue Nov 11 10:26:10 PST 2014


The new patch is quiet a bit bigger, sorry for leading you that way.

As for the unit test, I intended for something far smaller. Basically
just a test showing that foo and bar in your original email now
compile.

Your original patch with a minimal uniti test is OK.


On 11 November 2014 13:19, Speziale Ettore <speziale.ettore at gmail.com> wrote:
> Gently ping
>
>
> On Thu, Nov 6, 2014 at 4:26 PM, Speziale Ettore <speziale.ettore at gmail.com>
> wrote:
>>
>> Hello,
>>
>>>
>>>
>>>> Is it possible to avoid the code duplication?
>>>
>>>
>>> Not sure about that ... I'll try to do some experiments ...
>>
>>
>> I tried to factorize the const CallGraphNode * with CallGraphNode *
>> specialization and const CallGraph * with CallGraph * specialization. I had
>> to introduce some templates/traits to support that. It is working, but I am
>> not sure if it is the correct approach ... -- I'm not very expert with
>> templates ....
>>
>>>>
>>>> Since this is just an API change, can you add a unittest?
>>
>>
>> Added some simple unit tests that iterate over all nodes in different call
>> graphs using GraphTraits iterators.
>>
>> In the process, I added a couple of member functions to CallGraph -- i.e.
>> CallGraph::size() -- and CallGraph::empty() -- to implement sanity checks in
>> the unit tests.
>>
>> To build the test call graphs I used an utility function I found in
>> LazyCallGraphTests.cpp. I moved that function to a separate file to be used
>> by both tests.
>>
>> I modified the CMake files in order to build and link the new tests, but I
>> am not very expert of it. I tried to grep the codebase and I did not find an
>> header file being included by any add_llvm_unittest, so I did not add
>> CallGraphTestUtils.h there. Is that OK?
>>
>> Thanks,
>> Ettore Speziale
>
>



More information about the llvm-commits mailing list