| Message ID | 2c589ec7-41ec-e581-29ba-1e35825b2d51@gmail.com |
|---|---|
| State | New |
| Headers | show |
| Series | declare std::array members attribute const [PR101831] | expand |
On Tue, 1 Feb 2022 at 18:54, Martin Sebor via Libstdc++ <libstdc++@gcc.gnu.org> wrote: > > Passing an uninitialized object to a function that takes its argument > by const reference is diagnosed by -Wmaybe-uninitialized because most > such functions read the argument. The exceptions are functions that > don't access the object but instead use its address to compute > a result. This includes a number of std::array member functions such > as std::array<N>::size() which returns the template argument N. Such > functions may be candidates for attribute const which also avoids > the warning. The attribute typically only benefits extern functions > that IPA cannot infer the property from, but in this case it helps > avoid the warning which runs very early on, even without optimization > or inlining. The attached patch adds the attribute to a subset of > those member functions of std::array. (It doesn't add it to const > member functions like cbegin() or front() that return a const_iterator > or const reference to the internal data.) > > It might be possible to infer this property from inline functions > earlier on than during IPA and avoid having to annotate them explicitly. > That seems like an enhancement worth considering in the future. > > Tested on x86_64-linux. > > Martin new file mode 100644 index 00000000000..b7743adf3c9 --- /dev/null +++ b/libstdc++-v3/testsuite/23_containers/array/iterators/begin_end.cc @@ -0,0 +1,56 @@ +// { dg-do compile { target c++11 } } +// +// Copyright (C) 2011-2022 Free Software Foundation, Inc. Those dates look wrong. I no longer bother putting a license text and copyright notice on simple tests like this. It's meaningless to assert copyright on something so trivial that doesn't do anything.
On 2/1/22 12:48, Jonathan Wakely wrote: > On Tue, 1 Feb 2022 at 18:54, Martin Sebor via Libstdc++ > <libstdc++@gcc.gnu.org> wrote: >> >> Passing an uninitialized object to a function that takes its argument >> by const reference is diagnosed by -Wmaybe-uninitialized because most >> such functions read the argument. The exceptions are functions that >> don't access the object but instead use its address to compute >> a result. This includes a number of std::array member functions such >> as std::array<N>::size() which returns the template argument N. Such >> functions may be candidates for attribute const which also avoids >> the warning. The attribute typically only benefits extern functions >> that IPA cannot infer the property from, but in this case it helps >> avoid the warning which runs very early on, even without optimization >> or inlining. The attached patch adds the attribute to a subset of >> those member functions of std::array. (It doesn't add it to const >> member functions like cbegin() or front() that return a const_iterator >> or const reference to the internal data.) >> >> It might be possible to infer this property from inline functions >> earlier on than during IPA and avoid having to annotate them explicitly. >> That seems like an enhancement worth considering in the future. >> >> Tested on x86_64-linux. >> >> Martin > > new file mode 100644 > index 00000000000..b7743adf3c9 > --- /dev/null > +++ b/libstdc++-v3/testsuite/23_containers/array/iterators/begin_end.cc > @@ -0,0 +1,56 @@ > +// { dg-do compile { target c++11 } } > +// > +// Copyright (C) 2011-2022 Free Software Foundation, Inc. > > Those dates look wrong. I no longer bother putting a license text and > copyright notice on simple tests like this. It's meaningless to assert > copyright on something so trivial that doesn't do anything. > Should I take to mean that you're okay with the rest of the change (i.e., with the notice removed)? Martin
On Wed, 2 Feb 2022 at 00:13, Martin Sebor <msebor@gmail.com> wrote: > > On 2/1/22 12:48, Jonathan Wakely wrote: > > On Tue, 1 Feb 2022 at 18:54, Martin Sebor via Libstdc++ > > <libstdc++@gcc.gnu.org> wrote: > >> > >> Passing an uninitialized object to a function that takes its argument > >> by const reference is diagnosed by -Wmaybe-uninitialized because most > >> such functions read the argument. The exceptions are functions that > >> don't access the object but instead use its address to compute > >> a result. This includes a number of std::array member functions such > >> as std::array<N>::size() which returns the template argument N. Such > >> functions may be candidates for attribute const which also avoids > >> the warning. The attribute typically only benefits extern functions > >> that IPA cannot infer the property from, but in this case it helps > >> avoid the warning which runs very early on, even without optimization > >> or inlining. The attached patch adds the attribute to a subset of > >> those member functions of std::array. (It doesn't add it to const > >> member functions like cbegin() or front() that return a const_iterator > >> or const reference to the internal data.) > >> > >> It might be possible to infer this property from inline functions > >> earlier on than during IPA and avoid having to annotate them explicitly. > >> That seems like an enhancement worth considering in the future. > >> > >> Tested on x86_64-linux. > >> > >> Martin > > > > new file mode 100644 > > index 00000000000..b7743adf3c9 > > --- /dev/null > > +++ b/libstdc++-v3/testsuite/23_containers/array/iterators/begin_end.cc > > @@ -0,0 +1,56 @@ > > +// { dg-do compile { target c++11 } } > > +// > > +// Copyright (C) 2011-2022 Free Software Foundation, Inc. > > > > Those dates look wrong. I no longer bother putting a license text and > > copyright notice on simple tests like this. It's meaningless to assert > > copyright on something so trivial that doesn't do anything. > > > > Should I take to mean that you're okay with the rest of the change > (i.e., with the notice removed)? Yes, OK for trunk either with the notice entirely removed, or just fix the dates (I don't think it is copied from an existing test dating from 2011, right?) Whichever you prefer.
On 2/1/22 17:15, Jonathan Wakely wrote: > On Wed, 2 Feb 2022 at 00:13, Martin Sebor <msebor@gmail.com> wrote: >> >> On 2/1/22 12:48, Jonathan Wakely wrote: >>> On Tue, 1 Feb 2022 at 18:54, Martin Sebor via Libstdc++ >>> <libstdc++@gcc.gnu.org> wrote: >>>> >>>> Passing an uninitialized object to a function that takes its argument >>>> by const reference is diagnosed by -Wmaybe-uninitialized because most >>>> such functions read the argument. The exceptions are functions that >>>> don't access the object but instead use its address to compute >>>> a result. This includes a number of std::array member functions such >>>> as std::array<N>::size() which returns the template argument N. Such >>>> functions may be candidates for attribute const which also avoids >>>> the warning. The attribute typically only benefits extern functions >>>> that IPA cannot infer the property from, but in this case it helps >>>> avoid the warning which runs very early on, even without optimization >>>> or inlining. The attached patch adds the attribute to a subset of >>>> those member functions of std::array. (It doesn't add it to const >>>> member functions like cbegin() or front() that return a const_iterator >>>> or const reference to the internal data.) >>>> >>>> It might be possible to infer this property from inline functions >>>> earlier on than during IPA and avoid having to annotate them explicitly. >>>> That seems like an enhancement worth considering in the future. >>>> >>>> Tested on x86_64-linux. >>>> >>>> Martin >>> >>> new file mode 100644 >>> index 00000000000..b7743adf3c9 >>> --- /dev/null >>> +++ b/libstdc++-v3/testsuite/23_containers/array/iterators/begin_end.cc >>> @@ -0,0 +1,56 @@ >>> +// { dg-do compile { target c++11 } } >>> +// >>> +// Copyright (C) 2011-2022 Free Software Foundation, Inc. >>> >>> Those dates look wrong. I no longer bother putting a license text and >>> copyright notice on simple tests like this. It's meaningless to assert >>> copyright on something so trivial that doesn't do anything. >>> >> >> Should I take to mean that you're okay with the rest of the change >> (i.e., with the notice removed)? > > Yes, OK for trunk either with the notice entirely removed, or just fix > the dates (I don't think it is copied from an existing test dating > from 2011, right?) I copied it from 23_containers/array/iterators/end_is_one_past.cc without even looking at the dates. > > Whichever you prefer. > Okay, pushed in r12-6992. Martin
On Wed, 2 Feb 2022 at 00:23, Martin Sebor <msebor@gmail.com> wrote: > > On 2/1/22 17:15, Jonathan Wakely wrote: > > On Wed, 2 Feb 2022 at 00:13, Martin Sebor <msebor@gmail.com> wrote: > >> > >> On 2/1/22 12:48, Jonathan Wakely wrote: > >>> On Tue, 1 Feb 2022 at 18:54, Martin Sebor via Libstdc++ > >>> <libstdc++@gcc.gnu.org> wrote: > >>>> > >>>> Passing an uninitialized object to a function that takes its argument > >>>> by const reference is diagnosed by -Wmaybe-uninitialized because most > >>>> such functions read the argument. The exceptions are functions that > >>>> don't access the object but instead use its address to compute > >>>> a result. This includes a number of std::array member functions such > >>>> as std::array<N>::size() which returns the template argument N. Such > >>>> functions may be candidates for attribute const which also avoids > >>>> the warning. The attribute typically only benefits extern functions > >>>> that IPA cannot infer the property from, but in this case it helps > >>>> avoid the warning which runs very early on, even without optimization > >>>> or inlining. The attached patch adds the attribute to a subset of > >>>> those member functions of std::array. (It doesn't add it to const > >>>> member functions like cbegin() or front() that return a const_iterator > >>>> or const reference to the internal data.) > >>>> > >>>> It might be possible to infer this property from inline functions > >>>> earlier on than during IPA and avoid having to annotate them explicitly. > >>>> That seems like an enhancement worth considering in the future. > >>>> > >>>> Tested on x86_64-linux. > >>>> > >>>> Martin > >>> > >>> new file mode 100644 > >>> index 00000000000..b7743adf3c9 > >>> --- /dev/null > >>> +++ b/libstdc++-v3/testsuite/23_containers/array/iterators/begin_end.cc > >>> @@ -0,0 +1,56 @@ > >>> +// { dg-do compile { target c++11 } } > >>> +// > >>> +// Copyright (C) 2011-2022 Free Software Foundation, Inc. > >>> > >>> Those dates look wrong. I no longer bother putting a license text and > >>> copyright notice on simple tests like this. It's meaningless to assert > >>> copyright on something so trivial that doesn't do anything. > >>> > >> > >> Should I take to mean that you're okay with the rest of the change > >> (i.e., with the notice removed)? > > > > Yes, OK for trunk either with the notice entirely removed, or just fix > > the dates (I don't think it is copied from an existing test dating > > from 2011, right?) > > I copied it from 23_containers/array/iterators/end_is_one_past.cc > without even looking at the dates. Yeah, we all do that, and we all forget to fix the dates. It's just one more reason not to bother putting 16 lines of licence notices in tests, when the notice is often bigger than the test itself! > > > > > Whichever you prefer. > > > > Okay, pushed in r12-6992. Thanks.
Declare std::array members with attribute const [PR101831]. Resolves: PR libstdc++/101831 - Spurious maybe-uninitialized warning on std::array::size libstdc++-v3/ChangeLog: * include/std/array (begin): Declare const member function attribute const. (end, rbegin, rend, size, max_size, empty, data): Same. * testsuite/23_containers/array/capacity/empty.cc: Add test cases. * testsuite/23_containers/array/capacity/max_size.cc: Same. * testsuite/23_containers/array/capacity/size.cc: Same. * testsuite/23_containers/array/iterators/begin_end.cc: New test. diff --git a/libstdc++-v3/include/std/array b/libstdc++-v3/include/std/array index b4d8fc81a52..e45143fb329 100644 --- a/libstdc++-v3/include/std/array +++ b/libstdc++-v3/include/std/array @@ -127,7 +127,7 @@ _GLIBCXX_BEGIN_NAMESPACE_VERSION { std::swap_ranges(begin(), end(), __other.begin()); } // Iterators. - [[__nodiscard__]] + [[__gnu__::__const__, __nodiscard__]] _GLIBCXX17_CONSTEXPR iterator begin() noexcept { return iterator(data()); } @@ -137,7 +137,7 @@ _GLIBCXX_BEGIN_NAMESPACE_VERSION begin() const noexcept { return const_iterator(data()); } - [[__nodiscard__]] + [[__gnu__::__const__, __nodiscard__]] _GLIBCXX17_CONSTEXPR iterator end() noexcept { return iterator(data() + _Nm); } @@ -147,7 +147,7 @@ _GLIBCXX_BEGIN_NAMESPACE_VERSION end() const noexcept { return const_iterator(data() + _Nm); } - [[__nodiscard__]] + [[__gnu__::__const__, __nodiscard__]] _GLIBCXX17_CONSTEXPR reverse_iterator rbegin() noexcept { return reverse_iterator(end()); } @@ -157,7 +157,7 @@ _GLIBCXX_BEGIN_NAMESPACE_VERSION rbegin() const noexcept { return const_reverse_iterator(end()); } - [[__nodiscard__]] + [[__gnu__::__const__, __nodiscard__]] _GLIBCXX17_CONSTEXPR reverse_iterator rend() noexcept { return reverse_iterator(begin()); } @@ -188,15 +188,15 @@ _GLIBCXX_BEGIN_NAMESPACE_VERSION { return const_reverse_iterator(begin()); } // Capacity. - [[__nodiscard__]] + [[__gnu__::__const__, __nodiscard__]] constexpr size_type size() const noexcept { return _Nm; } - [[__nodiscard__]] + [[__gnu__::__const__, __nodiscard__]] constexpr size_type max_size() const noexcept { return _Nm; } - [[__nodiscard__]] + [[__gnu__::__const__, __nodiscard__]] constexpr bool empty() const noexcept { return size() == 0; } @@ -278,7 +278,7 @@ _GLIBCXX_BEGIN_NAMESPACE_VERSION : _AT_Type::_S_ref(_M_elems, 0); } - [[__nodiscard__]] + [[__gnu__::__const__, __nodiscard__]] _GLIBCXX17_CONSTEXPR pointer data() noexcept { return _AT_Type::_S_ptr(_M_elems); } diff --git a/libstdc++-v3/testsuite/23_containers/array/capacity/empty.cc b/libstdc++-v3/testsuite/23_containers/array/capacity/empty.cc index 3f3f282ad9d..cecbae39e45 100644 --- a/libstdc++-v3/testsuite/23_containers/array/capacity/empty.cc +++ b/libstdc++-v3/testsuite/23_containers/array/capacity/empty.cc @@ -40,8 +40,26 @@ test01() } } +#pragma GCC push_options +#pragma GCC optimize "0" + +void +test02() +{ + { + const size_t len = 3; + typedef std::array<int, len> array_type; + array_type a; + + VERIFY( a.empty() == false ); // { dg-bogus "-Wmaybe-uninitialized" + } +} + +#pragma GCC pop_options + int main() { test01(); + test02(); return 0; } diff --git a/libstdc++-v3/testsuite/23_containers/array/capacity/max_size.cc b/libstdc++-v3/testsuite/23_containers/array/capacity/max_size.cc index 0e000258530..4629316161d 100644 --- a/libstdc++-v3/testsuite/23_containers/array/capacity/max_size.cc +++ b/libstdc++-v3/testsuite/23_containers/array/capacity/max_size.cc @@ -40,8 +40,26 @@ test01() } } +#pragma GCC push_options +#pragma GCC optimize "0" + +void +test02() +{ + { + const size_t len = 3; + typedef std::array<int, len> array_type; + array_type a; + + VERIFY( a.max_size() == len ); // { dg-bogus "-Wmaybe-uninitialized" + } +} + +#pragma GCC pop_options + int main() { test01(); + test02(); return 0; } diff --git a/libstdc++-v3/testsuite/23_containers/array/capacity/size.cc b/libstdc++-v3/testsuite/23_containers/array/capacity/size.cc index 3e4aa7143dc..dddd909a0ac 100644 --- a/libstdc++-v3/testsuite/23_containers/array/capacity/size.cc +++ b/libstdc++-v3/testsuite/23_containers/array/capacity/size.cc @@ -40,8 +40,26 @@ test01() } } +#pragma GCC push_options +#pragma GCC optimize "0" + +void +test02() +{ + { + const size_t len = 3; + typedef std::array<int, len> array_type; + array_type a; + + VERIFY( a.size() == len ); // { dg-bogus "-Wmaybe-uninitialized" + } +} + +#pragma GCC pop_options + int main() { test01(); + test02(); return 0; } diff --git a/libstdc++-v3/testsuite/23_containers/array/iterators/begin_end.cc b/libstdc++-v3/testsuite/23_containers/array/iterators/begin_end.cc new file mode 100644 index 00000000000..b7743adf3c9 --- /dev/null +++ b/libstdc++-v3/testsuite/23_containers/array/iterators/begin_end.cc @@ -0,0 +1,56 @@ +// { dg-do compile { target c++11 } } +// +// Copyright (C) 2011-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/>. + +#include <array> + +#pragma GCC push_options +#pragma GCC optimize "0" + +extern void +sink (const void*, ...); + +void +test01() +{ + { + const std::size_t len = 1; + typedef std::array<int, len> array_type; + typedef array_type::iterator iterator;; + array_type a; + + iterator b = a.begin(); // { dg-bogus "-Wmaybe-uninitialized" } + iterator e = a.end(); // { dg-bogus "-Wmaybe-uninitialized" } + + sink(&b, &e); + } + + { + const std::size_t len = 3; + typedef std::array<int, len> array_type; + typedef array_type::reverse_iterator reverse_iterator; + array_type a; + + reverse_iterator b = a.rbegin(); // { dg-bogus "-Wmaybe-uninitialized" } + reverse_iterator e = a.rend(); // { dg-bogus "-Wmaybe-uninitialized" } + + sink(&b, &e); + } +} + +#pragma GCC pop_options