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

[Arthur O'Dwyer via libcxx-commits]

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)?

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
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.llvm.org/pipermail/libcxx-commits/attachments/20190617/9466bfd4/attachment-0001.html>