Re: [PATCH] libstdc++: Diagnose visitors with different return types [PR95904]
On Sat, 17 Oct 2020 at 20:30, Stephan Bergmann wrote:
> Clang (with -std=c++17/20) now complains about
>
> > include/c++/11.0.0/variant:1032:10: error: no matching constructor for
> > initialization of 'std::__nonesuch'
> > return __nonesuch{};
> >^ ~~
> > include/c++/11.0.0/type_traits:2953:5: note: candidate constructor not
> > viable: requires 1 argument, but 0 were provided
> > __nonesuch(__nonesuch const&) = delete;
> > ^
>
> upon #include . (And I think legitimately so, as __nonsuch is
> not dependent?)
This is https://gcc.gnu.org/bugzilla/show_bug.cgi?id=97449, I'll
commit a fix today.
Re: [PATCH] libstdc++: Diagnose visitors with different return types [PR95904]
On 08/10/2020 16:27, Jonathan Wakely via Gcc-patches wrote:
On 05/10/20 22:35 +0300, Ville Voutilainen via Libstdc++ wrote:
On Mon, 5 Oct 2020 at 01:15, Ville Voutilainen
wrote:
The patch is borked, doesn't pass tests, fixing...
Unborked, ok for trunk if full testsuite passes?
Assuming it has passed by now, OK. Thanks.
Clang (with -std=c++17/20) now complains about
include/c++/11.0.0/variant:1032:10: error: no matching constructor for
initialization of 'std::__nonesuch'
return __nonesuch{};
^ ~~
include/c++/11.0.0/type_traits:2953:5: note: candidate constructor not viable:
requires 1 argument, but 0 were provided
__nonesuch(__nonesuch const&) = delete;
^
upon #include . (And I think legitimately so, as __nonsuch is
not dependent?)
Re: [PATCH] libstdc++: Diagnose visitors with different return types [PR95904]
On Sat, 10 Oct 2020 at 13:52, Jonathan Wakely wrote: > index_sequence uses size_t not unsigned long. This parameter pack > needs to be size_t... _Idxs, and the NTTP for __check_visitor_result > should be size_t _Idx. Fixed in https://gcc.gnu.org/git/?p=gcc.git;a=commit;h=02cbd79e4728319e0887ad7783297853b527bb13 after approval-over-irc.
Re: [PATCH] libstdc++: Diagnose visitors with different return types [PR95904]
On 05/10/20 22:35 +0300, Ville Voutilainen via Libstdc++ wrote:
On Mon, 5 Oct 2020 at 01:15, Ville Voutilainen
wrote:
The patch is borked, doesn't pass tests, fixing...
Unborked, ok for trunk if full testsuite passes?
2020-10-05 Ville Voutilainen
PR libstdc++/95904
* include/std/variant (__deduce_visit_result): Add a nested ::type.
(__gen_vtable_impl::_S_apply):
Check the visitor return type.
(__same_types): New.
(__check_visitor_result): Likewise.
(__check_visitor_results): Likewise.
(visit(_Visitor&&, _Variants&&...)): Use __check_visitor_results
in case we're visiting just one variant.
* testsuite/20_util/variant/visit_neg.cc: Adjust.
diff --git a/libstdc++-v3/include/std/variant b/libstdc++-v3/include/std/variant
index dd8847cf829..b32e564fd41 100644
--- a/libstdc++-v3/include/std/variant
+++ b/libstdc++-v3/include/std/variant
@@ -182,7 +182,7 @@ namespace __variant
// used for raw visitation with indices passed in
struct __variant_idx_cookie { using type = __variant_idx_cookie; };
// Used to enable deduction (and same-type checking) for std::visit:
- template struct __deduce_visit_result { };
+ template struct __deduce_visit_result { using type = _Tp; };
// Visit variants that might be valueless.
template
@@ -1017,7 +1017,26 @@ namespace __variant
static constexpr auto
_S_apply()
- { return _Array_type{&__visit_invoke}; }
+ {
+ if constexpr (_Array_type::__result_is_deduced::value)
+ {
+ constexpr bool __visit_ret_type_mismatch =
+ !is_same_v(),
+ std::declval<_Variants>()...))>;
+ if constexpr (__visit_ret_type_mismatch)
+ {
+ static_assert(!__visit_ret_type_mismatch,
+ "std::visit requires the visitor to have the same "
+ "return type for all alternatives of a variant");
+ return __nonesuch{};
+ }
+ else
+ return _Array_type{&__visit_invoke};
+ }
+ else
+ return _Array_type{&__visit_invoke};
+ }
};
template
@@ -1692,6 +1711,26 @@ namespace __variant
std::forward<_Variants>(__variants)...);
}
+ template
+ constexpr inline bool __same_types = (is_same_v<_Tp, _Types> && ...);
+
+ template
+decltype(auto)
+__check_visitor_result(_Visitor&& __vis, _Variant&& __variant)
+{
+ return std::__invoke(std::forward<_Visitor>(__vis),
+ std::get<_Idx>(std::forward<_Variant>(__variant)));
+}
+
+ template
index_sequence uses size_t not unsigned long. This parameter pack
needs to be size_t... _Idxs, and the NTTP for __check_visitor_result
should be size_t _Idx.
Re: [PATCH] libstdc++: Diagnose visitors with different return types [PR95904]
On 05/10/20 22:35 +0300, Ville Voutilainen via Libstdc++ wrote: On Mon, 5 Oct 2020 at 01:15, Ville Voutilainen wrote: The patch is borked, doesn't pass tests, fixing... Unborked, ok for trunk if full testsuite passes? Assuming it has passed by now, OK. Thanks.
Re: [PATCH] libstdc++: Diagnose visitors with different return types [PR95904]
On Mon, 5 Oct 2020 at 01:15, Ville Voutilainen
wrote:
> The patch is borked, doesn't pass tests, fixing...
Unborked, ok for trunk if full testsuite passes?
2020-10-05 Ville Voutilainen
PR libstdc++/95904
* include/std/variant (__deduce_visit_result): Add a nested ::type.
(__gen_vtable_impl::_S_apply):
Check the visitor return type.
(__same_types): New.
(__check_visitor_result): Likewise.
(__check_visitor_results): Likewise.
(visit(_Visitor&&, _Variants&&...)): Use __check_visitor_results
in case we're visiting just one variant.
* testsuite/20_util/variant/visit_neg.cc: Adjust.
diff --git a/libstdc++-v3/include/std/variant b/libstdc++-v3/include/std/variant
index dd8847cf829..b32e564fd41 100644
--- a/libstdc++-v3/include/std/variant
+++ b/libstdc++-v3/include/std/variant
@@ -182,7 +182,7 @@ namespace __variant
// used for raw visitation with indices passed in
struct __variant_idx_cookie { using type = __variant_idx_cookie; };
// Used to enable deduction (and same-type checking) for std::visit:
- template struct __deduce_visit_result { };
+ template struct __deduce_visit_result { using type = _Tp; };
// Visit variants that might be valueless.
template
@@ -1017,7 +1017,26 @@ namespace __variant
static constexpr auto
_S_apply()
- { return _Array_type{&__visit_invoke}; }
+ {
+ if constexpr (_Array_type::__result_is_deduced::value)
+ {
+ constexpr bool __visit_ret_type_mismatch =
+ !is_same_v(),
+std::declval<_Variants>()...))>;
+ if constexpr (__visit_ret_type_mismatch)
+ {
+ static_assert(!__visit_ret_type_mismatch,
+ "std::visit requires the visitor to have the same "
+ "return type for all alternatives of a variant");
+ return __nonesuch{};
+ }
+ else
+ return _Array_type{&__visit_invoke};
+ }
+ else
+ return _Array_type{&__visit_invoke};
+ }
};
template
@@ -1692,6 +1711,26 @@ namespace __variant
std::forward<_Variants>(__variants)...);
}
+ template
+ constexpr inline bool __same_types = (is_same_v<_Tp, _Types> && ...);
+
+ template
+decltype(auto)
+__check_visitor_result(_Visitor&& __vis, _Variant&& __variant)
+{
+ return std::__invoke(std::forward<_Visitor>(__vis),
+ std::get<_Idx>(std::forward<_Variant>(__variant)));
+}
+
+ template
+constexpr bool __check_visitor_results(std::index_sequence<_Idxs...>)
+{
+ return __same_types(
+ std::declval<_Visitor>(),
+ std::declval<_Variant>()))...>;
+}
+
+
template
constexpr decltype(auto)
visit(_Visitor&& __visitor, _Variants&&... __variants)
@@ -1704,8 +1743,28 @@ namespace __variant
using _Tag = __detail::__variant::__deduce_visit_result<_Result_type>;
- return std::__do_visit<_Tag>(std::forward<_Visitor>(__visitor),
- std::forward<_Variants>(__variants)...);
+ if constexpr (sizeof...(_Variants) == 1)
+ {
+ constexpr bool __visit_rettypes_match =
+ __check_visitor_results<_Visitor, _Variants...>(
+ std::make_index_sequence<
+ std::variant_size...>::value>());
+ if constexpr (!__visit_rettypes_match)
+ {
+ static_assert(__visit_rettypes_match,
+ "std::visit requires the visitor to have the same "
+ "return type for all alternatives of a variant");
+ return;
+ }
+ else
+ return std::__do_visit<_Tag>(
+ std::forward<_Visitor>(__visitor),
+ std::forward<_Variants>(__variants)...);
+ }
+ else
+ return std::__do_visit<_Tag>(
+ std::forward<_Visitor>(__visitor),
+ std::forward<_Variants>(__variants)...);
}
#if __cplusplus > 201703L
diff --git a/libstdc++-v3/testsuite/20_util/variant/visit_neg.cc b/libstdc++-v3/testsuite/20_util/variant/visit_neg.cc
index 6279dec5aa2..748eb21c1ad 100644
--- a/libstdc++-v3/testsuite/20_util/variant/visit_neg.cc
+++ b/libstdc++-v3/testsuite/20_util/variant/visit_neg.cc
@@ -21,7 +21,7 @@
#include
#include
-// { dg-error "invalid conversion" "" { target *-*-* } 0 }
+// { dg-error "same return type for all alternatives" "" { target *-*-* } 0 }
// { dg-prune-output "in 'constexpr' expansion" }
void
Re: [PATCH] libstdc++: Diagnose visitors with different return types [PR95904]
On Sat, 3 Oct 2020 at 01:14, Jonathan Wakely wrote: > OK for trunk with those leading spaces switched to tab. The patch is borked, doesn't pass tests, fixing...
Re: [PATCH] libstdc++: Diagnose visitors with different return types [PR95904]
On 29/09/20 19:35 +0300, Ville Voutilainen via Libstdc++ wrote:
On Tue, 29 Sep 2020 at 14:20, Jonathan Wakely wrote:
I think this is what we want:
template
constexpr inline __same_types = (is_same_v<_Tp, _Types> && ...);
is_same_v is very cheap, it uses the built-in directly, so you don't
need to instantiate any class templates at all.
>+
>+ template
typename not class please.
>+decltype(auto) __check_visitor_result(_Visitor&& __vis,
New line after the decltype(auto) please, not in the middle of the
parameter list.
Aye.
diff --git a/libstdc++-v3/include/std/variant b/libstdc++-v3/include/std/variant
index dd8847cf829..6f647d622c4 100644
--- a/libstdc++-v3/include/std/variant
+++ b/libstdc++-v3/include/std/variant
@@ -182,7 +182,7 @@ namespace __variant
// used for raw visitation with indices passed in
struct __variant_idx_cookie { using type = __variant_idx_cookie; };
// Used to enable deduction (and same-type checking) for std::visit:
- template struct __deduce_visit_result { };
+ template struct __deduce_visit_result { using type = _Tp; };
// Visit variants that might be valueless.
template
@@ -1017,7 +1017,22 @@ namespace __variant
static constexpr auto
_S_apply()
- { return _Array_type{&__visit_invoke}; }
+ {
+ constexpr bool __visit_ret_type_mismatch =
+ _Array_type::__result_is_deduced::value
+ && !is_same_v(),
+ std::declval<_Variants>()...))>;
+ if constexpr (__visit_ret_type_mismatch)
+ {
+ static_assert(!__visit_ret_type_mismatch,
+ "std::visit requires the visitor to have the same "
+ "return type for all alternatives of a variant");
+ return __nonesuch{};
+ }
+ else
+ return _Array_type{&__visit_invoke};
+ }
};
template
@@ -1692,6 +1707,26 @@ namespace __variant
std::forward<_Variants>(__variants)...);
}
+ template
+ constexpr inline bool __same_types = (is_same_v<_Tp, _Types> && ...);
+
+ template
+decltype(auto)
+__check_visitor_result(_Visitor&& __vis, _Variant&& __variant)
+{
+ return std::forward<_Visitor>(__vis)(
+std::get<_Idx>(std::forward<_Variant>(__variant)));
Looks good, the new error is nice.
git apply warns about some whitespace errors:
/dev/shm/pr95904.diff:51: indent with spaces.
std::get<_Idx>(std::forward<_Variant>(__variant)));
/dev/shm/pr95904.diff:73: indent with spaces.
{
/dev/shm/pr95904.diff:77: indent with spaces.
std::variant_size...>::value>());
/dev/shm/pr95904.diff:92: indent with spaces.
std::forward<_Visitor>(__visitor),
warning: 4 lines add whitespace errors.
OK for trunk with those leading spaces switched to tab.
Thanks!
Re: [PATCH] libstdc++: Diagnose visitors with different return types [PR95904]
On Tue, 29 Sep 2020 at 14:20, Jonathan Wakely wrote:
> I think this is what we want:
>
>template
> constexpr inline __same_types = (is_same_v<_Tp, _Types> && ...);
>
> is_same_v is very cheap, it uses the built-in directly, so you don't
> need to instantiate any class templates at all.
>
> >+
> >+ template
>
> typename not class please.
>
> >+decltype(auto) __check_visitor_result(_Visitor&& __vis,
>
> New line after the decltype(auto) please, not in the middle of the
> parameter list.
Aye.
diff --git a/libstdc++-v3/include/std/variant b/libstdc++-v3/include/std/variant
index dd8847cf829..6f647d622c4 100644
--- a/libstdc++-v3/include/std/variant
+++ b/libstdc++-v3/include/std/variant
@@ -182,7 +182,7 @@ namespace __variant
// used for raw visitation with indices passed in
struct __variant_idx_cookie { using type = __variant_idx_cookie; };
// Used to enable deduction (and same-type checking) for std::visit:
- template struct __deduce_visit_result { };
+ template struct __deduce_visit_result { using type = _Tp; };
// Visit variants that might be valueless.
template
@@ -1017,7 +1017,22 @@ namespace __variant
static constexpr auto
_S_apply()
- { return _Array_type{&__visit_invoke}; }
+ {
+ constexpr bool __visit_ret_type_mismatch =
+ _Array_type::__result_is_deduced::value
+ && !is_same_v(),
+ std::declval<_Variants>()...))>;
+ if constexpr (__visit_ret_type_mismatch)
+ {
+ static_assert(!__visit_ret_type_mismatch,
+ "std::visit requires the visitor to have the same "
+ "return type for all alternatives of a variant");
+ return __nonesuch{};
+ }
+ else
+ return _Array_type{&__visit_invoke};
+ }
};
template
@@ -1692,6 +1707,26 @@ namespace __variant
std::forward<_Variants>(__variants)...);
}
+ template
+ constexpr inline bool __same_types = (is_same_v<_Tp, _Types> && ...);
+
+ template
+decltype(auto)
+__check_visitor_result(_Visitor&& __vis, _Variant&& __variant)
+{
+ return std::forward<_Visitor>(__vis)(
+std::get<_Idx>(std::forward<_Variant>(__variant)));
+}
+
+ template
+constexpr bool __check_visitor_results(std::index_sequence<_Idxs...>)
+{
+ return __same_types(
+ std::declval<_Visitor>(),
+ std::declval<_Variant>()))...>;
+}
+
+
template
constexpr decltype(auto)
visit(_Visitor&& __visitor, _Variants&&... __variants)
@@ -1704,8 +1739,28 @@ namespace __variant
using _Tag = __detail::__variant::__deduce_visit_result<_Result_type>;
- return std::__do_visit<_Tag>(std::forward<_Visitor>(__visitor),
- std::forward<_Variants>(__variants)...);
+ if constexpr (sizeof...(_Variants) == 1)
+{
+ constexpr bool __visit_rettypes_match =
+ __check_visitor_results<_Visitor, _Variants...>(
+ std::make_index_sequence<
+ std::variant_size...>::value>());
+ if constexpr (!__visit_rettypes_match)
+ {
+ static_assert(__visit_rettypes_match,
+ "std::visit requires the visitor to have the same "
+ "return type for all alternatives of a variant");
+ return;
+ }
+ else
+ return std::__do_visit<_Tag>(
+ std::forward<_Visitor>(__visitor),
+ std::forward<_Variants>(__variants)...);
+ }
+ else
+ return std::__do_visit<_Tag>(
+ std::forward<_Visitor>(__visitor),
+ std::forward<_Variants>(__variants)...);
}
#if __cplusplus > 201703L
