[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