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

[Artyom Skrobov]

Hello Duncan,

Ping regarding that proposed change.

Also updating the email subject to be a bit more descriptive.


-----Original Message-----
From: Artyom Skrobov [mailto:Artyom.Skrobov at arm.com] 
Sent: 23 June 2014 15:37
To: 'Duncan P. N. Exon Smith'
Cc: llvm-commits
Subject: RE: [PATCH] Some code improvements (no functional change)

Thank you Duncan!
Committed as r210973--r210980, except for the namespace-related changes that
you pointed out.

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.


>> 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?


>> Index: lib/Object/MachOUniversal.cpp
>> ===================================================================
>> --- lib/Object/MachOUniversal.cpp	(revision 210891)
>> +++ lib/Object/MachOUniversal.cpp	(working copy)
>> @@ -19,30 +19,26 @@
>>  #include "llvm/Support/Host.h"
>>  #include "llvm/Support/MemoryBuffer.h"
>>  
>> -using namespace llvm;
>> -using namespace object;
>> +namespace llvm {
>> +namespace object {
>
> I don't see a reason to change the namespace setup, but even if it's a
> good idea it shouldn't be part of this commit.
>
>> @@ -165,3 +161,6 @@
>>    }
>>    return object_error::arch_not_found;
>>  }
>> +
>> +} // end namespace object
>> +} // end namespace llvm
>
> This is part of the unrelated change above; remove it too.

The only reason for that change was to be consistent with the other file.
Same as yourself, I'm not a big fan of "using" statements :-)

Is it OK to commit these two namespace changes?