Quuxplusone added inline comments.
================
Comment at: include/concepts:175
+template <class _Tp, class _Up>
+_LIBCPP_CONCEPT_DECL Same = __same_impl<_Tp, _Up> && __same_impl<_Up, _Tp>;
+
----------------
Peanut gallery asks: From lines 166-171 it looks awfully like `__same_impl<_Tp,
_Up>` is true if and only if `__same_impl<_Up, _Tp>` is true. So why bother
instantiating both templates?
================
Comment at: include/concepts:280
+ Constructible<_Tp, const _Tp&> && ConvertibleTo<const _Tp&, _Tp> &&
+ Constructible<_Tp, const _Tp> && ConvertibleTo<const _Tp, _Tp>;
+
----------------
Peanut gallery notices the lack of `Constructible<_Tp, const _Tp&&>` here, and
assumes it's intentional?
================
Comment at: include/concepts:295
+ requires ConvertibleTo<const remove_reference_t<_Tp>&, bool>;
+ !__t1; requires ConvertibleTo<decltype(!__t1), bool>;
+ __t1 && __t2; requires Same<decltype(__t1 && __t2), bool>;
----------------
Peanut gallery asks: Am I correct that the instances of `ConvertibleTo<...,
bool>` here could (according to the WD) be replaced with `-> bool`? Am I
correct that the instances of `Same<..., bool>` here could eventually be
replaced with `-> Same<bool>` once they fix that issue you wrote a paper about?
I assume they're written like this for portability reasons.
================
Comment at: include/concepts:379
+_LIBCPP_CONCEPT_DECL Invocable = requires(_Fn&& __fn, _Args&&... __args) {
+ __invoke_constexpr(static_cast<_Fn&&>(__fn),
static_cast<_Args&&>(__args)...);
+};
----------------
Does this need a `_VSTD::` for ADL reasons? (There's one in `<tuple>` that does
use `_VSTD::`; there's two in `<variant>` that don't.)
================
Comment at: include/concepts:394
+ CommonReference<
+ const remove_reference_t<_Tp>&,
+ const remove_reference_t<_Up>&> &&
----------------
I'm fairly confident that `const remove_reference_t<_Tp>&` is just `const _Tp&`.
https://reviews.llvm.org/D49120
_______________________________________________
cfe-commits mailing list
[email protected]
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
