[lld] r222455 - [mach-o] use reference with "auto" to prevent copies

[Sean Silva]

On Thu, Nov 20, 2014 at 2:49 PM, David Blaikie <dblaikie at gmail.com> wrote:

>
>
> On Thu, Nov 20, 2014 at 2:48 PM, Sean Silva <chisophugis at gmail.com> wrote:
>
>>
>>
>> On Thu, Nov 20, 2014 at 2:39 PM, David Blaikie <dblaikie at gmail.com>
>> wrote:
>>
>>>
>>>
>>> On Thu, Nov 20, 2014 at 2:39 PM, Sean Silva <chisophugis at gmail.com>
>>> wrote:
>>>
>>>>
>>>>
>>>> On Thu, Nov 20, 2014 at 1:46 PM, David Blaikie <dblaikie at gmail.com>
>>>> wrote:
>>>>
>>>>>
>>>>>
>>>>> On Thu, Nov 20, 2014 at 1:19 PM, Nick Kledzik <kledzik at apple.com>
>>>>> wrote:
>>>>>
>>>>>> Author: kledzik
>>>>>> Date: Thu Nov 20 15:19:58 2014
>>>>>> New Revision: 222455
>>>>>>
>>>>>> URL: http://llvm.org/viewvc/llvm-project?rev=222455&view=rev
>>>>>> Log:
>>>>>> [mach-o] use reference with "auto" to prevent copies
>>>>>>
>>>>>> Patch by Jean-Daniel Dupas
>>>>>>
>>>>>> Modified:
>>>>>>     lld/trunk/lib/ReaderWriter/MachO/File.h
>>>>>>
>>>>>> Modified: lld/trunk/lib/ReaderWriter/MachO/File.h
>>>>>> URL:
>>>>>> http://llvm.org/viewvc/llvm-project/lld/trunk/lib/ReaderWriter/MachO/File.h?rev=222455&r1=222454&r2=222455&view=diff
>>>>>>
>>>>>> ==============================================================================
>>>>>> --- lld/trunk/lib/ReaderWriter/MachO/File.h (original)
>>>>>> +++ lld/trunk/lib/ReaderWriter/MachO/File.h Thu Nov 20 15:19:58 2014
>>>>>> @@ -118,13 +118,13 @@ public:
>>>>>>    MachODefinedAtom *findAtomCoveringAddress(const Section &section,
>>>>>>                                              uint64_t offsetInSect,
>>>>>>                                              uint32_t
>>>>>> *foundOffsetAtom=nullptr) {
>>>>>> -    auto pos = _sectionAtoms.find(&section);
>>>>>> +    const auto& pos = _sectionAtoms.find(&section);
>>>>>>
>>>>>
>>>>> This one's a bit weird - 'find' (for any standard-like container, at
>>>>> least) returns an iterator by value. While taking a reference to that is
>>>>> valid (reference lifetime extension & all that) it's a bit surprising &
>>>>> unnecessary (has basically the same semantics as the non-reference code).
>>>>>
>>>>>
>>>>>>      if (pos == _sectionAtoms.end())
>>>>>>        return nullptr;
>>>>>> -    auto vec = pos->second;
>>>>>> +    const auto& vec = pos->second;
>>>>>>      assert(offsetInSect < section.content.size());
>>>>>>      // Vector of atoms for section are already sorted, so do binary
>>>>>> search.
>>>>>> -    auto atomPos = std::lower_bound(vec.begin(), vec.end(),
>>>>>> offsetInSect,
>>>>>> +    const auto& atomPos = std::lower_bound(vec.begin(), vec.end(),
>>>>>> offsetInSect,
>>>>>>
>>>>>
>>>>> Same here, lower_bound just returns an iterator by value, this doesn't
>>>>> look like it's avoiding any copies.
>>>>>
>>>>
>>>> Also, wouldn't this create a reference to a temporary?
>>>>
>>>
>>> Right - but reference extension semantics mean this isn't actually
>>> incorrect/unsafe...
>>>
>>
>> "reference extension semantics" doesn't ring a bell. Do you have a link
>> to a (no pun intended) reference? (or could you briefly explain?)
>>
>
> Taking a pseudorandom document from the Internet:
> http://herbsutter.com/2008/01/01/gotw-88-a-candidate-for-the-most-important-const/
>
>

Interesting. I guess that's one small class of bug that won't happen
anymore.

-- Sean Silva


>
>> -- Sean Silva
>>
>>
>>>
>>>
>>>>
>>>> -- Sean Silva
>>>>
>>>>
>>>>>
>>>>>
>>>>>>          [offsetInSect](const SectionOffsetAndAtom &ao,
>>>>>>                         uint64_t targetAddr) -> bool {
>>>>>>            // Each atom has a start offset of its slice of the
>>>>>>
>>>>>>
>>>>>> _______________________________________________
>>>>>> llvm-commits mailing list
>>>>>> llvm-commits at cs.uiuc.edu
>>>>>> http://lists.cs.uiuc.edu/mailman/listinfo/llvm-commits
>>>>>>
>>>>>
>>>>>
>>>>> _______________________________________________
>>>>> llvm-commits mailing list
>>>>> llvm-commits at cs.uiuc.edu
>>>>> http://lists.cs.uiuc.edu/mailman/listinfo/llvm-commits
>>>>>
>>>>>
>>>>
>>>
>>
>
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.llvm.org/pipermail/llvm-commits/attachments/20141120/6923e670/attachment.html>