[libcxx-commits] [PATCH] D58587: Implement P0433: deduction guides for <map>

[Louis Dionne via libcxx-commits]

+ Gauthier

This is the culprit:

commit a27d26e290545ff0686c0d08975baa53b9a4878b
Author: Gauthier Harnisch <tyker1 at outlook.com>
Date:   Fri Jun 14 08:40:04 2019 +0000

    [clang] Fixing incorrect implicit deduction guides (PR41549)

    Summary:
    [[ https://bugs.llvm.org/show_bug.cgi?id=41549 | bug report ]]

    Before this patch, implicit deduction guides were generated from the first declaration found by lookup.
    With this patch implicit deduction guides are generated from the definition of the class template.
    Also added test that was previously failing.

    Reviewers: rsmith

    Reviewed By: rsmith

    Subscribers: cfe-commits, Quuxplusone

    Tags: #clang

    Differential Revision: https://reviews.llvm.org/D63072

> On Jun 17, 2019, at 17:20, Michał Górny <mgorny at gentoo.org> wrote:
> 
> On Mon, 2019-06-17 at 17:02 -0400, Louis Dionne wrote:
>>> On Jun 17, 2019, at 15:46, Louis Dionne via libcxx-commits <libcxx-commits at lists.llvm.org> wrote:
>>> 
>>> 
>>> 
>>>> On Jun 17, 2019, at 14:50, Michał Górny <mgorny at gentoo.org> wrote:
>>>> 
>>>> On Mon, 2019-06-17 at 13:49 -0400, Arthur O'Dwyer wrote:
>>>>> On Mon, Jun 17, 2019 at 12:30 PM Michał Górny via Phabricator <
>>>>> reviews at reviews.llvm.org> wrote:
>>>>> 
>>>>>> mgorny added a comment.
>>>>>> 
>>>>>> Marshall, `std/containers/associative/map/map.cons/deduct_const.pass.cpp`
>>>>>> test seems to be reliably failing from commit one on both Gentoo Linux and
>>>>>> NetBSD. Could you please look at the classical example of totally cryptic
>>>>>> C++ error? ;-)
>>>>>> 
>>>>>> 
>>>>>> http://lab.llvm.org:8011/builders/netbsd-amd64/builds/20/steps/run%20unit%20tests/logs/FAIL%3A%20libc%2B%2B%3A%3Adeduct_const.pass.cpp
>>>>>> 
>>>>> 
>>>>> Well, I can contribute that that's the error that you get when CTAD deduces
>>>>> the wrong thing. Here it's looking at lines 98–100 of deduct_const.pass.cpp
>>>>> 
>>>>>  std::map m({ PC{1,1L}, PC{2,2L}, PC{1,1L}, PC{INT_MAX,1L}, PC{3,1L} },
>>>>> test_allocator<PC>(0, 45));
>>>>>  ASSERT_SAME_TYPE(decltype(m), std::map<int, long, std::less<int>,
>>>>> test_allocator<PC>>);
>>>>> 
>>>>> and deducing the correct value_type for `m`, but it's incorrectly thinking
>>>>> that `test_allocator<PC>` should be used as the *comparator* type, not
>>>>> the *allocator
>>>>> *type.  This is surprising/incorrect, because if all is going as expected,
>>>>> then CTAD should be considering the deduction guides
>>>>> 
>>>>> template<class _Key, class _Tp, class _Compare = less<typename
>>>>> remove_const<_Key>::type>,
>>>>>       class _Allocator = allocator<pair<const _Key, _Tp>>,
>>>>>       class = typename enable_if<!__is_allocator<_Compare>::value,
>>>>> void>::type,
>>>>>       class = typename enable_if<__is_allocator<_Allocator>::value,
>>>>> void>::type>
>>>>> map(initializer_list<pair<_Key, _Tp>>, _Compare = _Compare(), _Allocator =
>>>>> _Allocator())
>>>>> -> map<typename remove_const<_Key>::type, _Tp, _Compare, _Allocator>;
>>>>> 
>>>>> template<class _Key, class _Tp, class _Allocator,
>>>>>       class = typename enable_if<__is_allocator<_Allocator>::value,
>>>>> void>::type>
>>>>> map(initializer_list<pair<_Key, _Tp>>, _Allocator)
>>>>> -> map<typename remove_const<_Key>::type, _Tp, less<typename
>>>>> remove_const<_Key>::type>, _Allocator>;
>>>>> 
>>>>> CTAD should see that the first guide fails substitution because
>>>>> __is_allocator<test_allocator<PC>>::value (that is, test_allocator should
>>>>> not be taken as the comparator type).
>>>>> CTAD should see that the second guide does not fail substitution, so CTAD
>>>>> should unambiguously use the second guide.
>>>>> 
>>>>> The test passes on OSX using clang trunk.  I don't see any #ifdefs or
>>>>> anything in include/ or in test/support/test_allocator.h that would cause
>>>>> different behavior on Linux or NetBSD.  If you try clang trunk on Linux or
>>>>> NetBSD, do you see different behavior (versus what you see with clang
>>>>> 9.0.0)?
>>>> 
>>>> This is trunk.  On NetBSD, it's tested using a single tree with
>>>> cxx_under_test being pointed to just-built clang.  On Gentoo, I was
>>>> testing out-of-source build against ~same revision of trunk (± delay
>>>> between fetching the two repos separately).
>>>> 
>>>> And yes, I'm surprised that Debian buildbots don't hit this.
>>> 
>>> So you're hitting that with Clang trunk? This is not the kind of problem where the platform should matter, only the compiler and the libc++ version. Let me try to reproduce locally.
>> 
>> Yes, I can reproduce locally with Clang trunk. I think this isn't showing up in the Linux bots because they use older compilers. This must be something that was introduced recently-ish. I'll bisect.
>> 
> 
> Oh, and I was wondering if everybody had to hack in just-built clang
> like I did ;-).  Thanks for looking into it.
> 
> 
> -- 
> Best regards,
> Michał Górny

-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.llvm.org/pipermail/libcxx-commits/attachments/20190617/f1aeba37/attachment-0001.html>