[PATCH] Some code improvements (no functional change)

[Artyom Skrobov]

Chandler, the motivation for the renaming was as follows:


-----Original Message-----
From: Duncan P. N. Exon Smith [mailto:dexonsmith at apple.com]
Sent: 16 April 2014 20:10
To: Artyom Skrobov
Cc: llvm-commits
Subject: Re: [PATCH] Some code improvements (no functional change)





On 2014-Apr-16, at 11:30, Artyom Skrobov <Artyom.Skrobov at arm.com<mailto:Artyom.Skrobov at arm.com>> wrote:



> Hello,

>

> I suggest these three very simple improvements to LLVM source:

>

> 1)      Extracting the definition of SwapValue(), copypasted between lib/Object/MachOObjectFile.cpp and lib/Object/MachOUniversal.cpp, into include/llvm/Support/SwapByteOrder.h



This is fine in principal, but I'm not sure I like the name.  When the

function is local to a file, SwapValue() is clear enough, but once it's

API:  how do the names SwapValue() and SwapByteOrder() match up

together?  Why does the former swap the argument (taken by reference)

and the latter return a swapped copy of the argument (taken by value)?



I.e., you might be able to convince me with a better name.  But it's a

oneliner function with only two users, so the name had better be pretty

good.




From: Chandler Carruth [mailto:chandlerc at google.com]
Sent: 12 June 2014 16:43
To: Duncan P. N. Exon Smith
Cc: Artyom Skrobov; llvm-commits
Subject: Re: [PATCH] Some code improvements (no functional change)

I'm lacking context sadly. =/

On Thu, Jun 12, 2014 at 4:35 PM, Duncan P. N. Exon Smith <dexonsmith at apple.com<mailto:dexonsmith at apple.com>> wrote:
> On 2014 May 29, at 03:41, Artyom Skrobov <Artyom.Skrobov at arm.com<mailto:Artyom.Skrobov at arm.com>> wrote:
>
> I'd like to ask your opinion on naming the byte-swapping
> functions swapByteOrder for the in-place variants, and getBytesSwapped for
> the by-value/return variants; what do you think?
> In addition to adhering to the lowercase-initial convention advised in the
> Coding Standards, I believe this would also reflect more clearly what the
> functions are doing, than their current names of SwapValue and
> SwapByteOrder.

Generally we don't change the names of old functions that don't match the
current style guidelines (there's *a lot* of code that predates the style
guidelines, and this would cause a lot of churn).

Nevertheless, it'd be nice to have in-place variants.  I think something
like `SwapByteOrderInPlace()` or `ReplaceByteOrder()` might work with the
naming scheme in the file and minimize code churn.

+echristo,rafael,bogner

Anyone else have an opinion on this?  Personally, I prefer Artyom's
proposal (renaming `SwapByteOrder`).
_______________________________________________
llvm-commits mailing list
llvm-commits at cs.uiuc.edu<mailto:llvm-commits at cs.uiuc.edu>
http://lists.cs.uiuc.edu/mailman/listinfo/llvm-commits


-- IMPORTANT NOTICE: The contents of this email and any attachments are confidential and may also be privileged. If you are not the intended recipient, please notify the sender immediately and do not disclose the contents to any other person, use it for any purpose, or store or copy the information in any medium. Thank you.

ARM Limited, Registered office 110 Fulbourn Road, Cambridge CB1 9NJ, Registered in England & Wales, Company No: 2557590
ARM Holdings plc, Registered office 110 Fulbourn Road, Cambridge CB1 9NJ, Registered in England & Wales, Company No: 2548782
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.llvm.org/pipermail/llvm-commits/attachments/20140612/234f7a86/attachment.html>