| Message ID | 20180613163030.GA23501@redhat.com |
|---|---|
| State | New |
| Headers | show |
| Series | Decorate string_view members with nonnull attribute | expand |
On 06/13/2018 10:30 AM, Jonathan Wakely wrote: > The C++ committee has confirmed that passing a null pointer to the > unary basic_string_view constructor is undefined. This removes the check > from our implementation, and adds the nonnull attribute to warn when the > compiler can detect undefined input. > > Any objections to this change? I have a general question about using the new C++ attributes in libstdc++ (standard or otherwise): what does C++ have to say about programs that define macros with the same names? E.g., is this a valid program? #define nonnull "..." ... #include <string_view> How about: #define noreturn "..." ... #include <string_view> The view in WG14 is that the corresponding C programs (if C2X were to adopt the proposed C++ attributes) would be valid and that it's up to implementations to make it work. Martin
On 14/06/18 10:46 -0600, Martin Sebor wrote: >On 06/13/2018 10:30 AM, Jonathan Wakely wrote: >>The C++ committee has confirmed that passing a null pointer to the >>unary basic_string_view constructor is undefined. This removes the check >>from our implementation, and adds the nonnull attribute to warn when the >>compiler can detect undefined input. >> >>Any objections to this change? > >I have a general question about using the new C++ attributes in >libstdc++ (standard or otherwise): what does C++ have to say about >programs that define macros with the same names? E.g., is this >a valid program? > > #define nonnull "..." > ... > #include <string_view> > >How about: > > #define noreturn "..." > ... > #include <string_view> > >The view in WG14 is that the corresponding C programs (if C2X >were to adopt the proposed C++ attributes) would be valid and >that it's up to implementations to make it work. Good point, I suppose I have to use __attribute__((__nonnull__)) instead. The reason I didn't is that I find the grammar for the position of C++11 attributes much easier to understand. But in this instance the same location works for either.
On 14 June 2018 at 19:51, Jonathan Wakely <jwakely@redhat.com> wrote: > On 14/06/18 10:46 -0600, Martin Sebor wrote: >> >> On 06/13/2018 10:30 AM, Jonathan Wakely wrote: >>> >>> The C++ committee has confirmed that passing a null pointer to the >>> unary basic_string_view constructor is undefined. This removes the check >>> from our implementation, and adds the nonnull attribute to warn when the >>> compiler can detect undefined input. >>> >>> Any objections to this change? >> >> >> I have a general question about using the new C++ attributes in >> libstdc++ (standard or otherwise): what does C++ have to say about >> programs that define macros with the same names? E.g., is this >> a valid program? >> >> #define nonnull "..." >> ... >> #include <string_view> >> >> How about: >> >> #define noreturn "..." >> ... >> #include <string_view> >> >> The view in WG14 is that the corresponding C programs (if C2X >> were to adopt the proposed C++ attributes) would be valid and >> that it's up to implementations to make it work. > > > Good point, I suppose I have to use __attribute__((__nonnull__)) > instead. The reason I didn't is that I find the grammar for the > position of C++11 attributes much easier to understand. But in this > instance the same location works for either. [macro.names]/2 forbids #defining macros with the same names as the standard attributes. The programs Martin shows as examples are not valid.
On 14/06/18 19:54 +0300, Ville Voutilainen wrote: >On 14 June 2018 at 19:51, Jonathan Wakely <jwakely@redhat.com> wrote: >> On 14/06/18 10:46 -0600, Martin Sebor wrote: >>> >>> On 06/13/2018 10:30 AM, Jonathan Wakely wrote: >>>> >>>> The C++ committee has confirmed that passing a null pointer to the >>>> unary basic_string_view constructor is undefined. This removes the check >>>> from our implementation, and adds the nonnull attribute to warn when the >>>> compiler can detect undefined input. >>>> >>>> Any objections to this change? >>> >>> >>> I have a general question about using the new C++ attributes in >>> libstdc++ (standard or otherwise): what does C++ have to say about >>> programs that define macros with the same names? E.g., is this >>> a valid program? >>> >>> #define nonnull "..." >>> ... >>> #include <string_view> >>> >>> How about: >>> >>> #define noreturn "..." >>> ... >>> #include <string_view> >>> >>> The view in WG14 is that the corresponding C programs (if C2X >>> were to adopt the proposed C++ attributes) would be valid and >>> that it's up to implementations to make it work. >> >> >> Good point, I suppose I have to use __attribute__((__nonnull__)) >> instead. The reason I didn't is that I find the grammar for the >> position of C++11 attributes much easier to understand. But in this >> instance the same location works for either. > >[macro.names]/2 forbids #defining macros with the same names as the >standard attributes. >The programs Martin shows as examples are not valid. But nonnull isn't a standard attribute though. So we can't use [[gnu::xxx]] attributes in libstdc++ and have to use __attribute__ instead. [[sad_face]]
On 14 June 2018 at 19:57, Jonathan Wakely <jwakely@redhat.com> wrote: >> [macro.names]/2 forbids #defining macros with the same names as the >> standard attributes. >> The programs Martin shows as examples are not valid. > > > But nonnull isn't a standard attribute though. So we can't use > [[gnu::xxx]] attributes in libstdc++ and have to use __attribute__ > instead. Oops, indeed. But for gnu-attributes, surely we can decide whatever we want about what's valid and what's not?
On 14/06/18 20:02 +0300, Ville Voutilainen wrote: >On 14 June 2018 at 19:57, Jonathan Wakely <jwakely@redhat.com> wrote: >>> [macro.names]/2 forbids #defining macros with the same names as the >>> standard attributes. >>> The programs Martin shows as examples are not valid. >> >> >> But nonnull isn't a standard attribute though. So we can't use >> [[gnu::xxx]] attributes in libstdc++ and have to use __attribute__ >> instead. > >Oops, indeed. But for gnu-attributes, surely we can decide whatever we >want about what's >valid and what's not? We could say that #defining 'nonnull' and/or 'gnu' as a macro is undefined, but then programs that the standard says are valid would fail to compile, and we'd be a non-conforming implementation. We can use __attribute__(__nonnull__)) so that we accept those programs (and be a conforming implementation). Why would we choose to be non-conforming when we don't have to? We can use [[gnu::__nonnull__]] but that would still reject programs that #define gnu so maybe we want the front-end to also recognise __gnu__ as our attribute namespace.
On 14 June 2018 at 20:08, Jonathan Wakely <jwakely@redhat.com> wrote: >> Oops, indeed. But for gnu-attributes, surely we can decide whatever we >> want about what's >> valid and what's not? > > > We could say that #defining 'nonnull' and/or 'gnu' as a macro is > undefined, but then programs that the standard says are valid would > fail to compile, and we'd be a non-conforming implementation. > > We can use __attribute__(__nonnull__)) so that we accept those > programs (and be a conforming implementation). Why would we choose to > be non-conforming when we don't have to? I can read the specification liberally enough that it might give us the freedom to disallow programs that #define names that our implementation uses for vendor attributes.
On 14 June 2018 at 20:21, Ville Voutilainen <ville.voutilainen@gmail.com> wrote: > On 14 June 2018 at 20:08, Jonathan Wakely <jwakely@redhat.com> wrote: >>> Oops, indeed. But for gnu-attributes, surely we can decide whatever we >>> want about what's >>> valid and what's not? >> >> >> We could say that #defining 'nonnull' and/or 'gnu' as a macro is >> undefined, but then programs that the standard says are valid would >> fail to compile, and we'd be a non-conforming implementation. >> >> We can use __attribute__(__nonnull__)) so that we accept those >> programs (and be a conforming implementation). Why would we choose to >> be non-conforming when we don't have to? > > I can read the specification liberally enough that it might give us > the freedom to disallow programs > that #define names that our implementation uses for vendor attributes. Namely "For an attribute-token (including an attribute-scoped-token) not specified in this document, the behavior is implementation-defined.", aka [dcl.attr.grammar]/6.
On 14/06/18 20:27 +0300, Ville Voutilainen wrote: >On 14 June 2018 at 20:21, Ville Voutilainen <ville.voutilainen@gmail.com> wrote: >> On 14 June 2018 at 20:08, Jonathan Wakely <jwakely@redhat.com> wrote: >>>> Oops, indeed. But for gnu-attributes, surely we can decide whatever we >>>> want about what's >>>> valid and what's not? >>> >>> >>> We could say that #defining 'nonnull' and/or 'gnu' as a macro is >>> undefined, but then programs that the standard says are valid would >>> fail to compile, and we'd be a non-conforming implementation. >>> >>> We can use __attribute__(__nonnull__)) so that we accept those >>> programs (and be a conforming implementation). Why would we choose to >>> be non-conforming when we don't have to? >> >> I can read the specification liberally enough that it might give us >> the freedom to disallow programs >> that #define names that our implementation uses for vendor attributes. > >Namely "For an attribute-token (including an attribute-scoped-token) >not specified in this document, the behavior is >implementation-defined.", aka [dcl.attr.grammar]/6. As I said on IRC, if the user does #define gnu R"( ,= ,-_-. =. ((_/)o o(\_)) `-'(. .)`-' \_/ )" then the attribute-scoped-token isn't even valid, and so the behaviour of the implementation-defined gnu::nonnull attribute doesn't matter. The code doesn't contain such an attribute. This seems like an unnecessary discussion: we should just use the __attribute__ form instead.
On 14 June 2018 at 20:40, Jonathan Wakely <jwakely@redhat.com> wrote: >> Namely "For an attribute-token (including an attribute-scoped-token) >> not specified in this document, the behavior is >> implementation-defined.", aka [dcl.attr.grammar]/6. > > > As I said on IRC, if the user does > > #define gnu R"( > ,= ,-_-. =. > ((_/)o o(\_)) > `-'(. .)`-' > \_/ > )" > > then the attribute-scoped-token isn't even valid, and so the behaviour > of the implementation-defined gnu::nonnull attribute doesn't matter. > The code doesn't contain such an attribute. > > This seems like an unnecessary discussion: we should just use the > __attribute__ form instead. That may well be, but deciding to handle preprocessor definitions of vendor attributes in a particular way doesn't make us non-conforming. The behaviour of a gnu::nonnull would, as far as I understand it, possibly include restrictions on what the preprocessor can do with it, just like the specification for standard attributes does. The preprocessor is part of the implementation, not a magical separate step that can do whatever it pleases. Conformance being a moot point, I'd prefer using the standard attribute syntax if we can, instead of using the gnuattribute syntax, but that's not a strong preference.
On 06/14/2018 10:54 AM, Ville Voutilainen wrote: > On 14 June 2018 at 19:51, Jonathan Wakely <jwakely@redhat.com> wrote: >> On 14/06/18 10:46 -0600, Martin Sebor wrote: >>> >>> On 06/13/2018 10:30 AM, Jonathan Wakely wrote: >>>> >>>> The C++ committee has confirmed that passing a null pointer to the >>>> unary basic_string_view constructor is undefined. This removes the check >>>> from our implementation, and adds the nonnull attribute to warn when the >>>> compiler can detect undefined input. >>>> >>>> Any objections to this change? >>> >>> >>> I have a general question about using the new C++ attributes in >>> libstdc++ (standard or otherwise): what does C++ have to say about >>> programs that define macros with the same names? E.g., is this >>> a valid program? >>> >>> #define nonnull "..." >>> ... >>> #include <string_view> >>> >>> How about: >>> >>> #define noreturn "..." >>> ... >>> #include <string_view> >>> >>> The view in WG14 is that the corresponding C programs (if C2X >>> were to adopt the proposed C++ attributes) would be valid and >>> that it's up to implementations to make it work. >> >> >> Good point, I suppose I have to use __attribute__((__nonnull__)) >> instead. The reason I didn't is that I find the grammar for the >> position of C++11 attributes much easier to understand. But in this >> instance the same location works for either. > > [macro.names]/2 forbids #defining macros with the same names as the > standard attributes. > The programs Martin shows as examples are not valid. Okay, that's good to know about the one that uses the standard attribute (and thanks for the reference). WG14 should consider this when integrating attributes into C2X. FWIW, the problem I suspect will come up in WG14 is that C tries hard to avoid introducing new reserved words into the public namespace. At the same time, two of the biggest selling points of the C++ attribute syntax proposal in WG14 (i.e., the double brackets, as opposed to something more C-ish like _Attribute()) are a) compatibility and interoperability with C++ and b) being able to avoid relying on preprocessor conditionals and macros to use attributes in portable code. If C2X programs (and libraries) are going to continue to be allowed to #define macros with the same names as even standard attributes, we (i.e., both WG14 and to an extent even WG21) will have failed to deliver on these goals. To avoid name clashes, mixed C/C++ programs will still have to use #ifdefs and macros in both languages to make use of attributes because the spelling of the "private" attribute names (like __noreturn__) is not specified. Even if C2X were to impose the same restriction on macros as C++, there likely are C headers out there today that do just that, and not all those headers will necessarily migrate to C2X. Martin
On 14/06/18 17:51 +0100, Jonathan Wakely wrote: >On 14/06/18 10:46 -0600, Martin Sebor wrote: >>On 06/13/2018 10:30 AM, Jonathan Wakely wrote: >>>The C++ committee has confirmed that passing a null pointer to the >>>unary basic_string_view constructor is undefined. This removes the check >>>from our implementation, and adds the nonnull attribute to warn when the >>>compiler can detect undefined input. >>> >>>Any objections to this change? >> >>I have a general question about using the new C++ attributes in >>libstdc++ (standard or otherwise): what does C++ have to say about >>programs that define macros with the same names? E.g., is this >>a valid program? >> >> #define nonnull "..." >> ... >> #include <string_view> >> >>How about: >> >> #define noreturn "..." >> ... >> #include <string_view> >> >>The view in WG14 is that the corresponding C programs (if C2X >>were to adopt the proposed C++ attributes) would be valid and >>that it's up to implementations to make it work. > >Good point, I suppose I have to use __attribute__((__nonnull__)) >instead. The reason I didn't is that I find the grammar for the >position of C++11 attributes much easier to understand. But in this >instance the same location works for either. Nobody's objected to the patch in princple, so I'll commit this version, using __attribute__((__nonnull__)) instead of [[gnu::nonnull]. commit 5a1626332e84b64adc049b7ce13c2ed8917a7fa3 Author: Jonathan Wakely <jwakely@redhat.com> Date: Wed Jun 13 16:54:47 2018 +0100 Decorate string_view members with nonnull attribute The C++ committee has confirmed that passing a null pointer to the unary basic_string_view constructor is undefined. This removes the check from our implementation, and adds the nonnull attribute to warn when the compiler can detect undefined input. * include/std/string_view (basic_string_view(const CharT*)): Remove check for null pointer and add nonnull attribute. (compare(const CharT*), compare(size_type, size_type, const CharT*)) (find(const CharT*, size_type), rfind(const CharT*, size_type)) (find_first_of(const CharT*, size_type)) (find_last_of(const CharT*, size_type)) (find_first_not_of(const CharT*, size_type)) (find_last_not_of(const CharT*, size_type)): Add nonnull attribute. * testsuite/21_strings/basic_string_view/cons/char/nonnull.cc: New. * testsuite/21_strings/basic_string_view/operations/compare/char/ nonnull.cc: New. * testsuite/21_strings/basic_string_view/operations/find/char/ nonnull.cc: New. * testsuite/21_strings/basic_string_view/operations/rfind/char/ nonnull.cc: New. diff --git a/libstdc++-v3/include/std/string_view b/libstdc++-v3/include/std/string_view index 9f39df853e8..f84664ca286 100644 --- a/libstdc++-v3/include/std/string_view +++ b/libstdc++-v3/include/std/string_view @@ -96,8 +96,9 @@ _GLIBCXX_BEGIN_NAMESPACE_VERSION constexpr basic_string_view(const basic_string_view&) noexcept = default; - constexpr basic_string_view(const _CharT* __str) noexcept - : _M_len{__str == nullptr ? 0 : traits_type::length(__str)}, + __attribute__((__nonnull__)) constexpr + basic_string_view(const _CharT* __str) noexcept + : _M_len{traits_type::length(__str)}, _M_str{__str} { } @@ -270,11 +271,11 @@ _GLIBCXX_BEGIN_NAMESPACE_VERSION return this->substr(__pos1, __n1).compare(__str.substr(__pos2, __n2)); } - constexpr int + __attribute__((__nonnull__)) constexpr int compare(const _CharT* __str) const noexcept { return this->compare(basic_string_view{__str}); } - constexpr int + __attribute__((__nonnull__)) constexpr int compare(size_type __pos1, size_type __n1, const _CharT* __str) const { return this->substr(__pos1, __n1).compare(basic_string_view{__str}); } @@ -296,7 +297,7 @@ _GLIBCXX_BEGIN_NAMESPACE_VERSION constexpr size_type find(const _CharT* __str, size_type __pos, size_type __n) const noexcept; - constexpr size_type + __attribute__((__nonnull__)) constexpr size_type find(const _CharT* __str, size_type __pos = 0) const noexcept { return this->find(__str, __pos, traits_type::length(__str)); } @@ -310,7 +311,7 @@ _GLIBCXX_BEGIN_NAMESPACE_VERSION constexpr size_type rfind(const _CharT* __str, size_type __pos, size_type __n) const noexcept; - constexpr size_type + __attribute__((__nonnull__)) constexpr size_type rfind(const _CharT* __str, size_type __pos = npos) const noexcept { return this->rfind(__str, __pos, traits_type::length(__str)); } @@ -323,9 +324,10 @@ _GLIBCXX_BEGIN_NAMESPACE_VERSION { return this->find(__c, __pos); } constexpr size_type - find_first_of(const _CharT* __str, size_type __pos, size_type __n) const noexcept; + find_first_of(const _CharT* __str, size_type __pos, + size_type __n) const noexcept; - constexpr size_type + __attribute__((__nonnull__)) constexpr size_type find_first_of(const _CharT* __str, size_type __pos = 0) const noexcept { return this->find_first_of(__str, __pos, traits_type::length(__str)); } @@ -342,7 +344,7 @@ _GLIBCXX_BEGIN_NAMESPACE_VERSION find_last_of(const _CharT* __str, size_type __pos, size_type __n) const noexcept; - constexpr size_type + __attribute__((__nonnull__)) constexpr size_type find_last_of(const _CharT* __str, size_type __pos = npos) const noexcept { return this->find_last_of(__str, __pos, traits_type::length(__str)); } @@ -358,7 +360,7 @@ _GLIBCXX_BEGIN_NAMESPACE_VERSION find_first_not_of(const _CharT* __str, size_type __pos, size_type __n) const noexcept; - constexpr size_type + __attribute__((__nonnull__)) constexpr size_type find_first_not_of(const _CharT* __str, size_type __pos = 0) const noexcept { return this->find_first_not_of(__str, __pos, @@ -377,7 +379,7 @@ _GLIBCXX_BEGIN_NAMESPACE_VERSION find_last_not_of(const _CharT* __str, size_type __pos, size_type __n) const noexcept; - constexpr size_type + __attribute__((__nonnull__)) constexpr size_type find_last_not_of(const _CharT* __str, size_type __pos = npos) const noexcept { diff --git a/libstdc++-v3/testsuite/21_strings/basic_string_view/cons/char/nonnull.cc b/libstdc++-v3/testsuite/21_strings/basic_string_view/cons/char/nonnull.cc new file mode 100644 index 00000000000..463ece718bd --- /dev/null +++ b/libstdc++-v3/testsuite/21_strings/basic_string_view/cons/char/nonnull.cc @@ -0,0 +1,29 @@ +// 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 +// <http://www.gnu.org/licenses/>. + +// { dg-options "-std=gnu++17 -Wnonnull" } +// { dg-do compile { target c++17 } } + +#include <string_view> + +void +test01() +{ + std::string_view s((const char*)nullptr); // { dg-warning "null arg" } + std::string_view t((char*)nullptr); // { dg-warning "null arg" } + std::string_view u(nullptr); // { dg-warning "null arg" } +} diff --git a/libstdc++-v3/testsuite/21_strings/basic_string_view/cons/wchar_t/nonnull.cc b/libstdc++-v3/testsuite/21_strings/basic_string_view/cons/wchar_t/nonnull.cc new file mode 100644 index 00000000000..8df61427a49 --- /dev/null +++ b/libstdc++-v3/testsuite/21_strings/basic_string_view/cons/wchar_t/nonnull.cc @@ -0,0 +1,29 @@ +// 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 +// <http://www.gnu.org/licenses/>. + +// { dg-options "-std=gnu++17 -Wnonnull" } +// { dg-do compile { target c++17 } } + +#include <string_view> + +void +test01() +{ + std::wstring_view s((const wchar_t*)nullptr); // { dg-warning "null arg" } + std::wstring_view t((wchar_t*)nullptr); // { dg-warning "null arg" } + std::wstring_view u(nullptr); // { dg-warning "null arg" } +} diff --git a/libstdc++-v3/testsuite/21_strings/basic_string_view/operations/compare/char/nonnull.cc b/libstdc++-v3/testsuite/21_strings/basic_string_view/operations/compare/char/nonnull.cc new file mode 100644 index 00000000000..1d12b700d88 --- /dev/null +++ b/libstdc++-v3/testsuite/21_strings/basic_string_view/operations/compare/char/nonnull.cc @@ -0,0 +1,29 @@ +// 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 +// <http://www.gnu.org/licenses/>. + +// { dg-options "-std=gnu++17 -Wnonnull" } +// { dg-do compile { target c++17 } } + +#include <string_view> + +int +test01() +{ + std::string_view s = "abcd"; + return s.compare((const char*)nullptr); // { dg-warning "null arg" } + return s.compare(0, 2, (const char*)nullptr); // { dg-warning "null arg" } +} diff --git a/libstdc++-v3/testsuite/21_strings/basic_string_view/operations/find/char/nonnull.cc b/libstdc++-v3/testsuite/21_strings/basic_string_view/operations/find/char/nonnull.cc new file mode 100644 index 00000000000..69f1850d9dc --- /dev/null +++ b/libstdc++-v3/testsuite/21_strings/basic_string_view/operations/find/char/nonnull.cc @@ -0,0 +1,33 @@ +// 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 +// <http://www.gnu.org/licenses/>. + +// { dg-options "-std=gnu++17 -Wnonnull" } +// { dg-do compile { target c++17 } } + +#include <string_view> + +int +test01() +{ + std::string_view s = "abcd"; + return s.find((const char*)nullptr); // { dg-warning "null arg" } + return s.find((const char*)nullptr, 1); // { dg-warning "null arg" } + return s.find_first_of((const char*)nullptr); // { dg-warning "null arg" } + return s.find_first_of((const char*)nullptr, 1); // { dg-warning "null arg" } + return s.find_first_not_of((const char*)nullptr); // { dg-warning "null arg" } + return s.find_first_not_of((const char*)nullptr, 1); // { dg-warning "null arg" } +} diff --git a/libstdc++-v3/testsuite/21_strings/basic_string_view/operations/rfind/char/nonnull.cc b/libstdc++-v3/testsuite/21_strings/basic_string_view/operations/rfind/char/nonnull.cc new file mode 100644 index 00000000000..ca8527afa58 --- /dev/null +++ b/libstdc++-v3/testsuite/21_strings/basic_string_view/operations/rfind/char/nonnull.cc @@ -0,0 +1,29 @@ +// 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 +// <http://www.gnu.org/licenses/>. + +// { dg-options "-std=gnu++17 -Wnonnull" } +// { dg-do compile { target c++17 } } + +#include <string_view> + +int +test01() +{ + std::string_view s = "abcd"; + return s.rfind((const char*)nullptr); // { dg-warning "null arg" } + return s.rfind((const char*)nullptr, 1); // { dg-warning "null arg" } +}
diff --git a/libstdc++-v3/include/std/string_view b/libstdc++-v3/include/std/string_view index 9f39df853e8..4de484a96ba 100644 --- a/libstdc++-v3/include/std/string_view +++ b/libstdc++-v3/include/std/string_view @@ -96,8 +96,9 @@ _GLIBCXX_BEGIN_NAMESPACE_VERSION constexpr basic_string_view(const basic_string_view&) noexcept = default; - constexpr basic_string_view(const _CharT* __str) noexcept - : _M_len{__str == nullptr ? 0 : traits_type::length(__str)}, + [[gnu::nonnull]] constexpr + basic_string_view(const _CharT* __str) noexcept + : _M_len{traits_type::length(__str)}, _M_str{__str} { } @@ -270,11 +271,11 @@ _GLIBCXX_BEGIN_NAMESPACE_VERSION return this->substr(__pos1, __n1).compare(__str.substr(__pos2, __n2)); } - constexpr int + [[gnu::nonnull]] constexpr int compare(const _CharT* __str) const noexcept { return this->compare(basic_string_view{__str}); } - constexpr int + [[gnu::nonnull]] constexpr int compare(size_type __pos1, size_type __n1, const _CharT* __str) const { return this->substr(__pos1, __n1).compare(basic_string_view{__str}); } @@ -296,7 +297,7 @@ _GLIBCXX_BEGIN_NAMESPACE_VERSION constexpr size_type find(const _CharT* __str, size_type __pos, size_type __n) const noexcept; - constexpr size_type + [[gnu::nonnull]] constexpr size_type find(const _CharT* __str, size_type __pos = 0) const noexcept { return this->find(__str, __pos, traits_type::length(__str)); } @@ -310,7 +311,7 @@ _GLIBCXX_BEGIN_NAMESPACE_VERSION constexpr size_type rfind(const _CharT* __str, size_type __pos, size_type __n) const noexcept; - constexpr size_type + [[gnu::nonnull]] constexpr size_type rfind(const _CharT* __str, size_type __pos = npos) const noexcept { return this->rfind(__str, __pos, traits_type::length(__str)); } @@ -323,9 +324,10 @@ _GLIBCXX_BEGIN_NAMESPACE_VERSION { return this->find(__c, __pos); } constexpr size_type - find_first_of(const _CharT* __str, size_type __pos, size_type __n) const noexcept; + find_first_of(const _CharT* __str, size_type __pos, + size_type __n) const noexcept; - constexpr size_type + [[gnu::nonnull]] constexpr size_type find_first_of(const _CharT* __str, size_type __pos = 0) const noexcept { return this->find_first_of(__str, __pos, traits_type::length(__str)); } @@ -342,7 +344,7 @@ _GLIBCXX_BEGIN_NAMESPACE_VERSION find_last_of(const _CharT* __str, size_type __pos, size_type __n) const noexcept; - constexpr size_type + [[gnu::nonnull]] constexpr size_type find_last_of(const _CharT* __str, size_type __pos = npos) const noexcept { return this->find_last_of(__str, __pos, traits_type::length(__str)); } @@ -358,7 +360,7 @@ _GLIBCXX_BEGIN_NAMESPACE_VERSION find_first_not_of(const _CharT* __str, size_type __pos, size_type __n) const noexcept; - constexpr size_type + [[gnu::nonnull]] constexpr size_type find_first_not_of(const _CharT* __str, size_type __pos = 0) const noexcept { return this->find_first_not_of(__str, __pos, @@ -377,7 +379,7 @@ _GLIBCXX_BEGIN_NAMESPACE_VERSION find_last_not_of(const _CharT* __str, size_type __pos, size_type __n) const noexcept; - constexpr size_type + [[gnu::nonnull]] constexpr size_type find_last_not_of(const _CharT* __str, size_type __pos = npos) const noexcept { diff --git a/libstdc++-v3/testsuite/21_strings/basic_string_view/cons/char/nonnull.cc b/libstdc++-v3/testsuite/21_strings/basic_string_view/cons/char/nonnull.cc new file mode 100644 index 00000000000..463ece718bd --- /dev/null +++ b/libstdc++-v3/testsuite/21_strings/basic_string_view/cons/char/nonnull.cc @@ -0,0 +1,29 @@ +// 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 +// <http://www.gnu.org/licenses/>. + +// { dg-options "-std=gnu++17 -Wnonnull" } +// { dg-do compile { target c++17 } } + +#include <string_view> + +void +test01() +{ + std::string_view s((const char*)nullptr); // { dg-warning "null arg" } + std::string_view t((char*)nullptr); // { dg-warning "null arg" } + std::string_view u(nullptr); // { dg-warning "null arg" } +} diff --git a/libstdc++-v3/testsuite/21_strings/basic_string_view/cons/wchar_t/nonnull.cc b/libstdc++-v3/testsuite/21_strings/basic_string_view/cons/wchar_t/nonnull.cc new file mode 100644 index 00000000000..8df61427a49 --- /dev/null +++ b/libstdc++-v3/testsuite/21_strings/basic_string_view/cons/wchar_t/nonnull.cc @@ -0,0 +1,29 @@ +// 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 +// <http://www.gnu.org/licenses/>. + +// { dg-options "-std=gnu++17 -Wnonnull" } +// { dg-do compile { target c++17 } } + +#include <string_view> + +void +test01() +{ + std::wstring_view s((const wchar_t*)nullptr); // { dg-warning "null arg" } + std::wstring_view t((wchar_t*)nullptr); // { dg-warning "null arg" } + std::wstring_view u(nullptr); // { dg-warning "null arg" } +} diff --git a/libstdc++-v3/testsuite/21_strings/basic_string_view/operations/compare/char/nonnull.cc b/libstdc++-v3/testsuite/21_strings/basic_string_view/operations/compare/char/nonnull.cc new file mode 100644 index 00000000000..1d12b700d88 --- /dev/null +++ b/libstdc++-v3/testsuite/21_strings/basic_string_view/operations/compare/char/nonnull.cc @@ -0,0 +1,29 @@ +// 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 +// <http://www.gnu.org/licenses/>. + +// { dg-options "-std=gnu++17 -Wnonnull" } +// { dg-do compile { target c++17 } } + +#include <string_view> + +int +test01() +{ + std::string_view s = "abcd"; + return s.compare((const char*)nullptr); // { dg-warning "null arg" } + return s.compare(0, 2, (const char*)nullptr); // { dg-warning "null arg" } +} diff --git a/libstdc++-v3/testsuite/21_strings/basic_string_view/operations/find/char/nonnull.cc b/libstdc++-v3/testsuite/21_strings/basic_string_view/operations/find/char/nonnull.cc new file mode 100644 index 00000000000..69f1850d9dc --- /dev/null +++ b/libstdc++-v3/testsuite/21_strings/basic_string_view/operations/find/char/nonnull.cc @@ -0,0 +1,33 @@ +// 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 +// <http://www.gnu.org/licenses/>. + +// { dg-options "-std=gnu++17 -Wnonnull" } +// { dg-do compile { target c++17 } } + +#include <string_view> + +int +test01() +{ + std::string_view s = "abcd"; + return s.find((const char*)nullptr); // { dg-warning "null arg" } + return s.find((const char*)nullptr, 1); // { dg-warning "null arg" } + return s.find_first_of((const char*)nullptr); // { dg-warning "null arg" } + return s.find_first_of((const char*)nullptr, 1); // { dg-warning "null arg" } + return s.find_first_not_of((const char*)nullptr); // { dg-warning "null arg" } + return s.find_first_not_of((const char*)nullptr, 1); // { dg-warning "null arg" } +} diff --git a/libstdc++-v3/testsuite/21_strings/basic_string_view/operations/rfind/char/nonnull.cc b/libstdc++-v3/testsuite/21_strings/basic_string_view/operations/rfind/char/nonnull.cc new file mode 100644 index 00000000000..ca8527afa58 --- /dev/null +++ b/libstdc++-v3/testsuite/21_strings/basic_string_view/operations/rfind/char/nonnull.cc @@ -0,0 +1,29 @@ +// 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 +// <http://www.gnu.org/licenses/>. + +// { dg-options "-std=gnu++17 -Wnonnull" } +// { dg-do compile { target c++17 } } + +#include <string_view> + +int +test01() +{ + std::string_view s = "abcd"; + return s.rfind((const char*)nullptr); // { dg-warning "null arg" } + return s.rfind((const char*)nullptr, 1); // { dg-warning "null arg" } +}
The C++ committee has confirmed that passing a null pointer to the unary basic_string_view constructor is undefined. This removes the check from our implementation, and adds the nonnull attribute to warn when the compiler can detect undefined input. Any objections to this change? commit 05d33d556eaabb3c81f4db15b9818c465b359c15 Author: Jonathan Wakely <jwakely@redhat.com> Date: Wed Jun 13 16:54:47 2018 +0100 Decorate string_view members with nonnull attribute The C++ committee has confirmed that passing a null pointer to the unary basic_string_view constructor is undefined. This removes the check from our implementation, and adds the nonnull attribute to warn when the compiler can detect undefined input. * include/std/string_view (basic_string_view(const CharT*)): Remove check for null pointer and add nonnull attribute. (compare(const CharT*), compare(size_type, size_type, const CharT*)) (find(const CharT*, size_type), rfind(const CharT*, size_type)) (find_first_of(const CharT*, size_type)) (find_last_of(const CharT*, size_type)) (find_first_not_of(const CharT*, size_type)) (find_last_not_of(const CharT*, size_type)): Add nonnull attribute. * testsuite/21_strings/basic_string_view/cons/char/nonnull.cc: New. * testsuite/21_strings/basic_string_view/operations/compare/char/ nonnull.cc: New. * testsuite/21_strings/basic_string_view/operations/find/char/ nonnull.cc: New. * testsuite/21_strings/basic_string_view/operations/rfind/char/ nonnull.cc: New.