public inbox for libstdc++@gcc.gnu.org
 help / color / mirror / Atom feed
* [PATCH][_GLIBCXX_DEBUG] Add basic_string::starts_with/ends_with checks
@ 2022-08-14 15:32 François Dumont
  2022-08-15 20:26 ` François Dumont
  2022-08-26  9:31 ` Jonathan Wakely
  0 siblings, 2 replies; 7+ messages in thread
From: François Dumont @ 2022-08-14 15:32 UTC (permalink / raw)
  To: libstdc++; +Cc: gcc-patches

I think we can add those checks.

Note that I wonder if it was needed as in basic_string_view I see usages 
of __attribute__((__nonnull__)). But running the test I saw no impact 
even after I try to apply this attribute to the starts_with/ends_with 
methods themselves.

Also note that several checks like the ones I am adding here are XFAILS 
when using 'make check' because of the segfault rather than on a proper 
debug checks. Would you prefer to add dg-require-debug-mode to those ?

     libstdc++: [_GLIBCXX_DEBUG] Add basic_string::starts_with/ends_with 
checks

     Add simple checks on C string parameters which should not be null.

     Review null string checks to show:
     _String != nullptr

     rather than:
     _String != 0

     libstdc++-v3/ChangeLog:

             * include/bits/basic_string.h (starts_with, ends_with): Add 
__glibcxx_check_string.
             * include/bits/cow_string.h (starts_with, ends_with): Likewise.
             * include/debug/debug.h: Use nullptr rather than '0' in 
checks in C++11.
             * include/debug/string: Likewise.
             * 
testsuite/21_strings/basic_string/operations/ends_with/char.cc: Use 
__gnu_test::string.
             * 
testsuite/21_strings/basic_string/operations/ends_with/wchar_t.cc: Use 
__gnu_test::wstring.
             * 
testsuite/21_strings/basic_string/operations/starts_with/wchar_t.cc: Use 
__gnu_test::wstring.
             * 
testsuite/21_strings/basic_string/operations/starts_with/char.cc: Use 
__gnu_test::string.
             * 
testsuite/21_strings/basic_string/operations/ends_with/char_neg.cc: New 
test.
             * 
testsuite/21_strings/basic_string/operations/ends_with/wchar_t_neg.cc: 
New test.
             * 
testsuite/21_strings/basic_string/operations/starts_with/char_neg.cc: 
New test.
             * 
testsuite/21_strings/basic_string/operations/starts_with/wchar_t_neg.cc: 
New test.

Tested under linux normal and debug modes.

François



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

* Re: [PATCH][_GLIBCXX_DEBUG] Add basic_string::starts_with/ends_with checks
  2022-08-14 15:32 [PATCH][_GLIBCXX_DEBUG] Add basic_string::starts_with/ends_with checks François Dumont
@ 2022-08-15 20:26 ` François Dumont
  2022-08-25 16:11   ` François Dumont
  2022-08-26  9:31 ` Jonathan Wakely
  1 sibling, 1 reply; 7+ messages in thread
From: François Dumont @ 2022-08-15 20:26 UTC (permalink / raw)
  To: libstdc++; +Cc: gcc-patches

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

With the patch !

On 14/08/22 17:32, François Dumont wrote:
> I think we can add those checks.
>
> Note that I wonder if it was needed as in basic_string_view I see 
> usages of __attribute__((__nonnull__)). But running the test I saw no 
> impact even after I try to apply this attribute to the 
> starts_with/ends_with methods themselves.
>
> Also note that several checks like the ones I am adding here are 
> XFAILS when using 'make check' because of the segfault rather than on 
> a proper debug checks. Would you prefer to add dg-require-debug-mode 
> to those ?
>
>     libstdc++: [_GLIBCXX_DEBUG] Add 
> basic_string::starts_with/ends_with checks
>
>     Add simple checks on C string parameters which should not be null.
>
>     Review null string checks to show:
>     _String != nullptr
>
>     rather than:
>     _String != 0
>
>     libstdc++-v3/ChangeLog:
>
>             * include/bits/basic_string.h (starts_with, ends_with): 
> Add __glibcxx_check_string.
>             * include/bits/cow_string.h (starts_with, ends_with): 
> Likewise.
>             * include/debug/debug.h: Use nullptr rather than '0' in 
> checks in C++11.
>             * include/debug/string: Likewise.
>             * 
> testsuite/21_strings/basic_string/operations/ends_with/char.cc: Use 
> __gnu_test::string.
>             * 
> testsuite/21_strings/basic_string/operations/ends_with/wchar_t.cc: Use 
> __gnu_test::wstring.
>             * 
> testsuite/21_strings/basic_string/operations/starts_with/wchar_t.cc: 
> Use __gnu_test::wstring.
>             * 
> testsuite/21_strings/basic_string/operations/starts_with/char.cc: Use 
> __gnu_test::string.
>             * 
> testsuite/21_strings/basic_string/operations/ends_with/char_neg.cc: 
> New test.
>             * 
> testsuite/21_strings/basic_string/operations/ends_with/wchar_t_neg.cc: 
> New test.
>             * 
> testsuite/21_strings/basic_string/operations/starts_with/char_neg.cc: 
> New test.
>             * 
> testsuite/21_strings/basic_string/operations/starts_with/wchar_t_neg.cc: 
> New test.
>
> Tested under linux normal and debug modes.
>
> François
>
>

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

diff --git a/libstdc++-v3/include/bits/basic_string.h b/libstdc++-v3/include/bits/basic_string.h
index b04fba95678..d06330e6c48 100644
--- a/libstdc++-v3/include/bits/basic_string.h
+++ b/libstdc++-v3/include/bits/basic_string.h
@@ -3402,7 +3402,10 @@ _GLIBCXX_BEGIN_NAMESPACE_CXX11
 
       constexpr bool
       starts_with(const _CharT* __x) const noexcept
-      { return __sv_type(this->data(), this->size()).starts_with(__x); }
+      {
+	__glibcxx_requires_string(__x);
+	return __sv_type(this->data(), this->size()).starts_with(__x);
+      }
 
       constexpr bool
       ends_with(basic_string_view<_CharT, _Traits> __x) const noexcept
@@ -3414,7 +3417,10 @@ _GLIBCXX_BEGIN_NAMESPACE_CXX11
 
       constexpr bool
       ends_with(const _CharT* __x) const noexcept
-      { return __sv_type(this->data(), this->size()).ends_with(__x); }
+      {
+	__glibcxx_requires_string(__x);
+	return __sv_type(this->data(), this->size()).ends_with(__x);
+      }
 #endif // C++20
 
 #if __cplusplus > 202002L
diff --git a/libstdc++-v3/include/bits/cow_string.h b/libstdc++-v3/include/bits/cow_string.h
index f16e33ac1ef..59b36a1006a 100644
--- a/libstdc++-v3/include/bits/cow_string.h
+++ b/libstdc++-v3/include/bits/cow_string.h
@@ -3014,7 +3014,10 @@ _GLIBCXX_BEGIN_NAMESPACE_VERSION
 
       bool
       starts_with(const _CharT* __x) const noexcept
-      { return __sv_type(this->data(), this->size()).starts_with(__x); }
+      {
+	__glibcxx_requires_string(__x);
+	return __sv_type(this->data(), this->size()).starts_with(__x);
+      }
 
       bool
       ends_with(basic_string_view<_CharT, _Traits> __x) const noexcept
@@ -3026,7 +3029,10 @@ _GLIBCXX_BEGIN_NAMESPACE_VERSION
 
       bool
       ends_with(const _CharT* __x) const noexcept
-      { return __sv_type(this->data(), this->size()).ends_with(__x); }
+      {
+	__glibcxx_requires_string(__x);
+	return __sv_type(this->data(), this->size()).ends_with(__x);
+      }
 #endif // C++20
 
 #if __cplusplus > 202011L
diff --git a/libstdc++-v3/include/debug/debug.h b/libstdc++-v3/include/debug/debug.h
index 28e250f0c50..f4233760426 100644
--- a/libstdc++-v3/include/debug/debug.h
+++ b/libstdc++-v3/include/debug/debug.h
@@ -118,10 +118,17 @@ namespace __gnu_debug
   __glibcxx_check_heap(_First,_Last)
 # define __glibcxx_requires_heap_pred(_First,_Last,_Pred)	\
   __glibcxx_check_heap_pred(_First,_Last,_Pred)
-# define __glibcxx_requires_string(_String)	\
+# if __cplusplus < 201103L
+#  define __glibcxx_requires_string(_String)	\
   _GLIBCXX_DEBUG_PEDASSERT(_String != 0)
-# define __glibcxx_requires_string_len(_String,_Len)	\
+#  define __glibcxx_requires_string_len(_String,_Len)	\
   _GLIBCXX_DEBUG_PEDASSERT(_String != 0 || _Len == 0)
+# else
+#  define __glibcxx_requires_string(_String)	\
+  _GLIBCXX_DEBUG_PEDASSERT(_String != nullptr)
+#  define __glibcxx_requires_string_len(_String,_Len)	\
+  _GLIBCXX_DEBUG_PEDASSERT(_String != nullptr || _Len == 0)
+# endif
 # define __glibcxx_requires_irreflexive(_First,_Last)	\
   __glibcxx_check_irreflexive(_First,_Last)
 # define __glibcxx_requires_irreflexive2(_First,_Last)	\
diff --git a/libstdc++-v3/include/debug/string b/libstdc++-v3/include/debug/string
index c32eb41eacd..f02c74431ed 100644
--- a/libstdc++-v3/include/debug/string
+++ b/libstdc++-v3/include/debug/string
@@ -50,14 +50,25 @@
 #endif
 
 #ifdef _GLIBCXX_DEBUG_PEDANTIC
-# define __glibcxx_check_string(_String)				\
+# if __cplusplus < 201103L
+#  define __glibcxx_check_string(_String)			\
   _GLIBCXX_DEBUG_VERIFY_STR_COND_AT(_String != 0,		\
 				    __FILE__, __LINE__,		\
 				    __PRETTY_FUNCTION__);
-# define __glibcxx_check_string_len(_String,_Len)		\
+#  define __glibcxx_check_string_len(_String,_Len)		\
   _GLIBCXX_DEBUG_VERIFY_STR_COND_AT(_String != 0 || _Len == 0,	\
 				    __FILE__, __LINE__,		\
 				    __PRETTY_FUNCTION__);
+# else
+#  define __glibcxx_check_string(_String)			\
+  _GLIBCXX_DEBUG_VERIFY_STR_COND_AT(_String != nullptr,		\
+				    __FILE__, __LINE__,		\
+				    __PRETTY_FUNCTION__);
+#  define __glibcxx_check_string_len(_String,_Len)		\
+  _GLIBCXX_DEBUG_VERIFY_STR_COND_AT(_String != nullptr || _Len == 0,	\
+				    __FILE__, __LINE__,		\
+				    __PRETTY_FUNCTION__);
+# endif
 #else
 # define __glibcxx_check_string(_String)
 # define __glibcxx_check_string_len(_String,_Len)
@@ -75,8 +86,13 @@ namespace __gnu_debug
 		   const char* __function __attribute__((__unused__)))
     {
 #ifdef _GLIBCXX_DEBUG_PEDANTIC
+# if __cplusplus < 201103L
       _GLIBCXX_DEBUG_VERIFY_STR_COND_AT(__s != 0 || __n == 0,
 					__file, __line, __function);
+# else
+      _GLIBCXX_DEBUG_VERIFY_STR_COND_AT(__s != nullptr || __n == 0,
+					__file, __line, __function);
+# endif
 #endif
       return __s;
     }
@@ -90,8 +106,13 @@ namespace __gnu_debug
 		   const char* __function __attribute__((__unused__)))
     {
 #ifdef _GLIBCXX_DEBUG_PEDANTIC
+# if __cplusplus < 201103L
       _GLIBCXX_DEBUG_VERIFY_STR_COND_AT(__s != 0,
 					__file, __line, __function);
+# else
+      _GLIBCXX_DEBUG_VERIFY_STR_COND_AT(__s != nullptr,
+					__file, __line, __function);
+# endif
 #endif
       return __s;
     }
@@ -1033,6 +1054,26 @@ namespace __gnu_debug
 	return _Base::compare(__pos1, __n1, __s, __n2);
       }
 
+#if __cplusplus >= 202002L
+      using _Base::starts_with;
+
+      __attribute__((__nonnull__)) constexpr bool
+      starts_with(const _CharT* __x) const noexcept
+      {
+	__glibcxx_check_string(__x);
+	return _Base::starts_with(__x);
+      }
+
+      using _Base::ends_with;
+
+      constexpr bool
+      ends_with(const _CharT* __x) const noexcept
+      {
+	__glibcxx_check_string(__x);
+	return _Base::ends_with(__x);
+      }
+#endif
+
       _Base&
       _M_base() _GLIBCXX_NOEXCEPT		{ return *this; }
 
diff --git a/libstdc++-v3/testsuite/21_strings/basic_string/operations/ends_with/char.cc b/libstdc++-v3/testsuite/21_strings/basic_string/operations/ends_with/char.cc
index cfe18e0186c..3cf85121e36 100644
--- a/libstdc++-v3/testsuite/21_strings/basic_string/operations/ends_with/char.cc
+++ b/libstdc++-v3/testsuite/21_strings/basic_string/operations/ends_with/char.cc
@@ -20,7 +20,7 @@
 
 // basic_string ends_with
 
-#include <string>
+#include <testsuite_string.h>
 #include <testsuite_hooks.h>
 
 void
@@ -31,7 +31,7 @@ test01()
   const char cstr_suf2[] = ".rgb";
   const std::string_view sv_suf2(".rgb");
 
-  const std::string s_test("slugs/slimy.jpg");
+  const __gnu_test::string s_test("slugs/slimy.jpg");
 
   const auto cstr_in_slugs = s_test.ends_with(cstr_suf);
   VERIFY( cstr_in_slugs );
diff --git a/libstdc++-v3/testsuite/21_strings/basic_string/operations/ends_with/char_neg.cc b/libstdc++-v3/testsuite/21_strings/basic_string/operations/ends_with/char_neg.cc
new file mode 100644
index 00000000000..7a7b8dd077d
--- /dev/null
+++ b/libstdc++-v3/testsuite/21_strings/basic_string/operations/ends_with/char_neg.cc
@@ -0,0 +1,37 @@
+// Copyright (C) 2022 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
+// <http://www.gnu.org/licenses/>.
+//
+// { dg-options "-std=gnu++2a -O0" }
+// { dg-do run { target c++2a xfail *-*-* } }
+
+#define _GLIBCXX_DEBUG_PEDANTIC
+
+#include <testsuite_string.h>
+
+void
+test01()
+{
+  const __gnu_test::string s_test("slugs/slimy.jpg");
+  s_test.ends_with(nullptr);
+}
+
+int
+main()
+{
+  test01();
+  return 0;
+}
diff --git a/libstdc++-v3/testsuite/21_strings/basic_string/operations/ends_with/wchar_t.cc b/libstdc++-v3/testsuite/21_strings/basic_string/operations/ends_with/wchar_t.cc
index d27e8246fb8..70b522ff69e 100644
--- a/libstdc++-v3/testsuite/21_strings/basic_string/operations/ends_with/wchar_t.cc
+++ b/libstdc++-v3/testsuite/21_strings/basic_string/operations/ends_with/wchar_t.cc
@@ -20,7 +20,7 @@
 
 // basic_string ends_with
 
-#include <string>
+#include <testsuite_string.h>
 #include <testsuite_hooks.h>
 
 void
@@ -31,7 +31,7 @@ test01()
   const wchar_t cstr_suf2[] = L".rgb";
   const std::wstring_view sv_suf2(L".rgb");
 
-  const std::wstring s_test(L"slugs/slimy.jpg");
+  const __gnu_test::wstring s_test(L"slugs/slimy.jpg");
 
   const auto cstr_in_slugs = s_test.ends_with(cstr_suf);
   VERIFY( cstr_in_slugs );
diff --git a/libstdc++-v3/testsuite/21_strings/basic_string/operations/ends_with/wchar_t_neg.cc b/libstdc++-v3/testsuite/21_strings/basic_string/operations/ends_with/wchar_t_neg.cc
new file mode 100644
index 00000000000..a6881bf406b
--- /dev/null
+++ b/libstdc++-v3/testsuite/21_strings/basic_string/operations/ends_with/wchar_t_neg.cc
@@ -0,0 +1,37 @@
+// Copyright (C) 2022 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
+// <http://www.gnu.org/licenses/>.
+//
+// { dg-options "-std=gnu++2a -O0" }
+// { dg-do run { target c++2a xfail *-*-* } }
+
+#define _GLIBCXX_DEBUG_PEDANTIC
+
+#include <testsuite_string.h>
+
+void
+test01()
+{
+  const __gnu_test::wstring s_test(L"slugs/slimy.jpg");
+  s_test.ends_with(nullptr);
+}
+
+int
+main()
+{
+  test01();
+  return 0;
+}
diff --git a/libstdc++-v3/testsuite/21_strings/basic_string/operations/starts_with/char.cc b/libstdc++-v3/testsuite/21_strings/basic_string/operations/starts_with/char.cc
index 5c0b3afc0b6..dddf51cee16 100644
--- a/libstdc++-v3/testsuite/21_strings/basic_string/operations/starts_with/char.cc
+++ b/libstdc++-v3/testsuite/21_strings/basic_string/operations/starts_with/char.cc
@@ -20,7 +20,7 @@
 
 // basic_string begins_with
 
-#include <string>
+#include <testsuite_string.h>
 #include <testsuite_hooks.h>
 
 void
@@ -31,7 +31,7 @@ test01()
   const char cstr_dir2[] = "worms/";
   const std::string_view sv_dir2("worms/");
 
-  const std::string s_test("slugs/slimy.jpg");
+  const __gnu_test::string s_test("slugs/slimy.jpg");
 
   const auto cstr_in_slugs = s_test.starts_with(cstr_dir);
   VERIFY( cstr_in_slugs );
diff --git a/libstdc++-v3/testsuite/21_strings/basic_string/operations/starts_with/char_neg.cc b/libstdc++-v3/testsuite/21_strings/basic_string/operations/starts_with/char_neg.cc
new file mode 100644
index 00000000000..f357aef2289
--- /dev/null
+++ b/libstdc++-v3/testsuite/21_strings/basic_string/operations/starts_with/char_neg.cc
@@ -0,0 +1,37 @@
+// Copyright (C) 2022 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
+// <http://www.gnu.org/licenses/>.
+//
+// { dg-options "-std=gnu++2a -O0" }
+// { dg-do run { target c++2a xfail *-*-* } }
+
+#define _GLIBCXX_DEBUG_PEDANTIC
+
+#include <testsuite_string.h>
+
+void
+test01()
+{
+  const __gnu_test::string s_test("slugs/slimy.jpg");
+  s_test.starts_with(nullptr);
+}
+
+int
+main()
+{
+  test01();
+  return 0;
+}
diff --git a/libstdc++-v3/testsuite/21_strings/basic_string/operations/starts_with/wchar_t.cc b/libstdc++-v3/testsuite/21_strings/basic_string/operations/starts_with/wchar_t.cc
index eda73212ea0..747b23ae5c2 100644
--- a/libstdc++-v3/testsuite/21_strings/basic_string/operations/starts_with/wchar_t.cc
+++ b/libstdc++-v3/testsuite/21_strings/basic_string/operations/starts_with/wchar_t.cc
@@ -20,7 +20,7 @@
 
 // basic_string begins_with
 
-#include <string>
+#include <testsuite_string.h>
 #include <testsuite_hooks.h>
 
 void
@@ -31,7 +31,7 @@ test01()
   const wchar_t cstr_dir2[] = L"worms/";
   const std::wstring_view sv_dir2(L"worms/");
 
-  const std::wstring s_test(L"slugs/slimy.jpg");
+  const __gnu_test::wstring s_test(L"slugs/slimy.jpg");
 
   const auto cstr_in_slugs = s_test.starts_with(cstr_dir);
   VERIFY( cstr_in_slugs );
diff --git a/libstdc++-v3/testsuite/21_strings/basic_string/operations/starts_with/wchar_t_neg.cc b/libstdc++-v3/testsuite/21_strings/basic_string/operations/starts_with/wchar_t_neg.cc
new file mode 100644
index 00000000000..90065a459b6
--- /dev/null
+++ b/libstdc++-v3/testsuite/21_strings/basic_string/operations/starts_with/wchar_t_neg.cc
@@ -0,0 +1,37 @@
+// Copyright (C) 2022 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
+// <http://www.gnu.org/licenses/>.
+//
+// { dg-options "-std=gnu++2a -O0" }
+// { dg-do run { target c++2a xfail *-*-* } }
+
+#define _GLIBCXX_DEBUG_PEDANTIC
+
+#include <testsuite_string.h>
+
+void
+test01()
+{
+  const __gnu_test::wstring s_test(L"slugs/slimy.jpg");
+  s_test.starts_with(nullptr);
+}
+
+int
+main()
+{
+  test01();
+  return 0;
+}

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

* Re: [PATCH][_GLIBCXX_DEBUG] Add basic_string::starts_with/ends_with checks
  2022-08-15 20:26 ` François Dumont
@ 2022-08-25 16:11   ` François Dumont
  2022-08-26  9:33     ` Jonathan Wakely
  0 siblings, 1 reply; 7+ messages in thread
From: François Dumont @ 2022-08-25 16:11 UTC (permalink / raw)
  To: libstdc++

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

I spent more time on this.

Is there some discussion in the C++ Standard Committee to do something 
like what I've done ? I guess adding nullptr_t overloads for all the 
methods taking pointer is not an option, is it ?

Note that compilation fails but despite the dg-do xfail the test ends up 
FAIL and not XFAIL. Is this the correct syntax:
// { dg-do compile { target c++2a xfail *-*-* } }
?

Thanks,
François


On 15/08/22 22:26, François Dumont wrote:
> With the patch !
>
> On 14/08/22 17:32, François Dumont wrote:
>> I think we can add those checks.
>>
>> Note that I wonder if it was needed as in basic_string_view I see 
>> usages of __attribute__((__nonnull__)). But running the test I saw no 
>> impact even after I try to apply this attribute to the 
>> starts_with/ends_with methods themselves.
>>
>> Also note that several checks like the ones I am adding here are 
>> XFAILS when using 'make check' because of the segfault rather than on 
>> a proper debug checks. Would you prefer to add dg-require-debug-mode 
>> to those ?
>>
>>     libstdc++: [_GLIBCXX_DEBUG] Add 
>> basic_string::starts_with/ends_with checks
>>
>>     Add simple checks on C string parameters which should not be null.
>>
>>     Review null string checks to show:
>>     _String != nullptr
>>
>>     rather than:
>>     _String != 0
>>
>>     libstdc++-v3/ChangeLog:
>>
>>             * include/bits/basic_string.h (starts_with, ends_with): 
>> Add __glibcxx_check_string.
>>             * include/bits/cow_string.h (starts_with, ends_with): 
>> Likewise.
>>             * include/debug/debug.h: Use nullptr rather than '0' in 
>> checks in C++11.
>>             * include/debug/string: Likewise.
>>             * 
>> testsuite/21_strings/basic_string/operations/ends_with/char.cc: Use 
>> __gnu_test::string.
>>             * 
>> testsuite/21_strings/basic_string/operations/ends_with/wchar_t.cc: 
>> Use __gnu_test::wstring.
>>             * 
>> testsuite/21_strings/basic_string/operations/starts_with/wchar_t.cc: 
>> Use __gnu_test::wstring.
>>             * 
>> testsuite/21_strings/basic_string/operations/starts_with/char.cc: Use 
>> __gnu_test::string.
>>             * 
>> testsuite/21_strings/basic_string/operations/ends_with/char_neg.cc: 
>> New test.
>>             * 
>> testsuite/21_strings/basic_string/operations/ends_with/wchar_t_neg.cc: 
>> New test.
>>             * 
>> testsuite/21_strings/basic_string/operations/starts_with/char_neg.cc: 
>> New test.
>>             * 
>> testsuite/21_strings/basic_string/operations/starts_with/wchar_t_neg.cc: 
>> New test.
>>
>> Tested under linux normal and debug modes.
>>
>> François
>>
>>

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

diff --git a/libstdc++-v3/include/bits/basic_string.h b/libstdc++-v3/include/bits/basic_string.h
index d06330e6c48..1be42e415f5 100644
--- a/libstdc++-v3/include/bits/basic_string.h
+++ b/libstdc++-v3/include/bits/basic_string.h
@@ -3407,6 +3407,9 @@ _GLIBCXX_BEGIN_NAMESPACE_CXX11
 	return __sv_type(this->data(), this->size()).starts_with(__x);
       }
 
+      bool
+      starts_with(nullptr_t) const noexcept = delete;
+
       constexpr bool
       ends_with(basic_string_view<_CharT, _Traits> __x) const noexcept
       { return __sv_type(this->data(), this->size()).ends_with(__x); }
@@ -3421,6 +3424,9 @@ _GLIBCXX_BEGIN_NAMESPACE_CXX11
 	__glibcxx_requires_string(__x);
 	return __sv_type(this->data(), this->size()).ends_with(__x);
       }
+
+      bool
+      ends_with(nullptr_t) const noexcept = delete;
 #endif // C++20
 
 #if __cplusplus > 202002L
diff --git a/libstdc++-v3/include/bits/cow_string.h b/libstdc++-v3/include/bits/cow_string.h
index 59b36a1006a..f8f0959554d 100644
--- a/libstdc++-v3/include/bits/cow_string.h
+++ b/libstdc++-v3/include/bits/cow_string.h
@@ -3019,6 +3019,9 @@ _GLIBCXX_BEGIN_NAMESPACE_VERSION
 	return __sv_type(this->data(), this->size()).starts_with(__x);
       }
 
+      bool
+      starts_with(nullptr_t) const noexcept = delete;
+
       bool
       ends_with(basic_string_view<_CharT, _Traits> __x) const noexcept
       { return __sv_type(this->data(), this->size()).ends_with(__x); }
@@ -3033,6 +3036,9 @@ _GLIBCXX_BEGIN_NAMESPACE_VERSION
 	__glibcxx_requires_string(__x);
 	return __sv_type(this->data(), this->size()).ends_with(__x);
       }
+
+      bool
+      ends_with(nullptr_t) const noexcept = delete;
 #endif // C++20
 
 #if __cplusplus > 202011L
diff --git a/libstdc++-v3/testsuite/21_strings/basic_string/operations/ends_with/char_neg.cc b/libstdc++-v3/testsuite/21_strings/basic_string/operations/ends_with/char_neg.cc
index 7a7b8dd077d..273e9a6a564 100644
--- a/libstdc++-v3/testsuite/21_strings/basic_string/operations/ends_with/char_neg.cc
+++ b/libstdc++-v3/testsuite/21_strings/basic_string/operations/ends_with/char_neg.cc
@@ -15,10 +15,8 @@
 // with this library; see the file COPYING3.  If not see
 // <http://www.gnu.org/licenses/>.
 //
-// { dg-options "-std=gnu++2a -O0" }
-// { dg-do run { target c++2a xfail *-*-* } }
-
-#define _GLIBCXX_DEBUG_PEDANTIC
+// { dg-options "-std=gnu++2a" }
+// { dg-do compile { target c++2a xfail *-*-* } }
 
 #include <testsuite_string.h>
 
diff --git a/libstdc++-v3/testsuite/21_strings/basic_string/operations/ends_with/wchar_t_neg.cc b/libstdc++-v3/testsuite/21_strings/basic_string/operations/ends_with/wchar_t_neg.cc
index a6881bf406b..07588ea2bf7 100644
--- a/libstdc++-v3/testsuite/21_strings/basic_string/operations/ends_with/wchar_t_neg.cc
+++ b/libstdc++-v3/testsuite/21_strings/basic_string/operations/ends_with/wchar_t_neg.cc
@@ -15,10 +15,8 @@
 // with this library; see the file COPYING3.  If not see
 // <http://www.gnu.org/licenses/>.
 //
-// { dg-options "-std=gnu++2a -O0" }
-// { dg-do run { target c++2a xfail *-*-* } }
-
-#define _GLIBCXX_DEBUG_PEDANTIC
+// { dg-options "-std=gnu++2a" }
+// { dg-do compile { target c++2a xfail *-*-* } }
 
 #include <testsuite_string.h>
 
diff --git a/libstdc++-v3/testsuite/21_strings/basic_string/operations/starts_with/char_neg.cc b/libstdc++-v3/testsuite/21_strings/basic_string/operations/starts_with/char_neg.cc
index f357aef2289..9b6b0d2ddda 100644
--- a/libstdc++-v3/testsuite/21_strings/basic_string/operations/starts_with/char_neg.cc
+++ b/libstdc++-v3/testsuite/21_strings/basic_string/operations/starts_with/char_neg.cc
@@ -15,10 +15,8 @@
 // with this library; see the file COPYING3.  If not see
 // <http://www.gnu.org/licenses/>.
 //
-// { dg-options "-std=gnu++2a -O0" }
-// { dg-do run { target c++2a xfail *-*-* } }
-
-#define _GLIBCXX_DEBUG_PEDANTIC
+// { dg-options "-std=gnu++2a" }
+// { dg-do compile { target c++2a xfail *-*-* } }
 
 #include <testsuite_string.h>
 
diff --git a/libstdc++-v3/testsuite/21_strings/basic_string/operations/starts_with/wchar_t_neg.cc b/libstdc++-v3/testsuite/21_strings/basic_string/operations/starts_with/wchar_t_neg.cc
index 90065a459b6..1ea10d1ab39 100644
--- a/libstdc++-v3/testsuite/21_strings/basic_string/operations/starts_with/wchar_t_neg.cc
+++ b/libstdc++-v3/testsuite/21_strings/basic_string/operations/starts_with/wchar_t_neg.cc
@@ -15,10 +15,8 @@
 // with this library; see the file COPYING3.  If not see
 // <http://www.gnu.org/licenses/>.
 //
-// { dg-options "-std=gnu++2a -O0" }
-// { dg-do run { target c++2a xfail *-*-* } }
-
-#define _GLIBCXX_DEBUG_PEDANTIC
+// { dg-options "-std=gnu++2a" }
+// { dg-do compile { target c++2a xfail *-*-* } }
 
 #include <testsuite_string.h>
 

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

* Re: [PATCH][_GLIBCXX_DEBUG] Add basic_string::starts_with/ends_with checks
  2022-08-14 15:32 [PATCH][_GLIBCXX_DEBUG] Add basic_string::starts_with/ends_with checks François Dumont
  2022-08-15 20:26 ` François Dumont
@ 2022-08-26  9:31 ` Jonathan Wakely
  2022-08-31  4:38   ` [PATCH][_GLIBCXX_DEBUG] Review null string assertions (was: Add basic_string::starts_with/ends_with checks) François Dumont
  1 sibling, 1 reply; 7+ messages in thread
From: Jonathan Wakely @ 2022-08-26  9:31 UTC (permalink / raw)
  To: François Dumont; +Cc: libstdc++, gcc-patches

On Sun, 14 Aug 2022 at 16:34, François Dumont via Libstdc++
<libstdc++@gcc.gnu.org> wrote:
>
> I think we can add those checks.
>
> Note that I wonder if it was needed as in basic_string_view I see usages
> of __attribute__((__nonnull__)). But running the test I saw no impact
> even after I try to apply this attribute to the starts_with/ends_with
> methods themselves.

That should cause warnings, and does when I try it.

As you say, the relevant string_view constructor already has that anyway:

      __attribute__((__nonnull__)) constexpr
      basic_string_view(const _CharT* __str) noexcept

And so does string_view::find. The problem is that those only help if
the compiler inlines the calls to those functions and so can propagate
the null value all the way down to a function with the attribute.
Adding the attribute to the relevant starts_with, ends_with and
contains functions makes the diagnostics more likely to be emitted
without optimization.

>
> Also note that several checks like the ones I am adding here are XFAILS
> when using 'make check' because of the segfault rather than on a proper
> debug checks. Would you prefer to add dg-require-debug-mode to those ?
>
>      libstdc++: [_GLIBCXX_DEBUG] Add basic_string::starts_with/ends_with
> checks
>
>      Add simple checks on C string parameters which should not be null.
>
>      Review null string checks to show:
>      _String != nullptr
>
>      rather than:
>      _String != 0

I don't really like the extra complexity in the macros, but this does
seem like a nice improvement for what users see.

We could use __null for C++98, which is a compiler keyword that
expands to a null pointer constant, but I'm not sure if that would be
clear to all users or not. Maybe 0 is better.


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

* Re: [PATCH][_GLIBCXX_DEBUG] Add basic_string::starts_with/ends_with checks
  2022-08-25 16:11   ` François Dumont
@ 2022-08-26  9:33     ` Jonathan Wakely
  0 siblings, 0 replies; 7+ messages in thread
From: Jonathan Wakely @ 2022-08-26  9:33 UTC (permalink / raw)
  To: François Dumont; +Cc: libstdc++

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

On Thu, 25 Aug 2022 at 17:12, François Dumont via Libstdc++
<libstdc++@gcc.gnu.org> wrote:
>
> I spent more time on this.
>
> Is there some discussion in the C++ Standard Committee to do something
> like what I've done ? I guess adding nullptr_t overloads for all the
> methods taking pointer is not an option, is it ?

No, I don't think so. That doesn't really help anyway, it would only
trigger if somebody calls the function with the nullptr literal, which
seems unlikely. It wouldn't help for starts_with((const char*)0) or
passing a pointer variable that happens to be null. Passing a literal
nullptr to those functions is just dumb, I don't think we should waste
our time (and slow down compilation) by handling it.

If they cannot be called with a null pointer then they should use
__attribute__((__nonnull__)), and let the compiler handle it. That's
more reliable, works without assertions enabled, and handles (const
char*)0 as well as literal nullptr constants.

I'll test the attached patch, which does that.

> Note that compilation fails but despite the dg-do xfail the test ends up
> FAIL and not XFAIL. Is this the correct syntax:
> // { dg-do compile { target c++2a xfail *-*-* } }
> ?

No, xfail means fails at runtime.

You would want to use just { target c++20 ) and then add dg-error to
the lines that are expected to trigger errors, or dg-warning for
warnings.

[-- Attachment #2: patch.txt --]
[-- Type: text/plain, Size: 4862 bytes --]

commit c1a5d9ebe73b6a7512757c5cb247d20689fa6cb6
Author: Jonathan Wakely <jwakely@redhat.com>
Date:   Fri Aug 26 09:43:32 2022

    libstdc++: Add nonnull to starts_with/ends_with/contains string members
    
    Ideally this wouldn't be needed, because eventually these pointers all
    get passed to either the basic_string_view(const CharT*) constructor, or
    to basic_string_view::find(const CharT*), both of which already have the
    attribute. But for that to work requires optimization, so that the null
    value gets propagated through the call chain.
    
    Adding it explicitly to each member that requires a non-null pointer
    makes the diagnostics more reliable even without optimization. It's
    better to give a diagnostic earlier anyway, at the actual problematic
    call in the user's code.
    
    libstdc++-v3/ChangeLog:
    
            * include/bits/basic_string.h (starts_with, ends_with, contains):
            Add nonnull attribute.
            * include/bits/cow_string.h (starts_with, ends_with, contains):
            Likewise.
            * include/std/string_view (starts_with, ends_with, contains):
            Likewise.

diff --git a/libstdc++-v3/include/bits/basic_string.h b/libstdc++-v3/include/bits/basic_string.h
index b04fba95678..9d8b415302b 100644
--- a/libstdc++-v3/include/bits/basic_string.h
+++ b/libstdc++-v3/include/bits/basic_string.h
@@ -3400,6 +3400,7 @@ _GLIBCXX_BEGIN_NAMESPACE_CXX11
       starts_with(_CharT __x) const noexcept
       { return __sv_type(this->data(), this->size()).starts_with(__x); }
 
+      [[__gnu__::__nonnull__]]
       constexpr bool
       starts_with(const _CharT* __x) const noexcept
       { return __sv_type(this->data(), this->size()).starts_with(__x); }
@@ -3412,6 +3413,7 @@ _GLIBCXX_BEGIN_NAMESPACE_CXX11
       ends_with(_CharT __x) const noexcept
       { return __sv_type(this->data(), this->size()).ends_with(__x); }
 
+      [[__gnu__::__nonnull__]]
       constexpr bool
       ends_with(const _CharT* __x) const noexcept
       { return __sv_type(this->data(), this->size()).ends_with(__x); }
@@ -3426,6 +3428,7 @@ _GLIBCXX_BEGIN_NAMESPACE_CXX11
       contains(_CharT __x) const noexcept
       { return __sv_type(this->data(), this->size()).contains(__x); }
 
+      [[__gnu__::__nonnull__]]
       constexpr bool
       contains(const _CharT* __x) const noexcept
       { return __sv_type(this->data(), this->size()).contains(__x); }
diff --git a/libstdc++-v3/include/bits/cow_string.h b/libstdc++-v3/include/bits/cow_string.h
index f16e33ac1ef..f5f03338eec 100644
--- a/libstdc++-v3/include/bits/cow_string.h
+++ b/libstdc++-v3/include/bits/cow_string.h
@@ -3012,6 +3012,7 @@ _GLIBCXX_BEGIN_NAMESPACE_VERSION
       starts_with(_CharT __x) const noexcept
       { return __sv_type(this->data(), this->size()).starts_with(__x); }
 
+      [[__gnu__::__nonnull__]]
       bool
       starts_with(const _CharT* __x) const noexcept
       { return __sv_type(this->data(), this->size()).starts_with(__x); }
@@ -3024,6 +3025,7 @@ _GLIBCXX_BEGIN_NAMESPACE_VERSION
       ends_with(_CharT __x) const noexcept
       { return __sv_type(this->data(), this->size()).ends_with(__x); }
 
+      [[__gnu__::__nonnull__]]
       bool
       ends_with(const _CharT* __x) const noexcept
       { return __sv_type(this->data(), this->size()).ends_with(__x); }
@@ -3038,6 +3040,7 @@ _GLIBCXX_BEGIN_NAMESPACE_VERSION
       contains(_CharT __x) const noexcept
       { return __sv_type(this->data(), this->size()).contains(__x); }
 
+      [[__gnu__::__nonnull__]]
       bool
       contains(const _CharT* __x) const noexcept
       { return __sv_type(this->data(), this->size()).contains(__x); }
diff --git a/libstdc++-v3/include/std/string_view b/libstdc++-v3/include/std/string_view
index 30ff136b1cb..5b96ffeecd6 100644
--- a/libstdc++-v3/include/std/string_view
+++ b/libstdc++-v3/include/std/string_view
@@ -360,6 +360,7 @@ _GLIBCXX_BEGIN_NAMESPACE_VERSION
       starts_with(_CharT __x) const noexcept
       { return !this->empty() && traits_type::eq(this->front(), __x); }
 
+      [[__gnu__::__nonnull__]]
       constexpr bool
       starts_with(const _CharT* __x) const noexcept
       { return this->starts_with(basic_string_view(__x)); }
@@ -377,6 +378,7 @@ _GLIBCXX_BEGIN_NAMESPACE_VERSION
       ends_with(_CharT __x) const noexcept
       { return !this->empty() && traits_type::eq(this->back(), __x); }
 
+      [[__gnu__::__nonnull__]]
       constexpr bool
       ends_with(const _CharT* __x) const noexcept
       { return this->ends_with(basic_string_view(__x)); }
@@ -392,6 +394,7 @@ _GLIBCXX_BEGIN_NAMESPACE_VERSION
       contains(_CharT __x) const noexcept
       { return this->find(__x) != npos; }
 
+      [[__gnu__::__nonnull__]]
       constexpr bool
       contains(const _CharT* __x) const noexcept
       { return this->find(__x) != npos; }

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

* Re: [PATCH][_GLIBCXX_DEBUG] Review null string assertions (was: Add basic_string::starts_with/ends_with checks)
  2022-08-26  9:31 ` Jonathan Wakely
@ 2022-08-31  4:38   ` François Dumont
  2022-08-31  9:25     ` Jonathan Wakely
  0 siblings, 1 reply; 7+ messages in thread
From: François Dumont @ 2022-08-31  4:38 UTC (permalink / raw)
  To: Jonathan Wakely; +Cc: libstdc++, gcc-patches

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

If I got your point correctly here is this patch again with just the 
change on '0' becoming 'nullptr' in assertions.

     libstdc++: [_GLIBCXX_DEBUG] Review nullptr assertion diagnostics

     Review null string checks to show:
     _String != nullptr

     rather than:
     _String != 0

     libstdc++-v3/ChangeLog:

             * include/debug/debug.h: Use nullptr rather than '0' in 
checks in post-C++11.
             * include/debug/string: Likewise.
             * 
testsuite/21_strings/basic_string/operations/ends_with/char.cc: Use 
__gnu_test::string.
             * 
testsuite/21_strings/basic_string/operations/ends_with/nonnull.cc: Likewise.
             * 
testsuite/21_strings/basic_string/operations/ends_with/wchar_t.cc: Likewise.
             * 
testsuite/21_strings/basic_string/operations/starts_with/wchar_t.cc: 
Likewise.
             * 
testsuite/21_strings/basic_string/operations/starts_with/nonnull.cc: 
Likewise.
             * 
testsuite/21_strings/basic_string/operations/starts_with/char.cc: Likewise..

Ok to commit ?

François

On 26/08/22 11:31, Jonathan Wakely wrote:
> On Sun, 14 Aug 2022 at 16:34, François Dumont via Libstdc++
> <libstdc++@gcc.gnu.org> wrote:
>> I think we can add those checks.
>>
>> Note that I wonder if it was needed as in basic_string_view I see usages
>> of __attribute__((__nonnull__)). But running the test I saw no impact
>> even after I try to apply this attribute to the starts_with/ends_with
>> methods themselves.
> That should cause warnings, and does when I try it.
>
> As you say, the relevant string_view constructor already has that anyway:
>
>        __attribute__((__nonnull__)) constexpr
>        basic_string_view(const _CharT* __str) noexcept
>
> And so does string_view::find. The problem is that those only help if
> the compiler inlines the calls to those functions and so can propagate
> the null value all the way down to a function with the attribute.
> Adding the attribute to the relevant starts_with, ends_with and
> contains functions makes the diagnostics more likely to be emitted
> without optimization.
>
>> Also note that several checks like the ones I am adding here are XFAILS
>> when using 'make check' because of the segfault rather than on a proper
>> debug checks. Would you prefer to add dg-require-debug-mode to those ?
>>
>>       libstdc++: [_GLIBCXX_DEBUG] Add basic_string::starts_with/ends_with
>> checks
>>
>>       Add simple checks on C string parameters which should not be null.
>>
>>       Review null string checks to show:
>>       _String != nullptr
>>
>>       rather than:
>>       _String != 0
> I don't really like the extra complexity in the macros, but this does
> seem like a nice improvement for what users see.
>
> We could use __null for C++98, which is a compiler keyword that
> expands to a null pointer constant, but I'm not sure if that would be
> clear to all users or not. Maybe 0 is better.
>
> .


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

diff --git a/libstdc++-v3/include/debug/debug.h b/libstdc++-v3/include/debug/debug.h
index 28e250f0c50..f4233760426 100644
--- a/libstdc++-v3/include/debug/debug.h
+++ b/libstdc++-v3/include/debug/debug.h
@@ -118,10 +118,17 @@ namespace __gnu_debug
   __glibcxx_check_heap(_First,_Last)
 # define __glibcxx_requires_heap_pred(_First,_Last,_Pred)	\
   __glibcxx_check_heap_pred(_First,_Last,_Pred)
-# define __glibcxx_requires_string(_String)	\
+# if __cplusplus < 201103L
+#  define __glibcxx_requires_string(_String)	\
   _GLIBCXX_DEBUG_PEDASSERT(_String != 0)
-# define __glibcxx_requires_string_len(_String,_Len)	\
+#  define __glibcxx_requires_string_len(_String,_Len)	\
   _GLIBCXX_DEBUG_PEDASSERT(_String != 0 || _Len == 0)
+# else
+#  define __glibcxx_requires_string(_String)	\
+  _GLIBCXX_DEBUG_PEDASSERT(_String != nullptr)
+#  define __glibcxx_requires_string_len(_String,_Len)	\
+  _GLIBCXX_DEBUG_PEDASSERT(_String != nullptr || _Len == 0)
+# endif
 # define __glibcxx_requires_irreflexive(_First,_Last)	\
   __glibcxx_check_irreflexive(_First,_Last)
 # define __glibcxx_requires_irreflexive2(_First,_Last)	\
diff --git a/libstdc++-v3/include/debug/string b/libstdc++-v3/include/debug/string
index c32eb41eacd..574a78e3dac 100644
--- a/libstdc++-v3/include/debug/string
+++ b/libstdc++-v3/include/debug/string
@@ -50,14 +50,25 @@
 #endif
 
 #ifdef _GLIBCXX_DEBUG_PEDANTIC
-# define __glibcxx_check_string(_String)				\
+# if __cplusplus < 201103L
+#  define __glibcxx_check_string(_String)			\
   _GLIBCXX_DEBUG_VERIFY_STR_COND_AT(_String != 0,		\
 				    __FILE__, __LINE__,		\
 				    __PRETTY_FUNCTION__);
-# define __glibcxx_check_string_len(_String,_Len)		\
+#  define __glibcxx_check_string_len(_String,_Len)		\
   _GLIBCXX_DEBUG_VERIFY_STR_COND_AT(_String != 0 || _Len == 0,	\
 				    __FILE__, __LINE__,		\
 				    __PRETTY_FUNCTION__);
+# else
+#  define __glibcxx_check_string(_String)			\
+  _GLIBCXX_DEBUG_VERIFY_STR_COND_AT(_String != nullptr,		\
+				    __FILE__, __LINE__,		\
+				    __PRETTY_FUNCTION__);
+#  define __glibcxx_check_string_len(_String,_Len)		\
+  _GLIBCXX_DEBUG_VERIFY_STR_COND_AT(_String != nullptr || _Len == 0,	\
+				    __FILE__, __LINE__,		\
+				    __PRETTY_FUNCTION__);
+# endif
 #else
 # define __glibcxx_check_string(_String)
 # define __glibcxx_check_string_len(_String,_Len)
@@ -75,8 +86,13 @@ namespace __gnu_debug
 		   const char* __function __attribute__((__unused__)))
     {
 #ifdef _GLIBCXX_DEBUG_PEDANTIC
+# if __cplusplus < 201103L
       _GLIBCXX_DEBUG_VERIFY_STR_COND_AT(__s != 0 || __n == 0,
 					__file, __line, __function);
+# else
+      _GLIBCXX_DEBUG_VERIFY_STR_COND_AT(__s != nullptr || __n == 0,
+					__file, __line, __function);
+# endif
 #endif
       return __s;
     }
@@ -90,8 +106,13 @@ namespace __gnu_debug
 		   const char* __function __attribute__((__unused__)))
     {
 #ifdef _GLIBCXX_DEBUG_PEDANTIC
+# if __cplusplus < 201103L
       _GLIBCXX_DEBUG_VERIFY_STR_COND_AT(__s != 0,
 					__file, __line, __function);
+# else
+      _GLIBCXX_DEBUG_VERIFY_STR_COND_AT(__s != nullptr,
+					__file, __line, __function);
+# endif
 #endif
       return __s;
     }
diff --git a/libstdc++-v3/testsuite/21_strings/basic_string/operations/ends_with/char.cc b/libstdc++-v3/testsuite/21_strings/basic_string/operations/ends_with/char.cc
index cfe18e0186c..3cf85121e36 100644
--- a/libstdc++-v3/testsuite/21_strings/basic_string/operations/ends_with/char.cc
+++ b/libstdc++-v3/testsuite/21_strings/basic_string/operations/ends_with/char.cc
@@ -20,7 +20,7 @@
 
 // basic_string ends_with
 
-#include <string>
+#include <testsuite_string.h>
 #include <testsuite_hooks.h>
 
 void
@@ -31,7 +31,7 @@ test01()
   const char cstr_suf2[] = ".rgb";
   const std::string_view sv_suf2(".rgb");
 
-  const std::string s_test("slugs/slimy.jpg");
+  const __gnu_test::string s_test("slugs/slimy.jpg");
 
   const auto cstr_in_slugs = s_test.ends_with(cstr_suf);
   VERIFY( cstr_in_slugs );
diff --git a/libstdc++-v3/testsuite/21_strings/basic_string/operations/ends_with/nonnull.cc b/libstdc++-v3/testsuite/21_strings/basic_string/operations/ends_with/nonnull.cc
index 1f2a156bace..70ab867fa4e 100644
--- a/libstdc++-v3/testsuite/21_strings/basic_string/operations/ends_with/nonnull.cc
+++ b/libstdc++-v3/testsuite/21_strings/basic_string/operations/ends_with/nonnull.cc
@@ -1,10 +1,10 @@
 // { dg-options "-std=gnu++20 -Wnonnull -O0" }
 // { dg-do compile { target c++20 } }
 
-#include <string>
+#include <testsuite_string.h>
 
 void
-test01(const std::string& s)
+test01(const __gnu_test::string& s)
 {
   s.ends_with((const char*)nullptr);  // { dg-warning "\\\[-Wnonnull" }
   s.ends_with((char*)nullptr);	      // { dg-warning "\\\[-Wnonnull" }
diff --git a/libstdc++-v3/testsuite/21_strings/basic_string/operations/ends_with/wchar_t.cc b/libstdc++-v3/testsuite/21_strings/basic_string/operations/ends_with/wchar_t.cc
index d27e8246fb8..70b522ff69e 100644
--- a/libstdc++-v3/testsuite/21_strings/basic_string/operations/ends_with/wchar_t.cc
+++ b/libstdc++-v3/testsuite/21_strings/basic_string/operations/ends_with/wchar_t.cc
@@ -20,7 +20,7 @@
 
 // basic_string ends_with
 
-#include <string>
+#include <testsuite_string.h>
 #include <testsuite_hooks.h>
 
 void
@@ -31,7 +31,7 @@ test01()
   const wchar_t cstr_suf2[] = L".rgb";
   const std::wstring_view sv_suf2(L".rgb");
 
-  const std::wstring s_test(L"slugs/slimy.jpg");
+  const __gnu_test::wstring s_test(L"slugs/slimy.jpg");
 
   const auto cstr_in_slugs = s_test.ends_with(cstr_suf);
   VERIFY( cstr_in_slugs );
diff --git a/libstdc++-v3/testsuite/21_strings/basic_string/operations/starts_with/char.cc b/libstdc++-v3/testsuite/21_strings/basic_string/operations/starts_with/char.cc
index 5c0b3afc0b6..dddf51cee16 100644
--- a/libstdc++-v3/testsuite/21_strings/basic_string/operations/starts_with/char.cc
+++ b/libstdc++-v3/testsuite/21_strings/basic_string/operations/starts_with/char.cc
@@ -20,7 +20,7 @@
 
 // basic_string begins_with
 
-#include <string>
+#include <testsuite_string.h>
 #include <testsuite_hooks.h>
 
 void
@@ -31,7 +31,7 @@ test01()
   const char cstr_dir2[] = "worms/";
   const std::string_view sv_dir2("worms/");
 
-  const std::string s_test("slugs/slimy.jpg");
+  const __gnu_test::string s_test("slugs/slimy.jpg");
 
   const auto cstr_in_slugs = s_test.starts_with(cstr_dir);
   VERIFY( cstr_in_slugs );
diff --git a/libstdc++-v3/testsuite/21_strings/basic_string/operations/starts_with/nonnull.cc b/libstdc++-v3/testsuite/21_strings/basic_string/operations/starts_with/nonnull.cc
index 8514359c877..34614be0ab6 100644
--- a/libstdc++-v3/testsuite/21_strings/basic_string/operations/starts_with/nonnull.cc
+++ b/libstdc++-v3/testsuite/21_strings/basic_string/operations/starts_with/nonnull.cc
@@ -1,10 +1,10 @@
 // { dg-options "-std=gnu++20 -Wnonnull -O0" }
 // { dg-do compile { target c++20 } }
 
-#include <string>
+#include <testsuite_string.h>
 
 void
-test01(const std::string& s)
+test01(const __gnu_test::string& s)
 {
   s.starts_with((const char*)nullptr);  // { dg-warning "\\\[-Wnonnull" }
   s.starts_with((char*)nullptr);	// { dg-warning "\\\[-Wnonnull" }
diff --git a/libstdc++-v3/testsuite/21_strings/basic_string/operations/starts_with/wchar_t.cc b/libstdc++-v3/testsuite/21_strings/basic_string/operations/starts_with/wchar_t.cc
index eda73212ea0..747b23ae5c2 100644
--- a/libstdc++-v3/testsuite/21_strings/basic_string/operations/starts_with/wchar_t.cc
+++ b/libstdc++-v3/testsuite/21_strings/basic_string/operations/starts_with/wchar_t.cc
@@ -20,7 +20,7 @@
 
 // basic_string begins_with
 
-#include <string>
+#include <testsuite_string.h>
 #include <testsuite_hooks.h>
 
 void
@@ -31,7 +31,7 @@ test01()
   const wchar_t cstr_dir2[] = L"worms/";
   const std::wstring_view sv_dir2(L"worms/");
 
-  const std::wstring s_test(L"slugs/slimy.jpg");
+  const __gnu_test::wstring s_test(L"slugs/slimy.jpg");
 
   const auto cstr_in_slugs = s_test.starts_with(cstr_dir);
   VERIFY( cstr_in_slugs );

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

* Re: [PATCH][_GLIBCXX_DEBUG] Review null string assertions (was: Add basic_string::starts_with/ends_with checks)
  2022-08-31  4:38   ` [PATCH][_GLIBCXX_DEBUG] Review null string assertions (was: Add basic_string::starts_with/ends_with checks) François Dumont
@ 2022-08-31  9:25     ` Jonathan Wakely
  0 siblings, 0 replies; 7+ messages in thread
From: Jonathan Wakely @ 2022-08-31  9:25 UTC (permalink / raw)
  To: François Dumont; +Cc: libstdc++, gcc-patches

On Wed, 31 Aug 2022 at 05:38, François Dumont <frs.dumont@gmail.com> wrote:
>
> If I got your point correctly here is this patch again with just the
> change on '0' becoming 'nullptr' in assertions.
>
>      libstdc++: [_GLIBCXX_DEBUG] Review nullptr assertion diagnostics
>
>      Review null string checks to show:
>      _String != nullptr
>
>      rather than:
>      _String != 0
>
>      libstdc++-v3/ChangeLog:
>
>              * include/debug/debug.h: Use nullptr rather than '0' in
> checks in post-C++11.
>              * include/debug/string: Likewise.
>              *
> testsuite/21_strings/basic_string/operations/ends_with/char.cc: Use
> __gnu_test::string.
>              *
> testsuite/21_strings/basic_string/operations/ends_with/nonnull.cc: Likewise.
>              *
> testsuite/21_strings/basic_string/operations/ends_with/wchar_t.cc: Likewise.
>              *
> testsuite/21_strings/basic_string/operations/starts_with/wchar_t.cc:
> Likewise.
>              *
> testsuite/21_strings/basic_string/operations/starts_with/nonnull.cc:
> Likewise.
>              *
> testsuite/21_strings/basic_string/operations/starts_with/char.cc: Likewise..
>
> Ok to commit ?

The testsuite changes seem unrelated to the actual code change, so I'd
have done a separate patch. But OK for trunk.


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

end of thread, other threads:[~2022-08-31  9:25 UTC | newest]

Thread overview: 7+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2022-08-14 15:32 [PATCH][_GLIBCXX_DEBUG] Add basic_string::starts_with/ends_with checks François Dumont
2022-08-15 20:26 ` François Dumont
2022-08-25 16:11   ` François Dumont
2022-08-26  9:33     ` Jonathan Wakely
2022-08-26  9:31 ` Jonathan Wakely
2022-08-31  4:38   ` [PATCH][_GLIBCXX_DEBUG] Review null string assertions (was: Add basic_string::starts_with/ends_with checks) François Dumont
2022-08-31  9:25     ` 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).