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

[Michał Górny via libcxx-commits]

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.

> Unfortunately there is no way to debug CTAD deduction guides — the error
> messages don't tell you which guides were considered as candidates or which
> candidate was ultimately chosen.
> 
> –Arthur

-- 
Best regards,
Michał Górny

-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 618 bytes
Desc: This is a digitally signed message part
URL: <http://lists.llvm.org/pipermail/libcxx-commits/attachments/20190617/84942ce9/attachment.sig>