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

[Michał Górny via libcxx-commits]

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 --------------
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/d6013c96/attachment.sig>