dblaikie added a comment.
In D67647#1713974 <https://reviews.llvm.org/D67647#1713974>, @comex wrote:
> So, I landed this patch but had to revert it as it broke the build on MSVC
> (and only MSVC). That was almost a month ago, but I haven't gotten back
> around to this until now – sorry :|
>
> In this updated patch, I switched is_random_iterator from a constexpr
> function:
>
> template <typename T>
> constexpr bool is_random_iterator() {
> return std::is_same<
> typename std::iterator_traits<T>::iterator_category,
> std::random_access_iterator_tag>::value;
> }
>
>
> to a type alias:
>
> template <typename T>
> using is_random_iterator =
> std::is_same<typename std::iterator_traits<T>::iterator_category,
> std::random_access_iterator_tag>;
>
>
> and changed the uses accordingly. For some reason, MSVC thought the two
> overloads of `drop_begin`, one with `enable_if<!is_random_iterator<T>()>` and
> one with `enable_if<is_random_iterator<T>()>`, were duplicate definitions.
> But with `is_random_iterator` changed to a type alias, MSVC is fine with
> them. GCC and Clang think both versions are valid, so I think it's just an
> MSVC bug.
>
> Simplified example for reference: https://gcc.godbolt.org/z/niXCy4
Sounds good to me. Maybe leave a comment explaining why a constexpr function
was not used?
Repository:
rG LLVM Github Monorepo
CHANGES SINCE LAST ACTION
https://reviews.llvm.org/D67647/new/
https://reviews.llvm.org/D67647
_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
