On 08/01/17 22:49 -0800, Tim Shen wrote:
On Tue, Jan 3, 2017 at 6:17 AM, Jonathan Wakely <jwak...@redhat.com> wrote:
On 01/01/17 04:17 -0800, Tim Shen via libstdc++ wrote:
+#define _VARIANT_RELATION_FUNCTION_TEMPLATE(__op, __name) \
+ template<typename... _Types> \
+ constexpr bool operator __op(const variant<_Types...>& __lhs, \
+ const variant<_Types...>& __rhs) \
+ { \
+ return __lhs._M##__name(__rhs,
std::index_sequence_for<_Types...>{}); \
+ } \
+\
+ constexpr bool operator __op(monostate, monostate) noexcept \
+ { return 0 __op 0; }
+
+ _VARIANT_RELATION_FUNCTION_TEMPLATE(<, _erased_less_than)
+ _VARIANT_RELATION_FUNCTION_TEMPLATE(<=, _erased_less_equal)
+ _VARIANT_RELATION_FUNCTION_TEMPLATE(==, _erased_equal)
+ _VARIANT_RELATION_FUNCTION_TEMPLATE(!=, _erased_not_equal)
+ _VARIANT_RELATION_FUNCTION_TEMPLATE(>=, _erased_greater_than)
+ _VARIANT_RELATION_FUNCTION_TEMPLATE(>, _erased_greater)
These need double underscore prefixes.
Done.
I'm sorry, I missed that they get appended to _M to form a member
function name, so they don't need a double underscore.
But since they all have the same prefix, why not use _M_erased_##name
and just use less_than, less_equal etc. in the macro invocations?
However, the names are weird, you have >= as greater_than (not
greater_equal) and > as greater (which is inconsistent with < as
less_than).
So I'd go with:
_VARIANT_RELATION_FUNCTION_TEMPLATE(<, less)
_VARIANT_RELATION_FUNCTION_TEMPLATE(<=, less_equal)
_VARIANT_RELATION_FUNCTION_TEMPLATE(==, equal)
_VARIANT_RELATION_FUNCTION_TEMPLATE(!=, not_equal)
_VARIANT_RELATION_FUNCTION_TEMPLATE(>=, greater_equal)
_VARIANT_RELATION_FUNCTION_TEMPLATE(>, greater)
+#define _VARIANT_RELATION_FUNCTION_TEMPLATE(__op, __name) \
I think we usually use all-caps for macro arguments, so _OP and _NAME,
but it doesn't really matter.
+ template<size_t... __indices> \
+ static constexpr bool \
+ (*_S##__name##_vtable[])(const variant&, const variant&) = \
+ { &__detail::__variant::__name<const variant&, __indices>... }; \
With the suggestions above this would change to use _S_erased_##_NAME
and &__detail::__variant::__erased_##_NAME
+ template<size_t... __indices> \
+ constexpr inline bool \
+ _M##__name(const variant& __rhs, \
+ std::index_sequence<__indices...>) const \
+ { \
+ auto __lhs_index = this->index(); \
+ auto __rhs_index = __rhs.index(); \
+ if (__lhs_index != __rhs_index || valueless_by_exception()) \
+ /* Intentinoal modulo addition. */ \
"Intentional" is spelled wrong, but I think simply "Modulo addition"
is clear enough that it's intentional.
+ return __lhs_index + 1 __op __rhs_index + 1; \
+ return _S##__name##_vtable<__indices...>[__lhs_index](*this, __rhs); \
}
- template<size_t... __indices>
And we'd usually use _Indices for template parameters, but this is
already inconsistent in <variant>.
The patch is OK with those naming tweaks. Thanks, and sorry for the
mixup about the underscores.
