[PATCH] D33680: [ELF] - Resolve references properly when using .symver directive

George Rimar via llvm-commits llvm-commits at lists.llvm.org
Thu Jun 15 12:35:33 PDT 2017


>> George Rimar via Phabricator <reviews at reviews.llvm.org> writes:
>>
>>>> You can do
>>>>
>>>>   for (size_t I = 0; I < SymVector.size(); ++I)
>>>>
>>>> to remove `DefaultV`, no?
>>> Yes, I tried something like that too when wrote it. I just found that its looks close to
>>> mutating arguments. When you debug a loop it is probably easier to understand logic when
>>> it's end is static. Having something like temp vector it is not ideal, but I would prefer to have temp vector than
>>> mutate loop bound probably.
>>
>> I think that removing DefaultV and having a comment on why we call
>> size() on each iteration is probably the best.
>
>Actually, since we just push_back, we can just use
>
> for (size_t I = 0, N = SymVector.size(); I < N; ++I)
>
>no?
>
>Cheers,
>Rafael

Yes, that is true for sure, but it complicates logic, no ?

Temp vector still keep it simple,
though if you think it is really better, I have probably no major arguments not to do that,
it is a matter of taste it seems.

Rui, will you be ok to do this change then ?

George.


More information about the llvm-commits mailing list