[PATCH] extracting swapStruct into include/llvm/Support/MachO.h (no functional change)

Artyom Skrobov Artyom.Skrobov at arm.com
Wed Jul 9 03:19:59 PDT 2014


Thank you Duncan

>> While committing this, I realised that the instantiations of swapStruct()
in
>> tools/lld/lib/ReaderWriter/MachO/MachONormalizedFileBinaryUtils.h are
>> identical to the private implementations in
lib/Object/MachOObjectFile.cpp
>> 
>> Should perhaps these swapStruct()'s be extracted into
>> include/llvm/Support/MachO.h ?
>> Attaching the patch for review.
>
> Sorry, I've lost context on this -- can you re-attach the patch?

No problem -- attaching that patch again.

 
>>>> Index: lib/Object/MachOObjectFile.cpp
>>>> ===================================================================
>>>> --- lib/Object/MachOObjectFile.cpp	(revision 210891)
>>>> +++ lib/Object/MachOObjectFile.cpp	(working copy)
>>>> @@ -24,9 +24,6 @@
>>>> #include <cstring>
>>>> #include <limits>
>>>> 
>>>> -using namespace llvm;
>>>> -using namespace object;
>>>> -
>>> 
>>> This cleanup seems unrelated.  You should leave this out.
>> 
>> Indeed this wasn't related to extracting SwapValue out of that file; but
is
>> this cleanup any controversial?

You didn't comment on this one: MachOObjectFile.cpp has both "using
namespace llvm::object;" and "namespace llvm { namespace object { } }" one
right after the other.

Surely it should only have one out of the two?


> So I think `MachOUniversal.cpp` is correct as is.  Personally, I prefer
> reading/writing `using` clauses for nested namespaces as:
>
>    using namespace llvm;
>    using namespace llvm::object;

OK, I'm happy to leave MachOUniversal.cpp alone.
-------------- next part --------------
A non-text attachment was scrubbed...
Name: SwapStruct.patch
Type: application/octet-stream
Size: 20361 bytes
Desc: not available
URL: <http://lists.llvm.org/pipermail/llvm-commits/attachments/20140709/75d1838b/attachment.obj>


More information about the llvm-commits mailing list