[PATCH] Some code improvements (no functional change)

Artyom Skrobov Artyom.Skrobov at arm.com
Mon Jun 23 07:37:13 PDT 2014


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?
-------------- 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/20140623/36cfaf3e/attachment.obj>


More information about the llvm-commits mailing list