[PATCH] D28481: Respect section groups in GC

[Evgenii Stepanov via llvm-commits]

Thanks for the pointer. I've may have sent a message to generic-abi
(if you get two, ignore the first one).

On Wed, Jan 11, 2017 at 11:21 AM, Rafael Avila de Espindola
<rafael.espindola at gmail.com> wrote:
>
> Please don't commit it just yet. There are important undecided semantic
> issues to resolve.
>
> Rui Ueyama via Phabricator via llvm-commits
> <llvm-commits at lists.llvm.org> writes:
>
>> ruiu accepted this revision.
>> ruiu added a comment.
>> This revision is now accepted and ready to land.
>>
>> LGTM with nits.
>>
>>
>>
>> ================
>> Comment at: ELF/MarkLive.cpp:83
>>                               std::function<void(ResolvedReloc<ELFT>)> Fn) {
>>    if (Sec.AreRelocsRela) {
>>      for (const typename ELFT::Rela &Rel : Sec.relas())
>> ----------------
>> ruiu wrote:
>>> // Visit relocations.
>> This belongs to this entire `if { ~ } else { ~ }` and not to this `if` clause, so it should be written before `if`.
>>
>>
>> ================
>> Comment at: ELF/MarkLive.cpp:95
>> +  // Visit SHT_GROUP members if this section is in a group.
>> +  for (auto SecIdx : Sec.GroupSections) {
>> +    InputSectionBase<ELFT> *SameGroupSection =
>> ----------------
>> auto -> uint32_t
>>
>>
>> ================
>> Comment at: ELF/MarkLive.cpp:187
>> +  // https://github.com/google/sanitizers/issues/260 for more context.
>> +  if (Sec->GroupSections.size() > 0)
>> +    return false;
>> ----------------
>> nit: use `empty()` if ArrayRef has that function.
>>
>>
>> Repository:
>>   rL LLVM
>>
>> https://reviews.llvm.org/D28481
>>
>>
>>
>> _______________________________________________
>> llvm-commits mailing list
>> llvm-commits at lists.llvm.org
>> http://lists.llvm.org/cgi-bin/mailman/listinfo/llvm-commits