[PATCH] D13904: [ELF2] .shstrtab section implemented

[Davide Italiano via llvm-commits]

On Tue, Oct 20, 2015 at 10:37 AM, George Rimar <grimar at accesssoftek.com> wrote:
>>От: davide.italiano at gmail.com [davide.italiano at gmail.com] от имени Davide Italiano [davide at freebsd.org]
>>Отправлено: 20 октября 2015 г. 21:21
>>Кому: reviews+D13904+public+03561865476a15e1 at reviews.llvm.org; George Rimar
>>Копия: Rafael Ávila de Espíndola; Davide Italiano; Rui Ueyama; llvm-commits
>>Тема: Re: [PATCH] D13904: [ELF2] .shstrtab section implemented
>>
>>On Tue, Oct 20, 2015 at 10:14 AM, George Rimar via llvm-commits
>><llvm-commits at lists.llvm.org> wrote:
>>> grimar added inline comments.
>>>
>>> ================
>>> Comment at: test/elf2/basic-mips.s:4
>>> @@ -3,3 +3,3 @@
>>>  # RUN: llvm-readobj -file-headers -sections -program-headers -symbols %t.exe \
>>> -# RUN:   | FileCheck %s
>>> +# RUN:   > C:\access_softek\temps\out.txt
>>>
>>> ----------------
>>> davide wrote:
>>>> what's this?
>>> Oops. Will fix before commit.
>>>
>>> ================
>>> Comment at: test/elf2/basic.s:192
>>> @@ -191,3 +205,2 @@
>>>
>>> -
>>>  # Test for the response file
>>> ----------------
>>> davide wrote:
>>>> Also this newline can be avoided.
>>> Will fix.
>>>
>>>
>>
>>Thank you. Sorry to bother you one more time, but I saw you removed
>>the dump of the index in the string table for many symbols.
>>
>>e.g.
>># CHECK-NEXT: Name: _start (7)
>>became
>># CHECK-NEXT: Name: _start
>>
>>I silently assumed you did this because indices changed after the test.
>>If yes, it's fine. Otherwise, if some of them are just cosmetic,
>>commit separately.
>>
>>Moreover, in basic-mips.s still .bss has the index dumped (not sure if
>>it's an oversight), as well as .strtab in basic64be.s
>>
>>
>>
>>--
>>Davide
>>
>>"There are no solved problems; there are only problems that are more
>>or less solved" -- Henri Poincare
>
> Yes, some of them were changed after test, but some another probably is just cosmetics. Sorry I was read your mail after commiting already.
> I did not think that is so important. I can revert and commit separately if needed, or leave as is.

No need to revert I think, but unrelated changes make the patch less
readable. Just a good practice to make our life easier if in 6 months
we look at this again, not a mandatory requirement.

-- 
Davide

"There are no solved problems; there are only problems that are more
or less solved" -- Henri Poincare