[libcxx-commits] [PATCH] D106824: [libc++] Implement the resolution of LWG3506 in all language modes.

[Louis Dionne via Phabricator via libcxx-commits]

ldionne requested changes to this revision.
ldionne added inline comments.
This revision now requires changes to proceed.


================
Comment at: libcxx/include/queue:175
 template<class InputIterator,
-         class Compare = less<typename iterator_traits<InputIterator>::value_type>,
-         class Container = vector<typename iterator_traits<InputIterator>::value_type>>
+         class Compare = less<typename iter-value-type<InputIterator>>,
+         class Container = vector<typename iter-value-type<InputIterator>>>
----------------
Maybe we can get rid of the `typename` here?


================
Comment at: libcxx/include/queue:537
 
+    template <class _InputIter, class _Alloc, class = _EnableIf<__is_cpp17_input_iterator<_InputIter>::value> >
+        _LIBCPP_INLINE_VISIBILITY
----------------
I'm not sure I understand why we're constraining those constructors with `is_cpp17_input_iterator`?

I understand it's just generally a bad idea to have unconstrained template parameters, but that seems to be what the spec has?


================
Comment at: libcxx/include/queue:554
+
+#ifndef _LIBCPP_CXX03_LANG
+    template <class _InputIter, class _Alloc, class = _EnableIf<__is_cpp17_input_iterator<_InputIter>::value> >
----------------
Doesn't this work even in C++03 mode with the rvalue references extension?


================
Comment at: libcxx/test/std/containers/container.adaptors/priority.queue/priqueue.cons.alloc/ctor_iter_iter_comp_cont_alloc.pass.cpp:21
+
+template<class T, class Cont, class Comp = std::less<T> >
+struct PQ : std::priority_queue<T, Cont, Comp> {
----------------
Why are you creating this derived type?


================
Comment at: libcxx/test/std/containers/container.adaptors/priority.queue/priqueue.cons/deduct.pass.cpp:20
 //          class Container = vector<typename iterator_traits<InputIterator>::value_type>>
 // priority_queue(InputIterator, InputIterator, Compare = Compare(), Container = Container())
 //     -> priority_queue<typename iterator_traits<InputIterator>::value_type, Container, Compare>;
----------------
This would need to be updated.


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D106824/new/

https://reviews.llvm.org/D106824