[PATCH] Some code improvements (no functional change)

[Artyom Skrobov]

Thank you all three for your comments!

Attaching the revised patch for the change, as described in Duncan's previous mail, for your review:

1) SwapValue() which had been copypasted between lib/Object/MachOObjectFile.cpp and lib/Object/MachOUniversal.cpp now extracted into include/llvm/Support/SwapByteOrder.h and renamed into swapByteOrder()
2) Tests for swapByteOrder() added into unittests/Support/SwapByteOrderTest.cpp
3) The more common use-case of applying SwapByteOrder() in-place replaced with a call to swapByteOrder() in lib/ExecutionEngine/RuntimeDyld/RuntimeDyldImpl.h, lib/MC/ELFObjectWriter.cpp, lib/Support/DataExtractor.cpp, include/llvm/ADT/Hashing.h, and include/llvm/Support/Endian.h
4) SwapByteOrder() renamed into getSwappedBytes(); the only two places where it had been used not in-place are lib/ProfileData/InstrProfReader.cpp and include/llvm/ProfileData/InstrProfReader.h (and its own unit test)

OK to commit it like this?


-----Original Message-----
From: Eric Christopher [mailto:echristo at gmail.com] 
Sent: 12 June 2014 21:01
To: Chandler Carruth
Cc: Duncan P. N. Exon Smith; llvm-commits; Artyom Skrobov
Subject: Re: [PATCH] Some code improvements (no functional change)

On Thu, Jun 12, 2014 at 8:54 AM, Chandler Carruth <chandlerc at google.com> wrote:
>
> On Thu, Jun 12, 2014 at 4:46 PM, Duncan P. N. Exon Smith
> <dexonsmith at apple.com> wrote:
>>
>> > On 2014 Jun 12, at 08:42, Chandler Carruth <chandlerc at google.com> wrote:
>> >
>> > I'm lacking context sadly. =/
>>
>> Idea is:
>>
>>   - SwapByteOrder takes args by value and returns swapped bytes.
>>   - Some users want to swap in place.
>>   - So, rename SwapByteOrder => getSwappedBytes.
>>   - Then, add swapByteOrder (which takes and modifies a reference).
>
>
> Anyways, to directly answer: this sounds reasonable to me, and if you're
> touching the code I would go ahead and fix the naming convention. The only
> time I really agree with touching code and preserving the old naming
> convention is when there is some huge interface convention that would
> suddenly become inconsistent. The canonical example is IRBuilder.
>

Agreed.

-eric
-------------- next part --------------
A non-text attachment was scrubbed...
Name: swapByteOrder.patch
Type: application/octet-stream
Size: 23112 bytes
Desc: not available
URL: <http://lists.llvm.org/pipermail/llvm-commits/attachments/20140613/46e89e18/attachment.obj>