[PATCH] D28481: Respect section groups in GC

[Rafael Avila de Espindola via llvm-commits]

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