[llvm] r303490 - COFF: migrate def parser from LLD to LLVM [1/2]

[Rui Ueyama via llvm-commits]

I wouldn't bother to rename it as it seems an unfortunate consequence that
C++ doesn't have unnamed structs in unions. Actually, I don't think leaving
that kind of "TODO" comment is a good thing because it just pose a
question. We rather should do it or don't do it.

On Thu, Jun 8, 2017 at 5:13 PM, Martell Malone <martellmalone at gmail.com>
wrote:

> From Galinas' commit
>
>> // TODO: Name.Offset.Offset here and in the all similar places below
>> // suggests a names refactoring. Maybe StringTableOffset.Value?
>
>  See rL305029
>
> On Fri, Jun 9, 2017 at 1:10 AM, Rui Ueyama <ruiu at google.com> wrote:
>
>> Which names are you talking about?
>>
>> On Thu, Jun 8, 2017 at 5:02 PM, Martell Malone <martellmalone at gmail.com>
>> wrote:
>>
>>> LGTM
>>> Removing the cast with a note about naming makes a lot of sense here.
>>>
>>> Thanks Galina for finding a reasonable solution to fix the warnings.
>>> Rui, I can follow up with a commit on the naming if you have a
>>> suggestion?
>>>
>>> Best,
>>> Martell
>>>
>>> On Fri, Jun 9, 2017 at 12:38 AM, Galina Kistanova <gkistanova at gmail.com>
>>> wrote:
>>>
>>>> I have committed the fix as r305029.
>>>>
>>>> Naming there looks a bit odd, but this is for somebody else to address.
>>>> :)
>>>>
>>>> Thanks
>>>>
>>>> Galina
>>>>
>>>
>>>
>>
>
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.llvm.org/pipermail/llvm-commits/attachments/20170608/84a6a5a5/attachment.html>