[PATCH] Some code improvements (no functional change)
Artyom.Skrobov at arm.com
Fri Jun 13 03:59:04 PDT 2014
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?
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.
-------------- next part --------------
A non-text attachment was scrubbed...
Size: 23112 bytes
Desc: not available
More information about the llvm-commits