Re: [PATCH] Decorate string_view members with nonnull attribute
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
How about:
#define noreturn "..."
...
#include
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
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
Re: [PATCH] Decorate string_view members with nonnull attribute
On 06/14/2018 10:54 AM, Ville Voutilainen wrote: On 14 June 2018 at 19:51, 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 How about: #define noreturn "..." ... #include 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
Re: [PATCH] Decorate string_view members with nonnull attribute
On 14 June 2018 at 20:40, Jonathan Wakely 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.
Re: [PATCH] Decorate string_view members with nonnull attribute
On 14/06/18 20:27 +0300, Ville Voutilainen wrote: On 14 June 2018 at 20:21, Ville Voutilainen wrote: On 14 June 2018 at 20:08, Jonathan Wakely 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.
Re: [PATCH] Decorate string_view members with nonnull attribute
On 14 June 2018 at 20:21, Ville Voutilainen wrote: > On 14 June 2018 at 20:08, Jonathan Wakely 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.
Re: [PATCH] Decorate string_view members with nonnull attribute
On 14 June 2018 at 20:08, Jonathan Wakely 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.
Re: [PATCH] Decorate string_view members with nonnull attribute
On 14/06/18 20:02 +0300, Ville Voutilainen wrote: On 14 June 2018 at 19:57, Jonathan Wakely 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.
Re: [PATCH] Decorate string_view members with nonnull attribute
On 14 June 2018 at 19:57, Jonathan Wakely 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?
Re: [PATCH] Decorate string_view members with nonnull attribute
On 14/06/18 19:54 +0300, Ville Voutilainen wrote: On 14 June 2018 at 19:51, 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 How about: #define noreturn "..." ... #include 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]]
Re: [PATCH] Decorate string_view members with nonnull attribute
On 14 June 2018 at 19:51, 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 >> >> How about: >> >> #define noreturn "..." >> ... >> #include >> >> 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.
Re: [PATCH] Decorate string_view members with nonnull attribute
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 How about: #define noreturn "..." ... #include 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.
Re: [PATCH] Decorate string_view members with nonnull attribute
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 How about: #define noreturn "..." ... #include 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
[PATCH] 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.
Any objections to this change?
commit 05d33d556eaabb3c81f4db15b9818c465b359c15
Author: Jonathan Wakely
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..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 @@
