This is the mail archive of the mailing list for the GCC project.

Index Nav: [Date Index] [Subject Index] [Author Index] [Thread Index]
Message Nav: [Date Prev] [Date Next] [Thread Prev] [Thread Next]
Other format: [Raw text]

Re: [v3 PATCH] Implement std::string_view and P0254r2, Integrating std::string_view and std::string.

On 25/07/16 00:41 +0300, Ville Voutilainen wrote:
Changelog as it was before, tested on Linux-x64.

I haven't fixed all section references of string tests, and I haven't
added section references
to string_view tests, because they didn't have any. I also haven't
added tests that test the various
exception-throwing cases for the various operations. Doing those changes is
a) going to change many many files
b) probably not urgent
so here's a modest suggestion:
let's put this patch in, and tweak the section references in the other
tests later with separate patches,
as well as add more comprehensive exception-tests.

Ok for trunk?

+#if __cplusplus > 201402L
+      /**
+       *  @brief  Append a string_view.
+       *  @param __sv  The string_view to be appended.
+       *  @return  Reference to this string.
+       */
+      basic_string& append(__sv_type __sv)
+      {
+       return this->append(, __sv.size());
+      }

One-liners like this don't need braces on a separate line:

     { return this->append(, __sv.size()); }

But they do need a blank line after them before the next function:

+      /**
+       *  @brief  Append a range of characters from a string_view.
+       *  @param __sv  The string_view to be appended from.
+       *  @param __pos The position in the string_view to append
+       *  @param __n   The number of characters to append from the
+       *  @return  Reference to this string.
+       */
+      basic_string& append(__sv_type __sv,
+                          size_type __pos, size_type __n = npos)
+      {
+       return _M_append(
+                        + __sv._M_check(__pos,
+                        __sv._M_limit(__pos, __n));
+      }
+#endif // C++17

The Doxygen @file comment in <bits/string_view.tcc> refers to
"experimental" still.

We need to close the versioned namespace before this:

+  namespace __detail
+  {

And reopen it afterwards. i.e.

 namespace __detail



Otherwise for the versioned namespace config we would have
std::__7::__detail and also std::__detail::__7 and thigns would get

I don't think we want the non-standard literals in namespace std:

+  // I added these EMSR.
+  inline namespace literals
+  {
+  inline namespace string_view_literals
+  {

I've recently discovered that the order of dejagnu directives matters,
and this is wrong:

+// { dg-do run { xfail *-*-* } }
+// { dg-options "-std=gnu++17 -O0" }
+// { dg-require-debug-mode "" }

It should be dg-options first, then dg-do, then any dg-require-xxx
last. It doesn't actually make any difference in any of the tests in
your patch though, so don't worry about changing it.

The case where it matters is where the dg-do directive has a target
which might be dependent on the dg-options:

// { dg-do compile { target c++1z } }
// { dg-options "-std=gnu++1z" }

This wouldn't get run, because the effective target is tested before
-std=gnu++1z is added to the options.

And this would always run, ignoring the dg-require:

// { dg-require-effective-target c++1z }
// { dg-do compile }

But as I said, don't worry about changing them.

OK for trunk with the changes to formatting, namespaces and literals.

Thanks for doing this!

Index Nav: [Date Index] [Subject Index] [Author Index] [Thread Index]
Message Nav: [Date Prev] [Date Next] [Thread Prev] [Thread Next]