[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