[PATCH] Some code improvements (no functional change)

[Duncan P. N. Exon Smith]

On 2014-Apr-16, at 11:30, Artyom Skrobov <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.

> 2)      Extracting the definition of wrap(DataLayout) and unwrap(LLVMTargetDataRef), copypasted between lib/Target/Target.cpp, lib/Target/TargetMachineC.cpp, and  lib/ExecutionEngine/ExecutionEngineBindings.cpp, into include/llvm-c/Target.h; and also deleting the unused definitions of wrap(TargetLibraryInfo) and unwrap(LLVMTargetLibraryInfoRef) from lib/Target/TargetMachineC.cpp

I think this one is definitely out, though.  The C API is stable, so
we'd have to support this forever.  And you're putting C++ code into it,
which is probably just wrong.

If you can find an appropriate place in include/llvm/, maybe?

>  
> The three patches are independent, and none of them introduce any functional change.
>  
> OK to commit?
> <include-guards.patch><lib-Object-MachO.patch><lib-Target.patch>_______________________________________________
> llvm-commits mailing list
> llvm-commits at cs.uiuc.edu
> http://lists.cs.uiuc.edu/mailman/listinfo/llvm-commits