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

Michael Spencer bigcheesegs at gmail.com
Fri Jul 18 10:34:41 PDT 2014


On Fri, Jul 18, 2014 at 9:04 AM, Duncan P. N. Exon Smith
<dexonsmith at apple.com> wrote:
>
>> On 2014-Jul-18, at 03:08, Artyom Skrobov <Artyom.Skrobov at arm.com> wrote:
>>
>> Would you be able to also comment on the other small patch I proposed,
>> deleting the two unnecessary "using" statements, as quoted below?
>>
>>
>> -----Original Message-----
>> From: Artyom Skrobov [mailto:Artyom.Skrobov at arm.com]
>> Sent: 09 July 2014 11:20
>> To: 'Duncan P. N. Exon Smith'
>> Cc: llvm-commits
>> Subject: RE: [PATCH] extracting swapStruct into include/llvm/Support/MachO.h
>> (no functional change)
>>
>>
>>>>>> 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?
>
> Unless Michael objects, I think the right thing is probably to keep the
> `using` statements and remove the namespaces.  You'd have to move
> `struct section_base` to an anonymous namespace and make `getStruct()`
> static.  There could be something that I've missed, so look for anything
> that might change semantics.
>
> This is probably something that can be post-commit reviewed.

I've no objection here.

- Michael Spencer



More information about the llvm-commits mailing list