[PATCH] Fix regex_replace

2018-01-12 Thread Tim Shen via gcc-patches 
This fixes PR 83601.

Tested on x86_64-linux-gnu.

-- 
Regards,
Tim Shen
commit 01a85ea3ae77acc7ad03430a6982db1b2b8c8bc3
Author: Tim Shen 
Date:   Fri Jan 12 22:22:45 2018 -0800

PR libstdc++/83601
* include/bits/regex.tcc (regex_replace): Fix escaping in sed.
* testsuite/28_regex/algorithms/regex_replace/char/pr83601.cc: Tests.
* testsuite/28_regex/algorithms/regex_replace/wchar_t/pr83601.cc: Tests.

diff --git a/libstdc++-v3/include/bits/regex.tcc b/libstdc++-v3/include/bits/regex.tcc
index 06cdaba09e4..b92edb9ab29 100644
--- a/libstdc++-v3/include/bits/regex.tcc
+++ b/libstdc++-v3/include/bits/regex.tcc
@@ -373,22 +373,32 @@ namespace __detail
 
   if (__flags & regex_constants::format_sed)
 	{
-	  for (; __fmt_first != __fmt_last;)
-	if (*__fmt_first == '&')
-	  {
-		__output(0);
-		++__fmt_first;
-	  }
-	else if (*__fmt_first == '\\')
-	  {
-		if (++__fmt_first != __fmt_last
-		&& __fctyp.is(__ctype_type::digit, *__fmt_first))
-		  __output(__traits.value(*__fmt_first++, 10));
-		else
-		  *__out++ = '\\';
-	  }
-	else
-	  *__out++ = *__fmt_first++;
+	  bool __escaping = false;
+	  for (; __fmt_first != __fmt_last; __fmt_first++)
+	{
+	  if (__escaping)
+		{
+		  __escaping = false;
+		  if (__fctyp.is(__ctype_type::digit, *__fmt_first))
+		__output(__traits.value(*__fmt_first, 10));
+		  else
+		*__out++ = *__fmt_first;
+		  continue;
+		}
+	  if (*__fmt_first == '\\')
+		{
+		  __escaping = true;
+		  continue;
+		}
+	  if (*__fmt_first == '&')
+		{
+		  __output(0);
+		  continue;
+		}
+	  *__out++ = *__fmt_first;
+	}
+	  if (__escaping)
+	*__out++ = '\\';
 	}
   else
 	{
diff --git a/libstdc++-v3/testsuite/28_regex/algorithms/regex_replace/char/pr83601.cc b/libstdc++-v3/testsuite/28_regex/algorithms/regex_replace/char/pr83601.cc
new file mode 100644
index 000..c4bb5d08a41
--- /dev/null
+++ b/libstdc++-v3/testsuite/28_regex/algorithms/regex_replace/char/pr83601.cc
@@ -0,0 +1,31 @@
+// { dg-do run { target c++11 } }
+
+// Copyright (C) 2018 Free Software Foundation, Inc.
+//
+// This file is part of the GNU ISO C++ Library.  This library is free
+// software; you can redistribute it and/or modify it under the
+// terms of the GNU General Public License as published by the
+// Free Software Foundation; either version 3, or (at your option)
+// any later version.
+//
+// This library is distributed in the hope that it will be useful,
+// but WITHOUT ANY WARRANTY; without even the implied warranty of
+// MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
+// GNU General Public License for more details.
+//
+// You should have received a copy of the GNU General Public License along
+// with this library; see the file COPYING3.  If not see
+// .
+//
+
+#include 
+#include 
+
+// libstdc++/83601
+int main() {
+auto format = std::regex_constants::format_sed;
+auto out = regex_replace("ab", std::regex("(a)(b)"), R"(\\1\&\\2)", format);
+VERIFY(out == R"(\1&\2)");
+
+return 0;
+}
diff --git a/libstdc++-v3/testsuite/28_regex/algorithms/regex_replace/wchar_t/pr83601.cc b/libstdc++-v3/testsuite/28_regex/algorithms/regex_replace/wchar_t/pr83601.cc
new file mode 100644
index 000..a318e900921
--- /dev/null
+++ b/libstdc++-v3/testsuite/28_regex/algorithms/regex_replace/wchar_t/pr83601.cc
@@ -0,0 +1,39 @@
+// { dg-do run { target c++11 } }
+
+// Copyright (C) 2018 Free Software Foundation, Inc.
+//
+// This file is part of the GNU ISO C++ Library.  This library is free
+// software; you can redistribute it and/or modify it under the
+// terms of the GNU General Public License as published by the
+// Free Software Foundation; either version 3, or (at your option)
+// any later version.
+//
+// This library is distributed in the hope that it will be useful,
+// but WITHOUT ANY WARRANTY; without even the implied warranty of
+// MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
+// GNU General Public License for more details.
+//
+// You should have received a copy of the GNU General Public License along
+// with this library; see the file COPYING3.  If not see
+// .
+//
+
+#include 
+#include 
+
+// libstdc++/83601
+void frep(const wchar_t *istr, const wchar_t *rstr, const wchar_t *ostr) {
+std::basic_regex wrgx(L"(a*)(b+)");
+std::basic_string wstr = istr, wret = ostr, test;
+std::regex_replace(std::back_inserter(test), wstr.begin(), wstr.end(),
+   wrgx, std::basic_string(rstr),
+   std::regex_constants::format_sed);
+VERIFY(test == wret);
+}
+
+int main() {
+frep(L"xbbyabz", L"!2!", L"x!\\2!y!\\2!z");
+frep(L"xbbyabz", L"!0!", L"x!\\0!y!\\0!z");
+frep(L"xbbyabz", L"!\\&!", L"x!&!y!&!z");
+return 0;
+}

Re: [libstdc++/71500] make back reference work with icase

2017-09-18 Thread Tim Shen via gcc-patches 
On Mon, Sep 18, 2017 at 4:01 PM, Jonathan Wakely  wrote:
> On 18/09/17 10:58 -0700, Tim Shen via libstdc++ wrote:
>>
>> On Mon, Sep 18, 2017 at 10:26 AM, Jonathan Wakely 
>> wrote:

 We need to rewrite this to check the lengths are equal first, and then
 call the 3-argument version of std::equal.

 Alternatively, we could move the implementation of the C++14
 std::equal overloads to __equal and make that available for C++11.
 I'll try that.
>>>
>>>
>>>
>>> Here's a proof of concept patch for that. It's a bit ugly.
>>
>>
>> Instead of having iterator tags in the interface, we can probe the
>> random-access-ness inside __equal4/__equal4_p, can't we? It's similar
>> to the existing "if (_RAIters()) { ... }".
>>
>> I'd expect the patches to be renaming the current implementations and
>> adding wrappers, instead of adding new implementations.
>
>
> Well I decided to split the existing functions up and use tag
> dispatching, which is conceptually cleaner anyway. But as the
> RandomAccessIterator version doesn't need any operations that aren't
> valid for other categories, it's not strictly necessary. The tag
> dispatching version should generate slightly smaller code for
> unoptimized builds, but that's not very important.

Unoptimized builds don't inline small functions, therefore the first
patch generate two weak symbols, instead of one by the second patch.
It's unclear to me how would number of symbols penalize the
performance/binary size.

>
> Here's the patch doing it as you suggest. We can't call the new
> functions __equal because t hat name is already taken by a helper
> struct, hence __equal4.
>
> Do you prefer this version?

Yes, I prefer this version for readability reasons:
1) subjectively, less scattered code; and
2) ideally I want `if constexpr (...)`), the if version is closer.

I agree that it's not a big difference. I just wanted to point out the
small difference. I'm fine with either version.

Thanks for the prototyping!


-- 
Regards,
Tim Shen

Re: [libstdc++/71500] make back reference work with icase

2017-09-18 Thread Tim Shen via gcc-patches 
On Mon, Sep 18, 2017 at 10:26 AM, Jonathan Wakely  wrote:
>> We need to rewrite this to check the lengths are equal first, and then
>> call the 3-argument version of std::equal.
>>
>> Alternatively, we could move the implementation of the C++14
>> std::equal overloads to __equal and make that available for C++11.
>> I'll try that.
>
>
> Here's a proof of concept patch for that. It's a bit ugly.

Instead of having iterator tags in the interface, we can probe the
random-access-ness inside __equal4/__equal4_p, can't we? It's similar
to the existing "if (_RAIters()) { ... }".

I'd expect the patches to be renaming the current implementations and
adding wrappers, instead of adding new implementations.


-- 
Regards,
Tim Shen

[libstdc++/71500] make back reference work with icase

2017-09-04 Thread Tim Shen via gcc-patches 
This fixes the follow-up comments in 71500.

Back-reference matching is different from other matching, as the
content the back-reference refers to is at "run-time", aka during
regex_match(), not regex() compilation.

For compilation we do have an abstraction layer to catch all
comparison customizations, namely _M_translator in regex_compiler.h.
Until this patch, we don't have an abstraction for "run-time"
matching. I believe that back-reference is the only place that needs
run-time matching, so I just build a _Backref_matcher in
regex_executot.tcc.

Tested on x86_64-linux-gnu.

Thanks!

-- 
Regards,
Tim Shen
commit a97b7fecd319e031ffc489a956b8cf3dc63eeb26
Author: Tim Shen 
Date:   Mon Sep 4 03:19:35 2017 -0700

PR libstdc++/71500
* include/bits/regex_executor.tcc: Support icase in
regex_tratis<...> for back reference matches.
* testsuite/28_regex/regression.cc: Test case.

diff --git a/libstdc++-v3/include/bits/regex_executor.tcc b/libstdc++-v3/include/bits/regex_executor.tcc
index 226e05856e1..f6149fecf9d 100644
--- a/libstdc++-v3/include/bits/regex_executor.tcc
+++ b/libstdc++-v3/include/bits/regex_executor.tcc
@@ -335,6 +335,54 @@ namespace __detail
 	  _M_states._M_queue(__state._M_next, _M_cur_results);
 }
 
+  template
+struct _Backref_matcher
+{
+  _Backref_matcher(bool __icase, const _TraitsT& __traits)
+  : _M_traits(__traits) { }
+
+  bool
+  _M_apply(_BiIter __expected_begin,
+	   _BiIter __expected_end, _BiIter __actual_begin,
+	   _BiIter __actual_end)
+  {
+	return _M_traits.transform(__expected_begin, __expected_end)
+	== _M_traits.transform(__actual_begin, __actual_end);
+  }
+
+  const _TraitsT& _M_traits;
+};
+
+  template
+struct _Backref_matcher<_BiIter, std::regex_traits<_CharT>>
+{
+  using _TraitsT = std::regex_traits<_CharT>;
+  _Backref_matcher(bool __icase, const _TraitsT& __traits)
+  : _M_icase(__icase), _M_traits(__traits) { }
+
+  bool
+  _M_apply(_BiIter __expected_begin,
+	   _BiIter __expected_end, _BiIter __actual_begin,
+	   _BiIter __actual_end)
+  {
+	if (!_M_icase)
+	  return std::equal(__expected_begin, __expected_end,
+			__actual_begin, __actual_end);
+	typedef std::ctype<_CharT> __ctype_type;
+	const auto& __fctyp = use_facet<__ctype_type>(_M_traits.getloc());
+	return std::equal(__expected_begin, __expected_end,
+			  __actual_begin, __actual_end,
+			  [this, &__fctyp](_CharT __lhs, _CharT __rhs)
+			  {
+			return __fctyp.tolower(__lhs)
+== __fctyp.tolower(__rhs);
+			  });
+  }
+
+  bool _M_icase;
+  const _TraitsT& _M_traits;
+};
+
   // First fetch the matched result from _M_cur_results as __submatch;
   // then compare it with
   // (_M_current, _M_current + (__submatch.second - __submatch.first)).
@@ -355,9 +403,10 @@ namespace __detail
 	   __last != _M_end && __tmp != __submatch.second;
 	   ++__tmp)
 	++__last;
-  if (_M_re._M_automaton->_M_traits.transform(__submatch.first,
-		  __submatch.second)
-	  == _M_re._M_automaton->_M_traits.transform(_M_current, __last))
+  if (_Backref_matcher<_BiIter, _TraitsT>(
+	  _M_re.flags() & regex_constants::icase,
+	  _M_re._M_automaton->_M_traits)._M_apply(
+		  __submatch.first, __submatch.second, _M_current, __last))
 	{
 	  if (__last != _M_current)
 	{
diff --git a/libstdc++-v3/testsuite/28_regex/regression.cc b/libstdc++-v3/testsuite/28_regex/regression.cc
index ee4d3e1e6f8..3fa9022eac4 100644
--- a/libstdc++-v3/testsuite/28_regex/regression.cc
+++ b/libstdc++-v3/testsuite/28_regex/regression.cc
@@ -93,6 +93,17 @@ test06()
   }
 }
 
+// PR libstdc++/71500
+void
+test07()
+{
+  bool test [[gnu::unused]] = true;
+
+  VERIFY(regex_match_debug("abc abc", regex("([a-z]+) \\1", regex::icase)));
+  VERIFY(regex_match_debug("Abc abc", regex("([a-z]+) \\1", regex::icase)));
+  VERIFY(regex_match_debug("abc Abc", regex("([a-z]+) \\1", regex::icase)));
+}
+
 int
 main()
 {
@@ -102,6 +113,7 @@ main()
   test04();
   test05();
   test06();
+  test07();
   return 0;
 }

Re: [Patch] Forward triviality in variant

2017-06-18 Thread Tim Shen via gcc-patches 
Besides the changes on the comments, I also changed the definition of
_S_trivial_copy_assign and _S_trivial_move_assign to match what union
has. See [class.copy.assign]p9.

On Thu, Jun 1, 2017 at 8:13 AM, Jonathan Wakely wrote:
> On 30/05/17 02:16 -0700, Tim Shen via libstdc++ wrote:
>>
>> diff --git a/libstdc++-v3/include/std/variant
>> b/libstdc++-v3/include/std/variant
>> index b9824a5182c..f81b815af09 100644
>> --- a/libstdc++-v3/include/std/variant
>> +++ b/libstdc++-v3/include/std/variant
>> @@ -290,6 +290,53 @@ _GLIBCXX_BEGIN_NAMESPACE_VERSION
>>   __ref_cast<_Tp>(__t));
>> }
>>
>> +  template
>> +struct _Traits
>> +{
>> +  static constexpr bool is_default_constructible_v =
>> +  is_default_constructible_v> _Types...>::type>;
>> +  static constexpr bool is_copy_constructible_v =
>> +  __and_...>::value;
>> +  static constexpr bool is_move_constructible_v =
>> +  __and_...>::value;
>> +  static constexpr bool is_copy_assignable_v =
>> +  is_copy_constructible_v && is_move_constructible_v
>> +  && __and_...>::value;
>> +  static constexpr bool is_move_assignable_v =
>> +  is_move_constructible_v
>> +  && __and_...>::value;
>
>
> It seems strange to me that these ones end with _v but the following
> ones don't. Could we make them all have no _v suffix?

Done. They are internal traits only for readability, so I shortened
the names and make them libstdc++ style, e.g. _S_copy_ctor.

>
>> +  static constexpr bool is_dtor_trivial =
>> +  __and_...>::value;
>> +  static constexpr bool is_copy_ctor_trivial =
>> +  __and_...>::value;
>> +  static constexpr bool is_move_ctor_trivial =
>> +  __and_...>::value;
>> +  static constexpr bool is_copy_assign_trivial =
>> +  is_dtor_trivial
>> +  && is_copy_ctor_trivial
>> +  && __and_...>::value;
>> +  static constexpr bool is_move_assign_trivial =
>> +  is_dtor_trivial
>> +  && is_move_ctor_trivial
>> +  && __and_...>::value;
>> +
>> +  static constexpr bool is_default_ctor_noexcept =
>> +  is_nothrow_default_constructible_v<
>> +  typename _Nth_type<0, _Types...>::type>;
>> +  static constexpr bool is_copy_ctor_noexcept =
>> +  is_copy_ctor_trivial;
>> +  static constexpr bool is_move_ctor_noexcept =
>> +  is_move_ctor_trivial
>> +  || __and_...>::value;
>> +  static constexpr bool is_copy_assign_noexcept =
>> +  is_copy_assign_trivial;
>> +  static constexpr bool is_move_assign_noexcept =
>> +  is_move_assign_trivial ||
>> +  (is_move_ctor_noexcept
>> +   && __and_...>::value);
>> +};
>
>
> Does using __and_ for any of those traits reduce the limit on the
> number of alternatives in a variant? We switched to using fold
> expressions in some contexts to avoid very deep instantiations, but I
> don't know if these will hit the same problem, but it looks like it
> will.

Done, use fold expression instead. At one point we changed some fold
expressions to __and_, because __and_ has short circuiting; does fold
expressions have short circuits too? Now that I think about it, short
circuiting in a constant fold expression should be a QoI issue.

>> @@ -928,12 +1107,19 @@ _GLIBCXX_BEGIN_NAMESPACE_VERSION
>> static constexpr size_t __index_of =
>>   __detail::__variant::__index_of_v<_Tp, _Types...>;
>>
>> +  using _Traits = __detail::__variant::_Traits<_Types...>;
>> +
>> public:
>> -  constexpr variant()
>> -  noexcept(is_nothrow_default_constructible_v<__to_type<0>>) =
>> default;
>> -  variant(const variant&) = default;
>> +  variant() noexcept(_Traits::is_default_ctor_noexcept) = default;
>
>
> Do we need the exception specifications here? Will the =default make
> the right thing happen anyway? (And if not, won't we get an error by
> trying to define the constructors as noexcept when the implicit
> definition would not be noexcept?)

Done. Removed unnecessary noexcept qualifiers.

It turns out I mistakenly thought using "variant() = default" means
`variant() noexcept(false) = default`.

-- 
Regards,
Tim Shen
commit 919492daf1c5ff78de4a33bdae265ea1acacc446
Author: Tim Shen 
Date:   Mon May 29 22:44:42 2017 -0700

2017-05-30  Tim Shen  

PR libstdc++/80187
* include/std/variant (variant::variant, variant::~variant,
variant::operator=): Implement triviality forwarding for four
special member

Re: [PATCH] PR libstdc++/80939 Remove unmeetable constexpr specifiers

2017-06-02 Thread Tim Shen via gcc-patches 
On Fri, Jun 2, 2017 at 6:07 AM, Jonathan Wakely wrote:
> As the PR points out, we aren't qualifying calls to __ref_cast, and
> have 'constexpr' on function templates that can never be usable in
> constant expressions.

Apology for the constexpr trolling, but that was not intentional. :)

I'm curious why no tests break. Is it because constexpr in a template
function is a no-op instead of a hard error, when the function
definition is not constexpr?

The patch looks good.

>
> This fixes it, and also simplifies __variant::__erased_dtor by using
> std::_Destroy, although that requires including quite a lot more code,
> for iterator_traits and allocator_traits.  If that matters (probably
> not) then  could be split up to move _Construct
> and _Destroy to a new .  Or maybe I should
> just leave __erased_dtor alone (apart from qualifying the __ref_cast
> call).
>
> Anybody feel strongly either way?
>
> PR libstdc++/80939
> * include/std/variant (__erased_ctor, __erased_assign,
> __erased_swap)
> (__erased_hash): Remove constexpr specifier and qualify calls to
> __ref_cast.
> (__erased_dtor): Remove constexpr specifier and use _Destroy.
>
>



-- 
Regards,
Tim Shen

Re: [Patch] Forward triviality in variant

2017-05-30 Thread Tim Shen via gcc-patches 
On Mon, May 29, 2017 at 11:29 PM, Tim Shen  wrote:
> This patch implements
> , but with more

Actually, it didn't. The copy assign and move assign conditions are
wrong in the patch. Fixed those.


-- 
Regards,
Tim Shen
commit 03387ef5007e171e4aeceeddf4d856caffeedb41
Author: Tim Shen 
Date:   Mon May 29 22:44:42 2017 -0700

2017-05-30  Tim Shen  

PR libstdc++/80187
* include/std/variant (variant::variant, variant::~variant,
variant::operator=): Implement triviality forwarding for four
special member functions.
* testsuite/20_util/variant/compile.cc: Tests.

diff --git a/libstdc++-v3/include/std/variant b/libstdc++-v3/include/std/variant
index b9824a5182c..f81b815af09 100644
--- a/libstdc++-v3/include/std/variant
+++ b/libstdc++-v3/include/std/variant
@@ -290,6 +290,53 @@ _GLIBCXX_BEGIN_NAMESPACE_VERSION
 	  __ref_cast<_Tp>(__t));
 }
 
+  template
+struct _Traits
+{
+  static constexpr bool is_default_constructible_v =
+  is_default_constructible_v::type>;
+  static constexpr bool is_copy_constructible_v =
+  __and_...>::value;
+  static constexpr bool is_move_constructible_v =
+  __and_...>::value;
+  static constexpr bool is_copy_assignable_v =
+  is_copy_constructible_v && is_move_constructible_v
+  && __and_...>::value;
+  static constexpr bool is_move_assignable_v =
+  is_move_constructible_v
+  && __and_...>::value;
+
+  static constexpr bool is_dtor_trivial =
+  __and_...>::value;
+  static constexpr bool is_copy_ctor_trivial =
+  __and_...>::value;
+  static constexpr bool is_move_ctor_trivial =
+  __and_...>::value;
+  static constexpr bool is_copy_assign_trivial =
+  is_dtor_trivial
+  && is_copy_ctor_trivial
+  && __and_...>::value;
+  static constexpr bool is_move_assign_trivial =
+  is_dtor_trivial
+  && is_move_ctor_trivial
+  && __and_...>::value;
+
+  static constexpr bool is_default_ctor_noexcept =
+  is_nothrow_default_constructible_v<
+  typename _Nth_type<0, _Types...>::type>;
+  static constexpr bool is_copy_ctor_noexcept =
+  is_copy_ctor_trivial;
+  static constexpr bool is_move_ctor_noexcept =
+  is_move_ctor_trivial
+  || __and_...>::value;
+  static constexpr bool is_copy_assign_noexcept =
+  is_copy_assign_trivial;
+  static constexpr bool is_move_assign_noexcept =
+  is_move_assign_trivial ||
+  (is_move_ctor_noexcept
+   && __and_...>::value);
+};
+
   // Defines members and ctors.
   template
 union _Variadic_union { };
@@ -355,6 +402,19 @@ _GLIBCXX_BEGIN_NAMESPACE_VERSION
   ~_Variant_storage()
   { _M_reset(); }
 
+  void*
+  _M_storage() const
+  {
+	return const_cast(static_cast(
+	std::addressof(_M_u)));
+  }
+
+  constexpr bool
+  _M_valid() const noexcept
+  {
+	return this->_M_index != __index_type(variant_npos);
+  }
+
   _Variadic_union<_Types...> _M_u;
   using __index_type = __select_index<_Types...>;
   __index_type _M_index;
@@ -374,59 +434,114 @@ _GLIBCXX_BEGIN_NAMESPACE_VERSION
   void _M_reset()
   { _M_index = variant_npos; }
 
+  void*
+  _M_storage() const
+  {
+	return const_cast(static_cast(
+	std::addressof(_M_u)));
+  }
+
+  constexpr bool
+  _M_valid() const noexcept
+  {
+	return this->_M_index != __index_type(variant_npos);
+  }
+
   _Variadic_union<_Types...> _M_u;
   using __index_type = __select_index<_Types...>;
   __index_type _M_index;
 };
 
-  // Helps SFINAE on special member functions. Otherwise it can live in variant
-  // class.
   template
-struct _Variant_base :
-  _Variant_storage<(std::is_trivially_destructible_v<_Types> && ...),
-			_Types...>
-{
-  using _Storage =
-	  _Variant_storage<(std::is_trivially_destructible_v<_Types> && ...),
-			_Types...>;
+using _Variant_storage_alias =
+_Variant_storage<_Traits<_Types...>::is_dtor_trivial, _Types...>;
 
-  constexpr
-  _Variant_base()
-  noexcept(is_nothrow_default_constructible_v<
-		 variant_alternative_t<0, variant<_Types...>>>)
-  : _Variant_base(in_place_index<0>) { }
+  // The following are (Copy|Move) (ctor|assign) layers for forwarding
+  // triviality and handling non-trivial SMF

[Patch] Forward triviality in variant

2017-05-30 Thread Tim Shen via gcc-patches 
This patch implements
, but with more
changes than the proposal's. It
1) Creates __detail::__variant::_Traits as a centralized place to hold
common (but not all yet) compile-time conditions.
2) Changes the noexcept conditions for the (copy|move) (ctor|assign)
SMFs, so that when one is trivial, one is also noexcept. It's not the
same as p0088r3, nor p0088r3 + D0602R1 anymore.
3) Creates 4 structs, namely (_Copy|_Move)_(ctor|assign)_(base|alias)
for dispatch on triviality. The code that were originally in
_Variant_base are moved into these four structs. There aren't
functional changes except for more triviality.

Sorry for having a large patch. Do tell me if you want me to split it.

Tested on x86_64-linux-gnu.

Thanks!


-- 
Regards,
Tim Shen
commit a4db7d21c6e4223300861114931eb0ef78bef1a6
Author: Tim Shen 
Date:   Mon May 29 22:44:42 2017 -0700

2017-05-30  Tim Shen  

PR libstdc++/80187
* include/std/variant (variant::variant, variant::~variant,
variant::operator=): Implement triviality forwarding for four
special member functions.
* testsuite/20_util/variant/compile.cc: Tests.

diff --git a/libstdc++-v3/include/std/variant b/libstdc++-v3/include/std/variant
index b9824a5182c..8736fcc75bc 100644
--- a/libstdc++-v3/include/std/variant
+++ b/libstdc++-v3/include/std/variant
@@ -290,6 +290,49 @@ _GLIBCXX_BEGIN_NAMESPACE_VERSION
 	  __ref_cast<_Tp>(__t));
 }
 
+  template
+struct _Traits
+{
+  static constexpr bool is_default_constructible_v =
+  is_default_constructible_v::type>;
+  static constexpr bool is_copy_constructible_v =
+  __and_...>::value;
+  static constexpr bool is_move_constructible_v =
+  __and_...>::value;
+  static constexpr bool is_copy_assignable_v =
+  is_copy_constructible_v && is_move_constructible_v
+  && __and_...>::value;
+  static constexpr bool is_move_assignable_v =
+  is_move_constructible_v
+  && __and_...>::value;
+
+  static constexpr bool is_copy_ctor_trivial =
+  __and_...>::value;
+  static constexpr bool is_move_ctor_trivial =
+  __and_...>::value;
+  static constexpr bool is_copy_assign_trivial =
+  __and_...>::value;
+  static constexpr bool is_move_assign_trivial =
+  __and_...>::value;
+  static constexpr bool is_dtor_trivial =
+  __and_...>::value;
+
+  static constexpr bool is_default_ctor_noexcept =
+  is_nothrow_default_constructible_v<
+  typename _Nth_type<0, _Types...>::type>;
+  static constexpr bool is_copy_ctor_noexcept =
+  is_copy_ctor_trivial;
+  static constexpr bool is_move_ctor_noexcept =
+  is_move_ctor_trivial
+  || __and_...>::value;
+  static constexpr bool is_copy_assign_noexcept =
+  is_copy_assign_trivial;
+  static constexpr bool is_move_assign_noexcept =
+  is_move_assign_trivial ||
+  (is_move_ctor_noexcept
+   && __and_...>::value);
+};
+
   // Defines members and ctors.
   template
 union _Variadic_union { };
@@ -355,6 +398,19 @@ _GLIBCXX_BEGIN_NAMESPACE_VERSION
   ~_Variant_storage()
   { _M_reset(); }
 
+  void*
+  _M_storage() const
+  {
+	return const_cast(static_cast(
+	std::addressof(_M_u)));
+  }
+
+  constexpr bool
+  _M_valid() const noexcept
+  {
+	return this->_M_index != __index_type(variant_npos);
+  }
+
   _Variadic_union<_Types...> _M_u;
   using __index_type = __select_index<_Types...>;
   __index_type _M_index;
@@ -374,59 +430,114 @@ _GLIBCXX_BEGIN_NAMESPACE_VERSION
   void _M_reset()
   { _M_index = variant_npos; }
 
+  void*
+  _M_storage() const
+  {
+	return const_cast(static_cast(
+	std::addressof(_M_u)));
+  }
+
+  constexpr bool
+  _M_valid() const noexcept
+  {
+	return this->_M_index != __index_type(variant_npos);
+  }
+
   _Variadic_union<_Types...> _M_u;
   using __index_type = __select_index<_Types...>;
   __index_type _M_index;
 };
 
-  // Helps SFINAE on special member functions. Otherwise it can live in variant
-  // class.
   template
-struct _Variant_base :
-  _Variant_storage<(std::is_trivially_destructible_v<_Types> && ...),
-			_Types...>
-{
-  using _Storage =
-	  _Variant_storage<(std::is_trivially_destructible_v<_Types> && ...),
-			_Types...>;
+using _Variant_storage_alias =
+

Re: [Patch] SFINAE on is_same first in variant's _Tp&& constructor

2017-05-28 Thread Tim Shen via gcc-patches 
On Tue, May 23, 2017 at 3:24 AM, Jonathan Wakely wrote:
> On 22/05/17 16:14 -0400, Tim Song wrote:
> Ah I see.
>
>> The original patch does that (assuming core issue 1227's resolution),
>> but the __and_ version doesn't; __and_ only short circuits the
>> immediate parameter, not things used in forming it.
>
>
> Then the original patch is OK for trunk and gcc-7-branch.
>
> Thank you Tim and Tim for the explanations.
>

Committed. I didn't bother using remove_cv only
because p0088r3 says decay_t.


-- 
Regards,
Tim Shen

Re: [Patch] SFINAE on is_same first in variant's _Tp&& constructor

2017-05-22 Thread Tim Shen via gcc-patches 
On Mon, May 22, 2017 at 11:05 AM, Tim Shen  wrote:
> On Mon, May 22, 2017 at 6:21 AM, Jonathan Wakely  wrote:
> I suggest to cc a front-end person (Jason?) to take a look, as I
> suggested in the bug, and the example: https://godbolt.org/g/AxUv16.

See more discussion in pr80737. Basically in the godbolt example,
turning on and off -DBUG results in different behaviors, but it
shouldn't.


-- 
Regards,
Tim Shen

Re: [Patch] SFINAE on is_same first in variant's _Tp&& constructor

2017-05-22 Thread Tim Shen via gcc-patches 
On Mon, May 22, 2017 at 6:21 AM, Jonathan Wakely  wrote:
> On 19/05/17 22:40 -0700, Tim Shen via libstdc++ wrote:
>>
>> diff --git a/libstdc++-v3/include/std/variant
>> b/libstdc++-v3/include/std/variant
>> index 0e04a820d69..b9824a5182c 100644
>> --- a/libstdc++-v3/include/std/variant
>> +++ b/libstdc++-v3/include/std/variant
>> @@ -936,9 +936,9 @@ _GLIBCXX_BEGIN_NAMESPACE_VERSION
>>   noexcept((is_nothrow_move_constructible_v<_Types> && ...)) =
>> default;
>>
>>   template> +  typename = enable_if_t, variant>>,
>>typename =
>> enable_if_t<__exactly_once<__accepted_type<_Tp&&>>
>> - && is_constructible_v<__accepted_type<_Tp&&>,
>> _Tp&&>
>> - && !is_same_v, variant>>>
>> + && is_constructible_v<__accepted_type<_Tp&&>,
>> _Tp&&>>>
>
>
> Does this definitely short-circuit? I seem to recall a similar case
> where either Clang or GCC (I think it was Clang) was evaluating the
> second default template argument even though the first had produce a
> substition failure.
>
> If we need to guarantee it short-circuits then we'd want:
>
>  templatetypename = enable_if_t<__and_<
>  __not_, variant>>,
>  __bool_constant<
>
> __exactly_once<__accepted_type<_Tp&&>>
>&&
> is_constructible_v<__accepted_type<_Tp&&>, _Tp&&>>>
>
> i.e. use __and_< is-this-type, everything-else> where
> "everything-else" still uses && to avoid making the instantiations too
> deep.

Good observation. I changed to use __and_ and __not_:

-  typename = enable_if_t<__exactly_once<__accepted_type<_Tp&&>>
- && is_constructible_v<__accepted_type<_Tp&&>, _Tp&&>
- && !is_same_v, variant>>>
+  typename = enable_if_t<__and_<
+ __not_, variant>>,
+ std::integral_constant>>,
+ is_constructible<__accepted_type<_Tp&&>,
_Tp&&>>::value>>

(I didn't use && at all, just to verify the correctness)

but the compile still fails with the similar error messages. If __and_
and __not_ are expected to work, then the root cause is unlikely "not
short-circuit" only.

I suggest to cc a front-end person (Jason?) to take a look, as I
suggested in the bug, and the example: https://godbolt.org/g/AxUv16.

>
> Also, this is another place where we could use an __is_samey
> trait that does is_same.
>

I never know that "samey" is a word. :)


-- 
Regards,
Tim Shen

[Patch] SFINAE on is_same first in variant's _Tp&& constructor

2017-05-19 Thread Tim Shen via gcc-patches 
This fixes PR libstdc++/80737.

I actually can't come up with a minimal test case, because I suspect
that there is a front-end bug in GCC. See discussions in the bug.

Tested on x86_64-linux-gnu.

Thanks!


-- 
Regards,
Tim Shen
commit 6f362991f025069328c4901d95b657d498aad250
Author: Tim Shen 
Date:   Fri May 19 22:26:58 2017 -0700

2017-05-20  Tim Shen  

PR libstdc++/80737
* include/std/variant(variant::variant): SFINAE on is_same first.
* testsuite/20_util/variant/any.cc: test case.

diff --git a/libstdc++-v3/include/std/variant b/libstdc++-v3/include/std/variant
index 0e04a820d69..b9824a5182c 100644
--- a/libstdc++-v3/include/std/variant
+++ b/libstdc++-v3/include/std/variant
@@ -936,9 +936,9 @@ _GLIBCXX_BEGIN_NAMESPACE_VERSION
   noexcept((is_nothrow_move_constructible_v<_Types> && ...)) = default;
 
   template, variant>>,
 	   typename = enable_if_t<__exactly_once<__accepted_type<_Tp&&>>
-			  && is_constructible_v<__accepted_type<_Tp&&>, _Tp&&>
-			  && !is_same_v, variant>>>
+			  && is_constructible_v<__accepted_type<_Tp&&>, _Tp&&>>>
 	constexpr
 	variant(_Tp&& __t)
 	noexcept(is_nothrow_constructible_v<__accepted_type<_Tp&&>, _Tp&&>)
diff --git a/libstdc++-v3/testsuite/20_util/variant/any.cc b/libstdc++-v3/testsuite/20_util/variant/any.cc
new file mode 100644
index 000..5811d0f055e
--- /dev/null
+++ b/libstdc++-v3/testsuite/20_util/variant/any.cc
@@ -0,0 +1,31 @@
+// { dg-options "-std=gnu++17" }
+// { dg-do compile }
+
+// Copyright (C) 2017 Free Software Foundation, Inc.
+//
+// This file is part of the GNU ISO C++ Library.  This library is free
+// software; you can redistribute it and/or modify it under the
+// terms of the GNU General Public License as published by the
+// Free Software Foundation; either version 3, or (at your option)
+// any later version.
+
+// This library is distributed in the hope that it will be useful,
+// but WITHOUT ANY WARRANTY; without even the implied warranty of
+// MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
+// GNU General Public License for more details.
+
+// You should have received a copy of the GNU General Public License along
+// with this library; see the file COPYING3.  If not see
+// .
+
+#include 
+#include 
+
+struct A { std::variant a; };
+
+void Bar(const A&);
+
+void Foo() {
+  A a;
+  Bar(a);
+}

Re: [PATCH] Implement P0393R3

2017-02-15 Thread Tim Shen via gcc-patches 
On Mon, Jan 9, 2017 at 2:52 AM, Jonathan Wakely  wrote:
> On 08/01/17 22:49 -0800, Tim Shen wrote:
>>
>> On Tue, Jan 3, 2017 at 6:17 AM, Jonathan Wakely 
>> wrote:
>>>
>>> On 01/01/17 04:17 -0800, Tim Shen via libstdc++ wrote:


 +#define _VARIANT_RELATION_FUNCTION_TEMPLATE(__op, __name) \
 +  template \
 +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 \
>> +   static constexpr bool \
>> +   (*_S##__name##_vtable[])(const variant&, const variant&) = \
>> + { &__detail::__variant::__name... };
>> \
>
>
> With the suggestions above this would change to use _S_erased_##_NAME
> and &__detail::__variant::__erased_##_NAME
>
>> +  template \
>> +   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); \
>> }

All done.

>>
>> -  template
>
>
> And we'd usually use _Indices for template parameters, but this is
> already inconsistent in .

I didn't change this one, since I prefer in-file consistency. That's
weird, So let's discuss this. There are several naming style in
libstdc++:
1) __underscore_name, for free functions, variables, and namespaces;
2) _CamelCase, for types, template type parameters in some files (e.g.
, I forgot why I did that :/).
3) _Camel_underscore_name, for types in many other files.
4) _S_underscore_name, _M_underscore name, for static and member
functions, respectively.

There are two questions:
*) It seems natural to have (1) for non-type template parameters,
since that are also variables.
*) _CamelCase vs _Camel_underscore_name? Which one is preferred?

>
> The patch is OK with those naming tweaks. Thanks, and sorry for the
> mixup about the underscores.
>

No worries! Tested and committed.


-- 
Regards,
Tim Shen

Re: [PATCH] Fix return type detection in visit()

2017-02-14 Thread Tim Shen via gcc-patches 
On Tue, Feb 14, 2017 at 2:49 PM, Jonathan Wakely  wrote:
> On 14/02/17 13:59 -0800, Tim Shen via libstdc++ wrote:
>>
>> This is an obvious missing std::forward. :)
>
>
> I was about to look into it, I assumed it would be something simple!
>
>> diff --git a/libstdc++-v3/testsuite/20_util/variant/compile.cc
>> b/libstdc++-v3/testsuite/20_util/variant/compile.cc
>> index 65f4326c397..d40a4ccb784 100644
>> --- a/libstdc++-v3/testsuite/20_util/variant/compile.cc
>> +++ b/libstdc++-v3/testsuite/20_util/variant/compile.cc
>> @@ -291,6 +291,13 @@ void test_visit()
>> };
>> static_assert(visit(Visitor(), variant(0)), "");
>>   }
>> +  // PR libstdc++/79513
>> +  {
>> +std::variant v(5);
>> +std::visit([](int&){}, v);
>> +std::visit([](int&&){}, std::move(v));
>> +(void)v;
>
>
> Is this to suppress an unused variable warning?
>
> If it is, please use an attribute instead, as it's more reliable:
>
>std::variant v __attribute__((unused)) (5);

Even better, I used the shiny new [[gnu::unused]]. :)

>
> OK for trunk if testing passes, thanks.
>

Tested and committed.


-- 
Regards,
Tim Shen

[PATCH] Fix return type detection in visit()

2017-02-14 Thread Tim Shen via gcc-patches 
This is an obvious missing std::forward. :)

Testing on x86_64-linux-gnu, but I expect it to pass.

-- 
Regards,
Tim Shen
commit 08235141a7e06db2b604b5869c9d8e4aaf8fa29b
Author: Tim Shen 
Date:   Tue Feb 14 13:55:18 2017 -0800

2017-02-14  Tim Shen  

PR libstdc++/79513
* include/std/variant (visit()): Forward variant types to the return
type detection code.
* testsuite/20_util/variant/compile.cc: Add test cases.

diff --git a/libstdc++-v3/include/std/variant b/libstdc++-v3/include/std/variant
index c5138e56803..866c4c40a61 100644
--- a/libstdc++-v3/include/std/variant
+++ b/libstdc++-v3/include/std/variant
@@ -1263,7 +1263,8 @@ _GLIBCXX_BEGIN_NAMESPACE_VERSION
 	__throw_bad_variant_access("Unexpected index");
 
   using _Result_type =
-	decltype(std::forward<_Visitor>(__visitor)(get<0>(__variants)...));
+	decltype(std::forward<_Visitor>(__visitor)(
+	get<0>(std::forward<_Variants>(__variants))...));
 
   constexpr auto& __vtable = __detail::__variant::__gen_vtable<
 	_Result_type, _Visitor&&, _Variants&&...>::_S_vtable;
diff --git a/libstdc++-v3/testsuite/20_util/variant/compile.cc b/libstdc++-v3/testsuite/20_util/variant/compile.cc
index 65f4326c397..d40a4ccb784 100644
--- a/libstdc++-v3/testsuite/20_util/variant/compile.cc
+++ b/libstdc++-v3/testsuite/20_util/variant/compile.cc
@@ -291,6 +291,13 @@ void test_visit()
 };
 static_assert(visit(Visitor(), variant(0)), "");
   }
+  // PR libstdc++/79513
+  {
+std::variant v(5);
+std::visit([](int&){}, v);
+std::visit([](int&&){}, std::move(v));
+(void)v;
+  }
 }
 
 void test_constexpr()

[PATCH] Reduce _GLIBCXX_REGEX_STATE_LIMIT

2017-02-10 Thread Tim Shen via gcc-patches 
Thanks Kostya for the fuzzing work!

Reduce it to a reasonably small number (but not too small), so that
libFuzzer doesn't find as many crashers (none in a short period of
time, actually) with a 8MB stack on a 64-bit machine.

Thanks!


-- 
Regards,
Tim Shen
commit 4021ce78ed48215e7b765e8879ca65612933ee62
Author: Tim Shen 
Date:   Fri Feb 10 17:34:45 2017 -0800

2017-02-10  Tim Shen  

* include/bits/regex_automaton.h: Reduce _GLIBCXX_REGEX_STATE_LIMIT
to 16384, that removes most stack overflow cases detected by
libFuzzer with a 8MB stack.
* testsuite/28_regex/regression.cc: Add a test.

diff --git a/libstdc++-v3/include/bits/regex_automaton.h b/libstdc++-v3/include/bits/regex_automaton.h
index a5fab6356cc..20fd02cea8f 100644
--- a/libstdc++-v3/include/bits/regex_automaton.h
+++ b/libstdc++-v3/include/bits/regex_automaton.h
@@ -30,7 +30,7 @@
 
 // This macro defines the maximal state number a NFA can have.
 #ifndef _GLIBCXX_REGEX_STATE_LIMIT
-#define _GLIBCXX_REGEX_STATE_LIMIT 10
+#define _GLIBCXX_REGEX_STATE_LIMIT 16384
 #endif
 
 namespace std _GLIBCXX_VISIBILITY(default)
diff --git a/libstdc++-v3/testsuite/28_regex/regression.cc b/libstdc++-v3/testsuite/28_regex/regression.cc
index b73b7641710..d3ffac81c77 100644
--- a/libstdc++-v3/testsuite/28_regex/regression.cc
+++ b/libstdc++-v3/testsuite/28_regex/regression.cc
@@ -110,6 +110,23 @@ test07()
   VERIFY(thrown);
 }
 
+// Too many states.
+void
+test08()
+{
+  bool thrown = false;
+  try
+{
+  std::regex re(".*{100}{300}");
+}
+  catch (const std::regex_error )
+{
+  if (e.code() == regex_constants::error_space)
+thrown = true;
+}
+  VERIFY(thrown);
+}
+
 int
 main()
 {
@@ -120,6 +137,7 @@ main()
   test05();
   test06();
   test07();
+  test08();
   return 0;
 }

Re: [PATCH] Fix the UB in regex caused by long decimal string

2017-02-10 Thread Tim Shen via gcc-patches 
Add gcc-patches.

On Fri, Feb 10, 2017 at 5:48 PM, Tim Shen  wrote:
> Oops I attached a diff without the ChangeLog. Attach again.
>
> --
> Regards,
> Tim Shen



-- 
Regards,
Tim Shen

Re: [v3 PATCH] Reduce the size of variant, it doesn't need an index of type size_t internally.

2017-01-10 Thread Tim Shen via gcc-patches 
On Tue, Jan 10, 2017 at 2:19 PM, Ville Voutilainen
 wrote:
> Cleanups based on review; there's no longer any public typedefs added
> to variant,
> and the test is greatly simpler with much less trickery.

Looks good to me.

Thanks!


-- 
Regards,
Tim Shen

Re: [PATCH] Implement P0393R3

2017-01-08 Thread Tim Shen via gcc-patches 
On Tue, Jan 3, 2017 at 6:17 AM, Jonathan Wakely  wrote:
> On 01/01/17 04:17 -0800, Tim Shen via libstdc++ wrote:
>>
>> +#define _VARIANT_RELATION_FUNCTION_TEMPLATE(__op, __name) \
>> +  template \
>> +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.

>
> Still reviewing the rest ...
>



-- 
Regards,
Tim Shen
commit fba8c3c8cca773a501766aff90b13f72e42d9355
Author: Tim Shen 
Date:   Sun Jan 1 04:07:15 2017 -0800

2017-01-01  Tim Shen  

PR libstdc++/78723
* include/std/variant: Implement P0393R3.
* testsuite/20_util/variant/compile.cc: Adjust tests.
* testsuite/20_util/variant/run.cc: Adjust tests.

diff --git a/libstdc++-v3/include/std/variant b/libstdc++-v3/include/std/variant
index 3d025a7..9ca61d6 100644
--- a/libstdc++-v3/include/std/variant
+++ b/libstdc++-v3/include/std/variant
@@ -263,21 +263,23 @@ _GLIBCXX_BEGIN_NAMESPACE_VERSION
   swap(__ref_cast<_Lhs>(__lhs), __ref_cast<_Rhs>(__rhs));
 }
 
-  template
-constexpr bool
-__erased_equal_to(_Variant&& __lhs, _Variant&& __rhs)
-{
-  return __get<_Np>(std::forward<_Variant>(__lhs))
- == __get<_Np>(std::forward<_Variant>(__rhs));
+#define _VARIANT_RELATION_FUNCTION_TEMPLATE(__op, __function_name) \
+  template \
+constexpr bool \
+__function_name(const _Variant& __lhs, const _Variant& __rhs) \
+{ \
+  return __get<_Np>(std::forward<_Variant>(__lhs)) \
+ __op __get<_Np>(std::forward<_Variant>(__rhs)); \
 }
 
-  template
-constexpr bool
-__erased_less_than(const _Variant& __lhs, const _Variant& __rhs)
-{
-  return __get<_Np>(std::forward<_Variant>(__lhs))
- < __get<_Np>(std::forward<_Variant>(__rhs));
-}
+  _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)
+
+#undef _VARIANT_RELATION_FUNCTION_TEMPLATE
 
   template
 constexpr size_t
@@ -800,63 +802,31 @@ _GLIBCXX_BEGIN_NAMESPACE_VERSION
   return get_if<__detail::__variant::__index_of_v<_Tp, _Types...>>(__ptr);
 }
 
-  template
-constexpr bool operator==(const variant<_Types...>& __lhs,
- const variant<_Types...>& __rhs)
-{
-  return __lhs._M_equal_to(__rhs, std::index_sequence_for<_Types...>{});
-}
-
-  template
-constexpr inline bool
-operator!=(const variant<_Types...>& __lhs, const variant<_Types...>& 
__rhs)
-{ return !(__lhs == __rhs); }
-
-  template
-constexpr inline bool
-operator<(const variant<_Types...>& __lhs, const variant<_Types...>& __rhs)
-{
-  return __lhs._M_less_than(__rhs, std::index_sequence_for<_Types...>{});
-}
-
-  template
-constexpr inline bool
-operator>(const variant<_Types...>& __lhs, const variant<_Types...>& __rhs)
-{ return __rhs < __lhs; }
-
-  template
-constexpr inline bool
-operator<=(const variant<_Types...>& __lhs, const variant<_Types...>& 
__rhs)
-{ return !(__lhs > __rhs); }
+  struct monostate { };
 
-  template
-constexpr inline bool
-operator>=(const variant<_Types...>& __lhs, const variant<_Types...>& 
__rhs)
-{ return !(__lhs < __rhs); }
+#define _VARIANT_RELATION_FUNCTION_TEMPLATE(__op, __name) \
+  template \
+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(>,

Re: [PATCH] s/__unused/__dimensions/ in

2017-01-04 Thread Tim Shen via gcc-patches 
On Wed, Jan 4, 2017 at 7:16 PM, Tim Shen  wrote:
> Since it's an obvious textual fix, I'm going to check it in directly.

Checked in with 80-columns limit respected. I have confusing tabstop
settings... :/


-- 
Regards,
Tim Shen

[PATCH] s/__unused/__dimensions/ in

2017-01-04 Thread Tim Shen via gcc-patches 
Since it's an obvious textual fix, I'm going to check it in directly.

-- 
Regards,
Tim Shen
commit b005916b043f52feed32b646f86ced80f3e5392b
Author: Tim Shen 
Date:   Wed Jan 4 19:07:07 2017 -0800

2017-01-05  Tim Shen  

PR libstdc++/78996
* include/std/variant: rename __unused to __dimensions to avoid
naming conflict.

diff --git a/libstdc++-v3/include/std/variant b/libstdc++-v3/include/std/variant
index 899198a..da57ed6 100644
--- a/libstdc++-v3/include/std/variant
+++ b/libstdc++-v3/include/std/variant
@@ -604,17 +604,18 @@ _GLIBCXX_BEGIN_NAMESPACE_VERSION
   template
 struct __gen_vtable_impl;
 
-  template
 struct __gen_vtable_impl<
-   _Multi_array<_Result_type (*)(_Visitor, _Variants...), __unused...>,
+   _Multi_array<_Result_type (*)(_Visitor, _Variants...), __dimensions...>,
tuple<_Variants...>, std::index_sequence<__indices...>>
 {
   using _Next =
  remove_reference_t::type>;
   using _Array_type =
- _Multi_array<_Result_type (*)(_Visitor, _Variants...), __unused...>;
+ _Multi_array<_Result_type (*)(_Visitor, _Variants...),
+  __dimensions...>;
 
   static constexpr _Array_type
   _S_apply()