public inbox for gcc-patches@gcc.gnu.org
 help / color / mirror / Atom feed
* experimental testsuite patch
@ 2014-01-15 16:56 François Dumont
  2014-01-15 17:17 ` Jonathan Wakely
  0 siblings, 1 reply; 6+ messages in thread
From: François Dumont @ 2014-01-15 16:56 UTC (permalink / raw)
  To: libstdc++, gcc-patches

[-- Attachment #1: Type: text/plain, Size: 1449 bytes --]

Hi

     Here is a patch to partially fix 2 string_view tests. It looks like 
we can't use 2 dg-options into the same source, one override the over, 
the dg-options directives have been merged into 1. I also update a 
script to make the experimental folder part of the folders to look for 
tests. I don't know if it was intentionally omitted because of the 
experimental aspect of what is tested, if so just tell me I won't apply 
this part.

     Remaining failures in string_view tests in debug mode are all 
coming from this kind of code:

       constexpr const _CharT&
       operator[](size_type __pos) const
       {
     _GLIBCXX_DEBUG_ASSERT(__pos <= this->_M_len);
     return *(this->_M_str + __pos);
       }

     In debug mode the _GLIBCXX_DEBUG_ASSERT is activated and the 
operator cannot be a constexpr anymore.  Maybe Ed can tell what should 
be done, remove the assertion or remove the constexpr (maybe only in 
debug mode ?) ?

2014-01-15  François Dumont <fdumont@gcc.gnu.org>

     * scripts/create_testsuite_files: Add testsuite/experimental in
     the list of folders to introspect for tests.
     * testsuite/experimental/string_view/element_access/wchar_t/2.cc:
     Merge dg-options directives into one.
     * testsuite/experimental/string_view/element_access/char/2.cc:
     Likewise. Remove invalid experimental namespace scope on
     string_view_type.

Tested under Linux x86_64 normal and debug modes.

François


[-- Attachment #2: experimental.patch --]
[-- Type: text/x-patch, Size: 1969 bytes --]

Index: scripts/create_testsuite_files
===================================================================
--- scripts/create_testsuite_files	(revision 206587)
+++ scripts/create_testsuite_files	(working copy)
@@ -32,7 +32,7 @@
 # This is the ugly version of "everything but the current directory".  It's
 # what has to happen when find(1) doesn't support -mindepth, or -xtype.
 dlist=`echo [0-9][0-9]*`
-dlist="$dlist abi backward ext performance tr1 tr2 decimal"
+dlist="$dlist abi backward ext performance tr1 tr2 decimal experimental"
 find $dlist "(" -type f -o -type l ")" -name "*.cc" -print > $tmp.01
 find $dlist "(" -type f -o -type l ")" -name "*.c" -print > $tmp.02
 cat  $tmp.01 $tmp.02 | sort > $tmp.1
Index: testsuite/experimental/string_view/element_access/char/2.cc
===================================================================
--- testsuite/experimental/string_view/element_access/char/2.cc	(revision 206587)
+++ testsuite/experimental/string_view/element_access/char/2.cc	(working copy)
@@ -1,6 +1,5 @@
-// { dg-options "-std=gnu++1y" }
 // { dg-do run { xfail *-*-* } }
-// { dg-options "-O0" }
+// { dg-options "-std=gnu++1y -O0" }
 // { dg-require-debug-mode "" }
 
 // Copyright (C) 2013-2014 Free Software Foundation, Inc.
@@ -26,6 +25,6 @@
 main()
 {
   typedef std::experimental::string_view string_view_type;
-  experimental::string_view_type s;
+  string_view_type s;
   s[0]; // abort
 }
Index: testsuite/experimental/string_view/element_access/wchar_t/2.cc
===================================================================
--- testsuite/experimental/string_view/element_access/wchar_t/2.cc	(revision 206587)
+++ testsuite/experimental/string_view/element_access/wchar_t/2.cc	(working copy)
@@ -1,6 +1,5 @@
-// { dg-options "-std=gnu++1y" }
 // { dg-do run { xfail *-*-* } }
-// { dg-options "-O0" }
+// { dg-options "-std=gnu++1y -O0" }
 // { dg-require-debug-mode "" }
 
 // Copyright (C) 2013-2014 Free Software Foundation, Inc.


^ permalink raw reply	[flat|nested] 6+ messages in thread

* Re: experimental testsuite patch
  2014-01-15 16:56 experimental testsuite patch François Dumont
@ 2014-01-15 17:17 ` Jonathan Wakely
  2014-01-15 17:22   ` Paolo Carlini
  0 siblings, 1 reply; 6+ messages in thread
From: Jonathan Wakely @ 2014-01-15 17:17 UTC (permalink / raw)
  To: François Dumont; +Cc: libstdc++, gcc-patches

On 15 January 2014 16:56, François Dumont wrote:
> Hi
>
>     Here is a patch to partially fix 2 string_view tests. It looks like we
> can't use 2 dg-options into the same source, one override the over, the
> dg-options directives have been merged into 1. I also update a script to
> make the experimental folder part of the folders to look for tests. I don't
> know if it was intentionally omitted because of the experimental aspect of
> what is tested, if so just tell me I won't apply this part.

I think it's OK.

>     Remaining failures in string_view tests in debug mode are all coming
> from this kind of code:
>
>       constexpr const _CharT&
>       operator[](size_type __pos) const
>       {
>     _GLIBCXX_DEBUG_ASSERT(__pos <= this->_M_len);
>     return *(this->_M_str + __pos);
>       }
>
>     In debug mode the _GLIBCXX_DEBUG_ASSERT is activated and the operator
> cannot be a constexpr anymore.  Maybe Ed can tell what should be done,
> remove the assertion or remove the constexpr (maybe only in debug mode ?) ?

I think we decided we want functions to be constexpr in debug mode if
they are constexpr in normal mode.

I think std::array has solved the same problem without losing the
constexpr qualifier.

> 2014-01-15  François Dumont <fdumont@gcc.gnu.org>
>
>     * scripts/create_testsuite_files: Add testsuite/experimental in
>     the list of folders to introspect for tests.

s/introspect/inspect/

The patch is OK with that change, thanks.,

^ permalink raw reply	[flat|nested] 6+ messages in thread

* Re: experimental testsuite patch
  2014-01-15 17:17 ` Jonathan Wakely
@ 2014-01-15 17:22   ` Paolo Carlini
  2014-01-15 17:43     ` Jonathan Wakely
  0 siblings, 1 reply; 6+ messages in thread
From: Paolo Carlini @ 2014-01-15 17:22 UTC (permalink / raw)
  To: Jonathan Wakely, François Dumont; +Cc: libstdc++, gcc-patches

On 01/15/2014 06:17 PM, Jonathan Wakely wrote:
> I think we decided we want functions to be constexpr in debug mode if
> they are constexpr in normal mode.
>
> I think std::array has solved the same problem without losing the
> constexpr qualifier.
Yes, I think the only complete solution we know of is that kind of 
strategy. But IMO it could wait, for the time being we could just 
comment out the checks and add comments about that (make sure first that 
the various make check* are clean as we are approaching the release of 
4.9.0)

Paolo.

^ permalink raw reply	[flat|nested] 6+ messages in thread

* Re: experimental testsuite patch
  2014-01-15 17:22   ` Paolo Carlini
@ 2014-01-15 17:43     ` Jonathan Wakely
  2014-01-19 20:50       ` François Dumont
  0 siblings, 1 reply; 6+ messages in thread
From: Jonathan Wakely @ 2014-01-15 17:43 UTC (permalink / raw)
  To: Paolo Carlini; +Cc: François Dumont, libstdc++, gcc-patches

On 15 January 2014 17:22, Paolo Carlini wrote:
> On 01/15/2014 06:17 PM, Jonathan Wakely wrote:
>>
>> I think we decided we want functions to be constexpr in debug mode if
>> they are constexpr in normal mode.
>>
>> I think std::array has solved the same problem without losing the
>> constexpr qualifier.
>
> Yes, I think the only complete solution we know of is that kind of strategy.
> But IMO it could wait, for the time being we could just comment out the
> checks and add comments about that (make sure first that the various make
> check* are clean as we are approaching the release of 4.9.0)

Yes, that would be the safest option. The assertions are not essential.

^ permalink raw reply	[flat|nested] 6+ messages in thread

* Re: experimental testsuite patch
  2014-01-15 17:43     ` Jonathan Wakely
@ 2014-01-19 20:50       ` François Dumont
  2014-01-20  9:41         ` Jonathan Wakely
  0 siblings, 1 reply; 6+ messages in thread
From: François Dumont @ 2014-01-19 20:50 UTC (permalink / raw)
  To: Jonathan Wakely; +Cc: Paolo Carlini, libstdc++, gcc-patches

[-- Attachment #1: Type: text/plain, Size: 1588 bytes --]

So here is another patch proposal with the faulty debug assertion 
commented for the moment.

2014-01-20  François Dumont  <fdumont@gcc.gnu.org>

     * scripts/create_testsuite_files: Add testsuite/experimental in
     the list of folders to introspect for tests.
     * include/experimental/string_view
     (basic_string_view<>::operator[]): Comment _GLIBCXX_DEBUG_ASSERT,
     incompatible with constexpr qualifier.
     (basic_string_view<>::front()): Likewise.
     (basic_string_view<>::back()): Likewise.
     * testsuite/experimental/string_view/element_access/wchar_t/2.cc:
     Merge dg-options directives into one.
     * testsuite/experimental/string_view/element_access/char/2.cc:
     Likewise. Remove invalid experimental namespace scope on
     string_view_type.

Tested under Linux x86_64 normal, debug modes.

Ok to commit ?

François

On 01/15/2014 06:43 PM, Jonathan Wakely wrote:
> On 15 January 2014 17:22, Paolo Carlini wrote:
>> On 01/15/2014 06:17 PM, Jonathan Wakely wrote:
>>> I think we decided we want functions to be constexpr in debug mode if
>>> they are constexpr in normal mode.
>>>
>>> I think std::array has solved the same problem without losing the
>>> constexpr qualifier.
>> Yes, I think the only complete solution we know of is that kind of strategy.
>> But IMO it could wait, for the time being we could just comment out the
>> checks and add comments about that (make sure first that the various make
>> check* are clean as we are approaching the release of 4.9.0)
> Yes, that would be the safest option. The assertions are not essential.
>


[-- Attachment #2: experimental.patch --]
[-- Type: text/x-patch, Size: 3047 bytes --]

Index: include/experimental/string_view
===================================================================
--- include/experimental/string_view	(revision 206782)
+++ include/experimental/string_view	(working copy)
@@ -181,7 +181,8 @@
       constexpr const _CharT&
       operator[](size_type __pos) const
       {
-	_GLIBCXX_DEBUG_ASSERT(__pos <= this->_M_len);
+	// TODO: Assert to restore in a way compatible with the constexpr.
+	// _GLIBCXX_DEBUG_ASSERT(__pos <= this->_M_len);
 	return *(this->_M_str + __pos);
       }
 
@@ -200,14 +201,16 @@
       constexpr const _CharT&
       front() const
       {
-	_GLIBCXX_DEBUG_ASSERT(this->_M_len > 0);
+	// TODO: Assert to restore in a way compatible with the constexpr.
+	// _GLIBCXX_DEBUG_ASSERT(this->_M_len > 0);
 	return *this->_M_str;
       }
 
       constexpr const _CharT&
       back() const
       {
-	_GLIBCXX_DEBUG_ASSERT(this->_M_len > 0);
+	// TODO: Assert to restore in a way compatible with the constexpr.
+	// _GLIBCXX_DEBUG_ASSERT(this->_M_len > 0);
 	return *(this->_M_str + this->_M_len - 1);
       }
 
Index: scripts/create_testsuite_files
===================================================================
--- scripts/create_testsuite_files	(revision 206782)
+++ scripts/create_testsuite_files	(working copy)
@@ -32,7 +32,7 @@
 # This is the ugly version of "everything but the current directory".  It's
 # what has to happen when find(1) doesn't support -mindepth, or -xtype.
 dlist=`echo [0-9][0-9]*`
-dlist="$dlist abi backward ext performance tr1 tr2 decimal"
+dlist="$dlist abi backward ext performance tr1 tr2 decimal experimental"
 find $dlist "(" -type f -o -type l ")" -name "*.cc" -print > $tmp.01
 find $dlist "(" -type f -o -type l ")" -name "*.c" -print > $tmp.02
 cat  $tmp.01 $tmp.02 | sort > $tmp.1
Index: testsuite/experimental/string_view/element_access/wchar_t/2.cc
===================================================================
--- testsuite/experimental/string_view/element_access/wchar_t/2.cc	(revision 206782)
+++ testsuite/experimental/string_view/element_access/wchar_t/2.cc	(working copy)
@@ -1,6 +1,5 @@
-// { dg-options "-std=gnu++1y" }
 // { dg-do run { xfail *-*-* } }
-// { dg-options "-O0" }
+// { dg-options "-std=gnu++1y -O0" }
 // { dg-require-debug-mode "" }
 
 // Copyright (C) 2013-2014 Free Software Foundation, Inc.
Index: testsuite/experimental/string_view/element_access/char/2.cc
===================================================================
--- testsuite/experimental/string_view/element_access/char/2.cc	(revision 206782)
+++ testsuite/experimental/string_view/element_access/char/2.cc	(working copy)
@@ -1,6 +1,5 @@
-// { dg-options "-std=gnu++1y" }
 // { dg-do run { xfail *-*-* } }
-// { dg-options "-O0" }
+// { dg-options "-std=gnu++1y -O0" }
 // { dg-require-debug-mode "" }
 
 // Copyright (C) 2013-2014 Free Software Foundation, Inc.
@@ -26,6 +25,6 @@
 main()
 {
   typedef std::experimental::string_view string_view_type;
-  experimental::string_view_type s;
+  string_view_type s;
   s[0]; // abort
 }

^ permalink raw reply	[flat|nested] 6+ messages in thread

* Re: experimental testsuite patch
  2014-01-19 20:50       ` François Dumont
@ 2014-01-20  9:41         ` Jonathan Wakely
  0 siblings, 0 replies; 6+ messages in thread
From: Jonathan Wakely @ 2014-01-20  9:41 UTC (permalink / raw)
  To: François Dumont; +Cc: Paolo Carlini, libstdc++, gcc-patches

On 19 January 2014 20:50, François Dumont wrote:
> So here is another patch proposal with the faulty debug assertion commented
> for the moment.
>
> 2014-01-20  François Dumont  <fdumont@gcc.gnu.org>
>
>
>     * scripts/create_testsuite_files: Add testsuite/experimental in
>     the list of folders to introspect for tests.
>     * include/experimental/string_view
>     (basic_string_view<>::operator[]): Comment _GLIBCXX_DEBUG_ASSERT,
>     incompatible with constexpr qualifier.
>     (basic_string_view<>::front()): Likewise.
>     (basic_string_view<>::back()): Likewise.
>
>     * testsuite/experimental/string_view/element_access/wchar_t/2.cc:
>     Merge dg-options directives into one.
>     * testsuite/experimental/string_view/element_access/char/2.cc:
>     Likewise. Remove invalid experimental namespace scope on
>     string_view_type.
>
> Tested under Linux x86_64 normal, debug modes.
>
> Ok to commit ?

Yes, but please change "introspect" in the ChangeLog to just "inspect"
or "search" - thanks.

^ permalink raw reply	[flat|nested] 6+ messages in thread

end of thread, other threads:[~2014-01-20  9:41 UTC | newest]

Thread overview: 6+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2014-01-15 16:56 experimental testsuite patch François Dumont
2014-01-15 17:17 ` Jonathan Wakely
2014-01-15 17:22   ` Paolo Carlini
2014-01-15 17:43     ` Jonathan Wakely
2014-01-19 20:50       ` François Dumont
2014-01-20  9:41         ` Jonathan Wakely

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for read-only IMAP folder(s) and NNTP newsgroup(s).