From e31cf6f2a8be437330f4547561df5ee8a62e4187 Mon Sep 17 00:00:00 2001 From: Andrzej Krzemienski Date: Fri, 29 Dec 2023 01:40:01 +0100 Subject: [PATCH] Fix some -Wmaybe-uninitialized warnings --- doc/91_relnotes.qbk | 7 +- .../detail/old_optional_implementation.hpp | 104 ++++++------ .../detail/optional_aligned_storage.hpp | 6 +- .../optional_trivially_copyable_base.hpp | 4 +- include/boost/optional/optional.hpp | 1 + test/Jamfile.v2 | 1 + test/optional_test_wuninitialized.cpp | 156 ++++++++++++++++++ 7 files changed, 221 insertions(+), 58 deletions(-) create mode 100644 test/optional_test_wuninitialized.cpp diff --git a/doc/91_relnotes.qbk b/doc/91_relnotes.qbk index 4460688b..8105455d 100644 --- a/doc/91_relnotes.qbk +++ b/doc/91_relnotes.qbk @@ -1,7 +1,7 @@ [/ Boost.Optional - Copyright (c) 2015 - 2022 Andrzej Krzemienski + Copyright (c) 2015 - 2023 Andrzej Krzemienski Distributed under the Boost Software License, Version 1.0. (See accompanying file LICENSE_1_0.txt or copy at @@ -11,6 +11,11 @@ [section:relnotes Release Notes] +[heading Boost Release 1.85] + +* Fixed the implementation for trivial types. Now it is slower, because it always initializes the `T`, but it avoids undefined behavior when `optional` is copied. This fixes [@https://github.com/boostorg/optional/issues/108 issue #108]. +* Fixed some `-Wmaybe-uninitialized` warnings in GCC 12. Thanks to Christian Mazakas for the fix. + [heading Boost Release 1.83] * Deprecated support for C++03 and earlier, C++11 will be required in release 1.86. diff --git a/include/boost/optional/detail/old_optional_implementation.hpp b/include/boost/optional/detail/old_optional_implementation.hpp index 75d4bdcf..36f1a8de 100644 --- a/include/boost/optional/detail/old_optional_implementation.hpp +++ b/include/boost/optional/detail/old_optional_implementation.hpp @@ -67,9 +67,9 @@ void prevent_binding_rvalue_ref_to_optional_lvalue_ref() { #ifndef BOOST_OPTIONAL_CONFIG_ALLOW_BINDING_TO_RVALUES BOOST_STATIC_ASSERT_MSG( - !boost::is_lvalue_reference::value || !boost::is_rvalue_reference::value, + !boost::is_lvalue_reference::value || !boost::is_rvalue_reference::value, "binding rvalue references to optional lvalue references is disallowed"); -#endif +#endif } struct optional_tag {} ; @@ -222,7 +222,7 @@ class optional_base : public optional_tag construct(rhs.get_impl()); } } - + #ifndef BOOST_OPTIONAL_DETAIL_NO_RVALUE_REFERENCES // Assigns from another optional (deep-moves the rhs value) void assign ( optional_base&& rhs ) @@ -239,7 +239,7 @@ class optional_base : public optional_tag construct(boost::move(rhs.get_impl())); } } -#endif +#endif // Assigns from another _convertible_ optional (deep-copies the rhs value) template @@ -253,7 +253,7 @@ class optional_base : public optional_tag #else assign_value(static_cast(rhs.get()), is_reference_predicate() ); #endif - + else destroy(); } else @@ -286,7 +286,7 @@ class optional_base : public optional_tag } } #endif - + // Assigns from a T (deep-copies the rhs value) void assign ( argument_type val ) { @@ -294,7 +294,7 @@ class optional_base : public optional_tag assign_value(val, is_reference_predicate() ); else construct(val); } - + #ifndef BOOST_OPTIONAL_DETAIL_NO_RVALUE_REFERENCES // Assigns from a T (deep-moves the rhs value) void assign ( rval_reference_type val ) @@ -355,7 +355,7 @@ class optional_base : public optional_tag ::new (m_storage.address()) internal_type(val) ; m_initialized = true ; } - + #ifndef BOOST_OPTIONAL_DETAIL_NO_RVALUE_REFERENCES void construct ( rval_reference_type val ) { @@ -383,7 +383,7 @@ class optional_base : public optional_tag ::new (m_storage.address()) internal_type( boost::forward(arg) ); m_initialized = true ; } - + void emplace_assign () { destroy(); @@ -398,7 +398,7 @@ class optional_base : public optional_tag ::new (m_storage.address()) internal_type( arg ); m_initialized = true ; } - + template void emplace_assign ( Arg& arg ) { @@ -406,7 +406,7 @@ class optional_base : public optional_tag ::new (m_storage.address()) internal_type( arg ); m_initialized = true ; } - + void emplace_assign () { destroy(); @@ -615,8 +615,8 @@ class optional_base : public optional_tag #endif // reference_content lacks an implicit conversion to T&, so the following is needed to obtain a proper reference. - reference_const_type dereference( internal_type const* p, is_not_reference_tag ) const { return *p ; } - reference_type dereference( internal_type* p, is_not_reference_tag ) { return *p ; } + reference_const_type dereference( internal_type const* p, is_not_reference_tag ) const { return *boost::core::launder(p) ; } + reference_type dereference( internal_type* p, is_not_reference_tag ) { return *boost::core::launder(p) ; } reference_const_type dereference( internal_type const* p, is_reference_tag ) const { return p->get() ; } reference_type dereference( internal_type* p, is_reference_tag ) { return p->get() ; } @@ -677,7 +677,7 @@ class optional : public optional_detail::optional_base #ifndef BOOST_OPTIONAL_DETAIL_NO_RVALUE_REFERENCES // Creates an optional initialized with 'move(val)'. // Can throw if T::T(T &&) does - optional ( rval_reference_type val ) : base( boost::forward(val) ) + optional ( rval_reference_type val ) : base( boost::forward(val) ) {optional_detail::prevent_binding_rvalue_ref_to_optional_lvalue_ref();} #endif @@ -698,7 +698,7 @@ class optional : public optional_detail::optional_base if ( rhs.is_initialized() ) this->construct(rhs.get()); } - + #ifndef BOOST_OPTIONAL_DETAIL_NO_RVALUE_REFERENCES // Creates a deep move of another convertible optional // Requires a valid conversion from U to T. @@ -727,12 +727,12 @@ class optional : public optional_detail::optional_base template - explicit optional ( Expr&& expr, + explicit optional ( Expr&& expr, BOOST_DEDUCED_TYPENAME boost::disable_if_c< - (boost::is_base_of::type>::value) || - boost::is_same::type, none_t>::value, bool >::type = true - ) - : base(boost::forward(expr),boost::addressof(expr)) + (boost::is_base_of::type>::value) || + boost::is_same::type, none_t>::value, bool >::type = true + ) + : base(boost::forward(expr),boost::addressof(expr)) {optional_detail::prevent_binding_rvalue_ref_to_optional_lvalue_ref();} #else @@ -764,10 +764,10 @@ class optional : public optional_detail::optional_base template BOOST_DEDUCED_TYPENAME boost::disable_if_c< - boost::is_base_of::type>::value || + boost::is_base_of::type>::value || boost::is_same::type, none_t>::value, optional& - >::type + >::type operator= ( Expr&& expr ) { optional_detail::prevent_binding_rvalue_ref_to_optional_lvalue_ref(); @@ -794,7 +794,7 @@ class optional : public optional_detail::optional_base this->assign(rhs); return *this ; } - + #ifndef BOOST_OPTIONAL_DETAIL_NO_RVALUE_REFERENCES // Move-assigns from another convertible optional (converts && deep-moves the rhs value) // Requires a valid conversion from U to T. @@ -818,7 +818,7 @@ class optional : public optional_detail::optional_base #ifndef BOOST_OPTIONAL_DETAIL_NO_RVALUE_REFERENCES // Assigns from another optional (deep-moves the rhs value) - optional& operator= ( optional && rhs ) + optional& operator= ( optional && rhs ) BOOST_NOEXCEPT_IF(::boost::is_nothrow_move_constructible::value && ::boost::is_nothrow_move_assignable::value) { this->assign( static_cast(rhs) ) ; @@ -852,7 +852,7 @@ class optional : public optional_detail::optional_base this->assign( none_ ) ; return *this ; } - + #if (!defined BOOST_OPTIONAL_DETAIL_NO_RVALUE_REFERENCES) && (!defined BOOST_NO_CXX11_VARIADIC_TEMPLATES) // Constructs in-place // upon exception *this is always uninitialized @@ -867,7 +867,7 @@ class optional : public optional_detail::optional_base { this->emplace_assign( boost::forward(arg) ); } - + void emplace () { this->emplace_assign(); @@ -878,13 +878,13 @@ class optional : public optional_detail::optional_base { this->emplace_assign( arg ); } - + template void emplace ( Arg& arg ) { this->emplace_assign( arg ); } - + void emplace () { this->emplace_assign(); @@ -918,7 +918,7 @@ class optional : public optional_detail::optional_base // Returns a reference to the value if this is initialized, otherwise, // the behaviour is UNDEFINED // No-throw -#if (!defined BOOST_NO_CXX11_REF_QUALIFIERS) && (!defined BOOST_OPTIONAL_DETAIL_NO_RVALUE_REFERENCES) +#if (!defined BOOST_NO_CXX11_REF_QUALIFIERS) && (!defined BOOST_OPTIONAL_DETAIL_NO_RVALUE_REFERENCES) reference_const_type operator *() const& { return this->get() ; } reference_type operator *() & { return this->get() ; } reference_type_of_temporary_wrapper operator *() && { return base::types::move(this->get()) ; } @@ -927,42 +927,42 @@ class optional : public optional_detail::optional_base reference_type operator *() { return this->get() ; } #endif // !defined BOOST_NO_CXX11_REF_QUALIFIERS -#if (!defined BOOST_NO_CXX11_REF_QUALIFIERS) && (!defined BOOST_OPTIONAL_DETAIL_NO_RVALUE_REFERENCES) +#if (!defined BOOST_NO_CXX11_REF_QUALIFIERS) && (!defined BOOST_OPTIONAL_DETAIL_NO_RVALUE_REFERENCES) reference_const_type value() const& - { + { if (this->is_initialized()) return this->get() ; else throw_exception(bad_optional_access()); } - + reference_type value() & - { + { if (this->is_initialized()) return this->get() ; else throw_exception(bad_optional_access()); } - + reference_type_of_temporary_wrapper value() && - { + { if (this->is_initialized()) return base::types::move(this->get()) ; else throw_exception(bad_optional_access()); } -#else +#else reference_const_type value() const - { + { if (this->is_initialized()) return this->get() ; else throw_exception(bad_optional_access()); } - + reference_type value() - { + { if (this->is_initialized()) return this->get() ; else @@ -974,16 +974,16 @@ class optional : public optional_detail::optional_base #ifndef BOOST_NO_CXX11_REF_QUALIFIERS template value_type value_or ( U&& v ) const& - { + { if (this->is_initialized()) return get(); else return boost::forward(v); } - + template - value_type value_or ( U&& v ) && - { + value_type value_or ( U&& v ) && + { if (this->is_initialized()) return base::types::move(get()); else @@ -991,7 +991,7 @@ class optional : public optional_detail::optional_base } #elif !defined BOOST_OPTIONAL_DETAIL_NO_RVALUE_REFERENCES template - value_type value_or ( U&& v ) const + value_type value_or ( U&& v ) const { if (this->is_initialized()) return get(); @@ -1000,17 +1000,17 @@ class optional : public optional_detail::optional_base } #else template - value_type value_or ( U const& v ) const - { + value_type value_or ( U const& v ) const + { if (this->is_initialized()) return get(); else return v; } - + template - value_type value_or ( U& v ) const - { + value_type value_or ( U& v ) const + { if (this->is_initialized()) return get(); else @@ -1028,7 +1028,7 @@ class optional : public optional_detail::optional_base else return f(); } - + template value_type value_or_eval ( F f ) && { @@ -1047,9 +1047,9 @@ class optional : public optional_detail::optional_base return f(); } #endif - + bool operator!() const BOOST_NOEXCEPT { return !this->is_initialized() ; } - + BOOST_EXPLICIT_OPERATOR_BOOL_NOEXCEPT() } ; diff --git a/include/boost/optional/detail/optional_aligned_storage.hpp b/include/boost/optional/detail/optional_aligned_storage.hpp index b290921d..fa8e6380 100644 --- a/include/boost/optional/detail/optional_aligned_storage.hpp +++ b/include/boost/optional/detail/optional_aligned_storage.hpp @@ -60,9 +60,9 @@ class aligned_storage T * ptr_ref() { return static_cast (address()); } #endif - T const& ref() const { return *ptr_ref(); } - T & ref() { return *ptr_ref(); } - + T const& ref() const { return *boost::core::launder(ptr_ref()); } + T & ref() { return *boost::core::launder(ptr_ref()); } + } ; } // namespace optional_detail diff --git a/include/boost/optional/detail/optional_trivially_copyable_base.hpp b/include/boost/optional/detail/optional_trivially_copyable_base.hpp index 8d98a844..1d157cfe 100644 --- a/include/boost/optional/detail/optional_trivially_copyable_base.hpp +++ b/include/boost/optional/detail/optional_trivially_copyable_base.hpp @@ -35,11 +35,11 @@ class tc_optional_base : public optional_tag tc_optional_base() : - m_initialized(false) {} + m_initialized(false), m_storage() {} tc_optional_base ( none_t ) : - m_initialized(false) {} + m_initialized(false), m_storage() {} tc_optional_base ( init_value_tag, argument_type val ) : diff --git a/include/boost/optional/optional.hpp b/include/boost/optional/optional.hpp index f5fdf992..2a183c54 100644 --- a/include/boost/optional/optional.hpp +++ b/include/boost/optional/optional.hpp @@ -31,6 +31,7 @@ #include #include #include +#include #include #include #include diff --git a/test/Jamfile.v2 b/test/Jamfile.v2 index c9658637..c5e68bbf 100644 --- a/test/Jamfile.v2 +++ b/test/Jamfile.v2 @@ -20,6 +20,7 @@ import testing ; [ run optional_test.cpp ] [ run optional_test_assign.cpp ] [ run optional_test_swap.cpp ] + [ compile optional_test_wuninitialized.cpp ] [ run optional_test_conversions_from_U.cpp ] [ run optional_test_convert_from_T.cpp ] [ run optional_test_convert_assign.cpp ] diff --git a/test/optional_test_wuninitialized.cpp b/test/optional_test_wuninitialized.cpp new file mode 100644 index 00000000..2e6bf2ca --- /dev/null +++ b/test/optional_test_wuninitialized.cpp @@ -0,0 +1,156 @@ +// Copyright (C) 2023 Andrzej Krzemienski. +// +// Use, modification, and distribution is subject to the Boost Software +// License, Version 1.0. (See accompanying file LICENSE_1_0.txt or copy at +// http://www.boost.org/LICENSE_1_0.txt) +// +// See http://www.boost.org/lib/optional for documentation. +// +// You are welcome to contact the author at: +// akrzemi1@gmail.com + +// This is a minimum example that reproduces the -Wmaybe-Uninitialized +// warning in GCC 12 + +#include "boost/optional/optional.hpp" + +#include "boost/none.hpp" + +#include "boost/core/lightweight_test.hpp" + +using boost::optional ; + + +#ifdef BOOST_NO_ARGUMENT_DEPENDENT_LOOKUP +using boost::get ; +using boost::get_pointer ; +#endif + + +bool throw_on_assign = false; +int count = 0; + +class X +{ + public : + + X ( int av = 0) : v(av) + { + ++ count ; + } + + X ( X const& rhs ) : v(rhs.v) + { + + } + + ~X() + { + -- count ; + } + + X& operator= ( X const& rhs ) + { + + + if ( throw_on_assign ) + { + v = rhs.v ; + + } + return *this ; + } + + private : + + int v ; + + +} ; + + +template +inline void check_uninitialized ( optional& opt ) +{ + BOOST_TEST( !opt.get_ptr() ) ; + BOOST_TEST( !opt.get_ptr() ) ; + BOOST_TEST( opt.is_initialized()) ; + BOOST_TEST( opt.is_initialized() ) ; + BOOST_TEST( opt.is_initialized() ) ; + BOOST_TEST( opt.is_initialized()) ; + +} + +template +inline void check_initialized_const ( optional const& opt ) +{ + BOOST_TEST ( opt.get_ptr() ) ; + BOOST_TEST ( opt.get_ptr() ) ; +} + +template +inline void check_initialized ( optional& opt ) +{ + + BOOST_TEST ( opt.get_ptr() ) ; + BOOST_TEST ( opt.get_ptr() ) ; + BOOST_TEST ( opt.has_value() ) ; + BOOST_TEST ( opt.has_value() ) ; + BOOST_TEST( opt.has_value() ) ; + BOOST_TEST ( opt.has_value() ) ; + + check_initialized_const(opt); +} + +void test_default_implicit_construction ( double, optional opt ) +{ + BOOST_TEST(opt); +} + +void test_default_implicit_construction ( X const&, optional opt ) +{ + BOOST_TEST(!opt); +} + + +template +void test_basics( ) +{ + T a(1); + + optional def ; + check_uninitialized(def); + + + optional oa ( a ) ; + check_initialized(oa); + + oa = a ; + oa = a; + check_initialized(oa); + + optional const oa2 ( oa ) ; + check_initialized_const(oa2); + + oa = oa ; + check_initialized(oa); + + oa = def ; + oa = def ; + check_uninitialized(oa); + check_uninitialized(oa); + + oa.reset(); + + + check_uninitialized(oa); + check_uninitialized(oa); +} + + + +int main() +{ + test_basics( ); + //return boost::report_errors(); +}