[llvm] r211309 - Add StringMap::insert(pair) consistent with the standard associative container concept.

[Rafael Espíndola]

LGTM since the old patch was OK and the only difference to the new one is

-  MoveOnly(const MoveOnly &);
-  MoveOnly &operator=(const MoveOnly &);
+  MoveOnly(const MoveOnly &) LLVM_DELETED_FUNCTION;
+  MoveOnly &operator=(const MoveOnly &) LLVM_DELETED_FUNCTION;

On 22 June 2014 17:55, Agustín K-ballo Bergé <kaballo86 at hotmail.com> wrote:
> On 20/06/2014 04:41 p.m., David Blaikie wrote:
>>
>> On Fri, Jun 20, 2014 at 11:41 AM, Agustín K-ballo Bergé
>> <kaballo86 at hotmail.com> wrote:
>>>
>>> On 19/06/2014 09:37 p.m., Rafael Espíndola wrote:
>>>>
>>>>
>>>> Looks like this broke the build with gcc 4.7 in some bots and I reverted
>>>> it.
>>>>
>>>> On 19 June 2014 16:08, David Blaikie <dblaikie at gmail.com> wrote:
>>>>>
>>>>>
>>>>> Author: dblaikie
>>>>> Date: Thu Jun 19 15:08:56 2014
>>>>> New Revision: 211309
>>>>>
>>>>> URL: http://llvm.org/viewvc/llvm-project?rev=211309&view=rev
>>>>> Log:
>>>>> Add StringMap::insert(pair) consistent with the standard associative
>>>>> container concept.
>>>>>
>>>>> Patch by Agustín Bergé.
>>>>>
>>>>> Modified:
>>>>>       llvm/trunk/include/llvm/ADT/StringMap.h
>>>>>       llvm/trunk/lib/Support/StringMap.cpp
>>>>>       llvm/trunk/unittests/ADT/StringMapTest.cpp
>>>>>
>>>
>>> It seems this is due to a regression in gcc 4.7 when movable-only types
>>> are
>>> used in conjunction with `std::pair`. I was able to reproduce this with
>>> 4.7,
>>> but not with 4.6 nor 4.8. Interestingly, this problem doesn't show if the
>>> copy-constructor is defined private and deleted, it happens only when the
>>> copy-constructor is declared private and not defined.
>>>
>>> Looks like the test case should have been used `LLVM_DELETED_FUNCTION`.
>>
>>
>> I'm OK with using LLVM_DELETED_FUNCTION - it's better anyway from a
>> diagnostic perspective and if it happens to workaround a buggy old
>> compiler, I'm OK with that too.
>>
>>> Is
>>> this an acceptable workaround (assuming it works), or is using
>>> movable-only
>>> types in pairs just something to avoid because of this gcc 4.7
>>> regression?
>
>
> The attached patch adds `LLVM_DELETED_FUNCTION` to r211309. With those
> changes, make runs successfully on gcc-4.7.
>
>
>
> Regards,
> --
> Agustín K-ballo Bergé.-
> http://talesofcpp.fusionfenix.com
>
> _______________________________________________
> llvm-commits mailing list
> llvm-commits at cs.uiuc.edu
> http://lists.cs.uiuc.edu/mailman/listinfo/llvm-commits
>