[PATCH] Sema: Eliminate recursion from checkForFunctionCall()

Duncan P. N. Exon Smith dexonsmith at apple.com
Wed Jul 22 16:24:25 PDT 2015


> On 2015-Jul-22, at 14:55, Vedant Kumar <vsk at apple.com> wrote:
> 
> Part 2 of the Sema::checkForFunctionCall() patch attached.
> 
>> Probably a `SmallVector<..., 16>` or some such would make sense
>> instead of a `std::vector<...>`.
> 
> Done.
> 
>> I haven't seen `reference_wrapper<>` at all in LLVM, so I'm not sure
>> all the supported standard libraries have it available.  Please use
>> `CFGBlock*` instead.
> 
> Done.
> 
>> 
>>> +  Stack.emplace_back(std::make_pair(std::ref(Block), State));
>> 
>> With `emplace_back()`, you can call any constructor:
>> 
>>   Stack.emplace_back(&Block, State);
> 
> That makes things much cleaner.
> 
>> I see you've maintained the visitation order by reversing iteration.
>> Makes sense for this commit, but it might be nice to clean this up
>> in a follow-up (assuming visitation order doesn't matter here, which
>> I don't think it does?):
> 
> I maintained the old iteration order just to be extra careful.
> 
> Switching to a normal forward iterator doesn’t seem to affect anything. Aesthetically, this seems preferable. Something for a follow-up?

Yup, better for a follow-up.

> <checkForFunctionCall-2.patch>

This LGTM, once the prep patch is ready.



More information about the cfe-commits mailing list