From 44d57a1d8bf1a4517c66f347bfa3065fb495affa Mon Sep 17 00:00:00 2001 From: Andrzej Krzemienski Date: Thu, 18 Feb 2016 23:18:01 +0100 Subject: [PATCH] Fix: prevented the binding illegal temporary to optional Older MSVC versions add illegal temporary when you want to assign from const integral value. --- doc/11_development.qbk | 2 +- doc/15_optional_references.qbk | 5 +- doc/28_ref_optional_semantics.qbk | 2 +- doc/90_dependencies.qbk | 8 +- doc/91_relnotes.qbk | 1 + .../optional_reference_binding.html | 26 +++-- ...ailed_semantics___optional_references.html | 2 +- doc/html/boost_optional/relnotes.html | 4 + .../design_overview/the_interface.html | 6 +- .../tutorial/in_place_factories.html | 6 +- .../tutorial/optional_references.html | 106 +++++++++--------- doc/html/index.html | 4 +- doc/html/optional/tutorial.html | 2 - .../detail/optional_reference_spec.hpp | 28 ++++- test/Jamfile.v2 | 1 + test/optional_ref_assign_test_defs.hpp | 20 +--- ...onal_test_ref_convert_assign_const_int.cpp | 6 +- ...ref_convert_assign_const_int_prevented.cpp | 32 ++++++ ...al_test_ref_convert_assign_mutable_int.cpp | 3 +- ...tional_test_ref_convert_assign_non_int.cpp | 6 +- 20 files changed, 161 insertions(+), 109 deletions(-) create mode 100644 test/optional_test_ref_convert_assign_const_int_prevented.cpp diff --git a/doc/11_development.qbk b/doc/11_development.qbk index 9e3ef44..de5d00d 100644 --- a/doc/11_development.qbk +++ b/doc/11_development.qbk @@ -228,7 +228,7 @@ For value access operations `optional<>` uses operators `*` and `->` to lexically warn about the possibly uninitialized state appealing to the familiar pointer semantics w.r.t. to null pointers. -[warning +[caution However, it is particularly important to note that `optional<>` objects are not pointers. [_`optional<>` is not, and does not model, a pointer]. ] diff --git a/doc/15_optional_references.qbk b/doc/15_optional_references.qbk index 2f8b3c6..acb1226 100644 --- a/doc/15_optional_references.qbk +++ b/doc/15_optional_references.qbk @@ -1,6 +1,8 @@  [section Optional references] +[section Overview] + This library allows the template parameter `T` to be of reference type: `T&`, and to some extent, `T const&`. @@ -21,7 +23,7 @@ will nonetheless refer to the same object. * Value-access will actually provide access to the referenced object rather than the reference itself. -[warning On compilers that do not conform to Standard C++ rules of reference binding, operations on optional references might give adverse results: rather than binding a reference to a designated object they may create an unexpected temporary and bind to it. For more details see [link boost_optional.dependencies_and_portability.optional_reference_binding Dependencies and Portability section].] +[caution On compilers that do not conform to Standard C++ rules of reference binding, some operations on optional references are disabled in order to prevent subtle bugs. For more details see [link boost_optional.dependencies_and_portability.optional_reference_binding Dependencies and Portability section].] [heading Rvalue references] @@ -118,3 +120,4 @@ In such a scenario, you can assign the value itself directly, as in: *opt=value; [endsect] +[endsect] diff --git a/doc/28_ref_optional_semantics.qbk b/doc/28_ref_optional_semantics.qbk index ca62937..31dc178 100644 --- a/doc/28_ref_optional_semantics.qbk +++ b/doc/28_ref_optional_semantics.qbk @@ -857,7 +857,7 @@ __SPACE__ * [*Postconditions:] `bool(*this) == bool(rhs)`. -* [*Notes:] This behaviour is called ['rebinding semantics]. See [link boost_optional.tutorial.rebinding_semantics_for_assignment_of_optional_references here] for details. +* [*Notes:] This behaviour is called ['rebinding semantics]. See [link boost_optional.tutorial.optional_references.rebinding_semantics_for_assignment_of_optional_references here] for details. * [*Example:] `` diff --git a/doc/90_dependencies.qbk b/doc/90_dependencies.qbk index cf678dc..80e1803 100644 --- a/doc/90_dependencies.qbk +++ b/doc/90_dependencies.qbk @@ -30,17 +30,17 @@ The implementation uses the following other Boost modules: [section Optional Reference Binding] -On compilers that do not conform to Standard C++ rules of reference binding, operations on optional references might give adverse results: rather than binding a reference to a designated object they may create an unexpected temporary and bind to it. Compilers known to have these deficiencies include GCC versions 4.2, 4.3, 4.4, 4.5; QCC 4.4.2; MSVC versions 8.0, 9.0, 10.0, 11.0, 12.0. On these compilers prefer using direct-initialization and copy assignment of optional references to copy-initialization and assignment from `T&`: +A number of compilers incorrectly treat const lvalues of integral type as rvalues, and create an illegal temporary when binding to an lvalue reference to const in some expressions. This could result in creating an optional lvalue reference that is in fact bound to an unexpected temporary rather than to the intended object. In order to prevent hard to find run-time bugs, this library performs compile-time checks to prevent expressions that would otherwise bind an optional reference to an unexpected temporary. As a consequence, on certain compilers certain pieces of functionality in optional references are missing. In order to maintain a portability of your code across diferent compilers, it is recommended that you only stick to the minimum portable interface of optional references: prefer direct-initialization and copy assignment of optional references to copy-initialization and assignment from `T&`: const int i = 0; optional or1; - optional or2 = i; // not portable - or1 = i; // not portable + optional or2 = i; // caution: not portable + or1 = i; // caution: not portable optional or3(i); // portable or1 = optional(i); // portable -In order to check if your compiler correctly implements reference binding use this test program. +Compilers known to have these deficiencies include GCC versions 4.2, 4.3, 4.4, 4.5, 5.1, 5.2; QCC 4.4.2; MSVC versions 8.0, 9.0, 10.0, 11.0, 12.0. In order to check if your compiler correctly implements reference binding use this test program. #include diff --git a/doc/91_relnotes.qbk b/doc/91_relnotes.qbk index 1dced5f..4af88a5 100644 --- a/doc/91_relnotes.qbk +++ b/doc/91_relnotes.qbk @@ -16,6 +16,7 @@ * Now `boost::optional` is specialized for reference parameters. This addresses a couple of issues: * the `sizeof` of optional reference is that of a pointer, * some bugs connected to copying optional references are gone, + * all run-time bugs caused by incorrect reference binding on some compilers are now turned into compile-time errors, * you can swap optional references: it is like swapping pointers: shalow, underlying objects are not affected, * optional references to abstract types work. * Documented nested typedefs ([@https://svn.boost.org/trac/boost/ticket/5193 Trac #5193]). diff --git a/doc/html/boost_optional/dependencies_and_portability/optional_reference_binding.html b/doc/html/boost_optional/dependencies_and_portability/optional_reference_binding.html index eef5322..fbe4ba8 100644 --- a/doc/html/boost_optional/dependencies_and_portability/optional_reference_binding.html +++ b/doc/html/boost_optional/dependencies_and_portability/optional_reference_binding.html @@ -28,24 +28,30 @@ Reference Binding

- On compilers that do not conform to Standard C++ rules of reference binding, - operations on optional references might give adverse results: rather than - binding a reference to a designated object they may create an unexpected - temporary and bind to it. Compilers known to have these deficiencies include - GCC versions 4.2, 4.3, 4.4, 4.5; QCC 4.4.2; MSVC versions 8.0, 9.0, 10.0, - 11.0, 12.0. On these compilers prefer using direct-initialization and copy - assignment of optional references to copy-initialization and assignment from - T&: + A number of compilers incorrectly treat const lvalues of integral type as + rvalues, and create an illegal temporary when binding to an lvalue reference + to const in some expressions. This could result in creating an optional lvalue + reference that is in fact bound to an unexpected temporary rather than to + the intended object. In order to prevent hard to find run-time bugs, this + library performs compile-time checks to prevent expressions that would otherwise + bind an optional reference to an unexpected temporary. As a consequence, + on certain compilers certain pieces of functionality in optional references + are missing. In order to maintain a portability of your code across diferent + compilers, it is recommended that you only stick to the minimum portable + interface of optional references: prefer direct-initialization and copy assignment + of optional references to copy-initialization and assignment from T&: 

const int i = 0;
 optional<const int&> or1;
-optional<const int&> or2 = i;  // not portable
-or1 = i;                       // not portable
+optional<const int&> or2 = i;  // caution: not portable
+or1 = i;                       // caution: not portable
 
 optional<const int&> or3(i);   // portable
 or1 = optional<const int&>(i); // portable

+ Compilers known to have these deficiencies include GCC versions 4.2, 4.3, + 4.4, 4.5, 5.1, 5.2; QCC 4.4.2; MSVC versions 8.0, 9.0, 10.0, 11.0, 12.0. In order to check if your compiler correctly implements reference binding use this test program.

diff --git a/doc/html/boost_optional/reference/header__boost_optional_optional_hpp_/detailed_semantics___optional_references.html b/doc/html/boost_optional/reference/header__boost_optional_optional_hpp_/detailed_semantics___optional_references.html index 05e3e21..0bca093 100644 --- a/doc/html/boost_optional/reference/header__boost_optional_optional_hpp_/detailed_semantics___optional_references.html +++ b/doc/html/boost_optional/reference/header__boost_optional_optional_hpp_/detailed_semantics___optional_references.html @@ -189,7 +189,7 @@
  • Notes: This behaviour is called rebinding - semantics. See here + semantics. See here for details.
  • diff --git a/doc/html/boost_optional/relnotes.html b/doc/html/boost_optional/relnotes.html index 53b9020..0f05f9e 100644 --- a/doc/html/boost_optional/relnotes.html +++ b/doc/html/boost_optional/relnotes.html @@ -43,6 +43,10 @@
  • some bugs connected to copying optional references are gone,
    • +
  • + all run-time bugs caused by incorrect reference binding on some compilers + are now turned into compile-time errors, +
  • you can swap optional references: it is like swapping pointers: shalow, underlying objects are not affected, diff --git a/doc/html/boost_optional/tutorial/design_overview/the_interface.html b/doc/html/boost_optional/tutorial/design_overview/the_interface.html index 86c886f..77e17a5 100644 --- a/doc/html/boost_optional/tutorial/design_overview/the_interface.html +++ b/doc/html/boost_optional/tutorial/design_overview/the_interface.html @@ -142,10 +142,10 @@ about the possibly uninitialized state appealing to the familiar pointer semantics w.r.t. to null pointers.

    -
    +
    - - + +
    [Warning]Warning[Caution]Caution

    However, it is particularly important to note that optional<> objects are not pointers. optional<> is not, and does not model, a diff --git a/doc/html/boost_optional/tutorial/in_place_factories.html b/doc/html/boost_optional/tutorial/in_place_factories.html index ed63397..3b72041 100644 --- a/doc/html/boost_optional/tutorial/in_place_factories.html +++ b/doc/html/boost_optional/tutorial/in_place_factories.html @@ -6,7 +6,7 @@ - + @@ -20,7 +20,7 @@

    -PrevUpHomeNext +PrevUpHomeNext

    @@ -191,7 +191,7 @@
    -PrevUpHomeNext +PrevUpHomeNext
    diff --git a/doc/html/boost_optional/tutorial/optional_references.html b/doc/html/boost_optional/tutorial/optional_references.html index ff03059..9dd45b3 100644 --- a/doc/html/boost_optional/tutorial/optional_references.html +++ b/doc/html/boost_optional/tutorial/optional_references.html @@ -7,7 +7,7 @@ - + @@ -20,85 +20,87 @@
    -PrevUpHomeNext +PrevUpHomeNext
    +
    +

    - This library allows the template parameter T - to be of reference type: T&, and to some extent, T - const&. -

    + This library allows the template parameter T + to be of reference type: T&, and to some extent, T const&. +

    - However, since references are not real objects some restrictions apply and - some operations are not available in this case: -

    + However, since references are not real objects some restrictions apply + and some operations are not available in this case: +

    • - Converting constructors -
      • + Converting constructors +
    • - Converting assignment -
      • + Converting assignment +
    • - InPlace construction -
      • + InPlace construction +
    • - InPlace assignment -
      • + InPlace assignment +
    • - Value-access via pointer -
      • + Value-access via pointer +

    - Also, even though optional<T&> - treats it wrapped pseudo-object much as a real value, a true real reference - is stored so aliasing will ocurr: -

    + Also, even though optional<T&> treats it wrapped pseudo-object + much as a real value, a true real reference is stored so aliasing will + ocurr: +

    • - Copies of optional<T&> - will copy the references but all these references will nonetheless refer - to the same object. -
      • + Copies of optional<T&> will copy the references but + all these references will nonetheless refer to the same object. +
    • - Value-access will actually provide access to the referenced object rather - than the reference itself. -
      • + Value-access will actually provide access to the referenced object + rather than the reference itself. +
    -
    +
    - - + + + On compilers that do not conform to Standard C++ rules of reference binding, + some operations on optional references are disabled in order to prevent + subtle bugs. For more details see Dependencies + and Portability section. +

    [Warning]Warning[Caution]Caution

    - On compilers that do not conform to Standard C++ rules of reference binding, - operations on optional references might give adverse results: rather than - binding a reference to a designated object they may create an unexpected - temporary and bind to it. For more details see Dependencies - and Portability section. -

    -
    - - Rvalue - references -
    +
    + + Rvalue + references +

    - Rvalue references and lvalue references to const have the ability in C++ - to extend the life time of a temporary they bind to. Optional references - do not have this capability, therefore to avoid surprising effects it is - not possible to initialize an optional references from a temporary. Optional - rvalue references are disabled altogether. Also, the initialization and assignment - of an optional reference to const from rvalue reference is disabled. -

    + Rvalue references and lvalue references to const have the ability in C++ + to extend the life time of a temporary they bind to. Optional references + do not have this capability, therefore to avoid surprising effects it is + not possible to initialize an optional references from a temporary. Optional + rvalue references are disabled altogether. Also, the initialization and + assignment of an optional reference to const from rvalue reference is disabled. + 

    const int& i = 1;            // legal
 optional<const int&> oi = 1; // illegal
    +
    Copyright © 2003-2007 Fernando Luis Cacciola Carballal
    Copyright © 2014-2016 Andrzej Krzemieński

    @@ -109,7 +111,7 @@

    -PrevUpHomeNext +PrevUpHomeNext
    diff --git a/doc/html/index.html b/doc/html/index.html index 15c6d1e..12d4f4d 100644 --- a/doc/html/index.html +++ b/doc/html/index.html @@ -62,8 +62,6 @@
    IO operators
    Optional references
    -
    Rebinding - semantics for assignment of optional references
    In-Place Factories
    A @@ -146,7 +144,7 @@

    - +

    Last revised: February 16, 2016 at 22:59:47 GMT

    Last revised: February 18, 2016 at 23:24:54 GMT
    diff --git a/doc/html/optional/tutorial.html b/doc/html/optional/tutorial.html index 8e41ac8..556944d 100644 --- a/doc/html/optional/tutorial.html +++ b/doc/html/optional/tutorial.html @@ -36,8 +36,6 @@
    IO operators
    Optional references
    -
    Rebinding - semantics for assignment of optional references
    In-Place Factories
    A diff --git a/include/boost/optional/detail/optional_reference_spec.hpp b/include/boost/optional/detail/optional_reference_spec.hpp index 0b08b84..eeabade 100644 --- a/include/boost/optional/detail/optional_reference_spec.hpp +++ b/include/boost/optional/detail/optional_reference_spec.hpp @@ -12,6 +12,11 @@ #ifndef BOOST_OPTIONAL_DETAIL_OPTIONAL_REFERENCE_SPEC_AJK_03OCT2015_HPP #define BOOST_OPTIONAL_DETAIL_OPTIONAL_REFERENCE_SPEC_AJK_03OCT2015_HPP +#ifdef BOOST_OPTIONAL_CONFIG_NO_PROPER_ASSIGN_FROM_CONST_INT +#include +#include +#endif + # if 1 namespace boost { @@ -39,6 +44,19 @@ BOOST_DEDUCED_TYPENAME boost::remove_reference::type& forward_reference(T&& r #endif // BOOST_OPTIONAL_DETAIL_NO_RVALUE_REFERENCES +template +void prevent_assignment_from_false_const_integral() +{ +#ifndef BOOST_OPTIONAL_CONFIG_ALLOW_BINDING_TO_RVALUES +#ifdef BOOST_OPTIONAL_CONFIG_NO_PROPER_ASSIGN_FROM_CONST_INT + // MSVC compiler without rvalue refernces: we need to disable the asignment from + // const integral lvalue reference, as it may be an invalid temporary + BOOST_STATIC_ASSERT_MSG(!(boost::is_const::value && boost::is_integral::value), + "binding const lvalue references to integral types is disabled in this compiler"); +#endif +#endif +} + template struct is_optional_ { @@ -140,11 +158,15 @@ public: template optional(bool cond, U& v, BOOST_DEDUCED_TYPENAME boost::enable_if >::type* = 0) BOOST_NOEXCEPT : ptr_(cond ? boost::addressof(v) : 0) {} - + template BOOST_DEDUCED_TYPENAME boost::enable_if, optional&>::type - operator=(U& v) BOOST_NOEXCEPT { ptr_ = boost::addressof(v); return *this; } - + operator=(U& v) BOOST_NOEXCEPT + { + detail::prevent_assignment_from_false_const_integral(); + ptr_ = boost::addressof(v); return *this; + } + template void emplace(U& v, BOOST_DEDUCED_TYPENAME boost::enable_if >::type* = 0) BOOST_NOEXCEPT { ptr_ = boost::addressof(v); } diff --git a/test/Jamfile.v2 b/test/Jamfile.v2 index 765fe53..e085c29 100644 --- a/test/Jamfile.v2 +++ b/test/Jamfile.v2 @@ -39,6 +39,7 @@ import testing ; [ run optional_test_emplace.cpp ] [ run optional_test_minimum_requirements.cpp ] [ run optional_test_msvc_bug_workaround.cpp ] + [ compile-fail optional_test_ref_convert_assign_const_int_prevented.cpp ] [ compile-fail optional_test_fail1.cpp ] [ compile-fail optional_test_fail3a.cpp ] [ compile-fail optional_test_fail3b.cpp ] diff --git a/test/optional_ref_assign_test_defs.hpp b/test/optional_ref_assign_test_defs.hpp index 292d0fc..1b2a1b0 100644 --- a/test/optional_ref_assign_test_defs.hpp +++ b/test/optional_ref_assign_test_defs.hpp @@ -163,26 +163,11 @@ void test_rebinding_assignment_semantics() BOOST_TEST_EQ(val(v), 2); } -template +template void test_converting_assignment() { typename concrete_type_of::type v1(1), v2(2), v3(3); - optional oA(v1), oB(none); - - oA = v2; - BOOST_TEST(oA); - BOOST_TEST(addressof(*oA) == addressof(v2)); - - oB = v3; - BOOST_TEST(oB); - BOOST_TEST(addressof(*oB) == addressof(v3)); -} - -template -void test_converting_assignment_for_noconst_const() -{ - typename concrete_type_of::type v1(1), v2(2), v3(3); - optional oA(v1), oB(none); + optional oA(v1), oB(none); oA = v2; BOOST_TEST(oA); @@ -194,4 +179,3 @@ void test_converting_assignment_for_noconst_const() } #endif //BOOST_OPTIONAL_TEST_OPTIONAL_REF_ASSIGN_TEST_DEFS_AK_07JAN2015_HPP - diff --git a/test/optional_test_ref_convert_assign_const_int.cpp b/test/optional_test_ref_convert_assign_const_int.cpp index 30d37a6..fb44aa1 100644 --- a/test/optional_test_ref_convert_assign_const_int.cpp +++ b/test/optional_test_ref_convert_assign_const_int.cpp @@ -23,8 +23,8 @@ int main() { - test_converting_assignment(); - test_converting_assignment_for_noconst_const(); - +#ifndef BOOST_OPTIONAL_CONFIG_NO_PROPER_ASSIGN_FROM_CONST_INT + test_converting_assignment(); +#endif return boost::report_errors(); } diff --git a/test/optional_test_ref_convert_assign_const_int_prevented.cpp b/test/optional_test_ref_convert_assign_const_int_prevented.cpp new file mode 100644 index 0000000..a08ca7a --- /dev/null +++ b/test/optional_test_ref_convert_assign_const_int_prevented.cpp @@ -0,0 +1,32 @@ +// Copyright (C) 2015 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 + + +#include "boost/optional/optional.hpp" + +#ifdef __BORLANDC__ +#pragma hdrstop +#endif + +#include "boost/core/addressof.hpp" +#include "boost/core/enable_if.hpp" +#include "boost/core/lightweight_test.hpp" +#include "testable_classes.hpp" +#include "optional_ref_assign_test_defs.hpp" + +int main() +{ +#ifdef BOOST_OPTIONAL_CONFIG_NO_PROPER_ASSIGN_FROM_CONST_INT + test_converting_assignment(); +#else + BOOST_STATIC_ASSERT(false, "EXPECTED TEST COMPILE-TIME FAILURE"); +#endif + return boost::report_errors(); +} diff --git a/test/optional_test_ref_convert_assign_mutable_int.cpp b/test/optional_test_ref_convert_assign_mutable_int.cpp index 4338e52..56a6e80 100644 --- a/test/optional_test_ref_convert_assign_mutable_int.cpp +++ b/test/optional_test_ref_convert_assign_mutable_int.cpp @@ -23,7 +23,8 @@ int main() { - test_converting_assignment(); + test_converting_assignment(); + test_converting_assignment(); return boost::report_errors(); } diff --git a/test/optional_test_ref_convert_assign_non_int.cpp b/test/optional_test_ref_convert_assign_non_int.cpp index 556b757..80566b2 100644 --- a/test/optional_test_ref_convert_assign_non_int.cpp +++ b/test/optional_test_ref_convert_assign_non_int.cpp @@ -25,9 +25,9 @@ template void test_all_const_cases() { - test_converting_assignment(); - test_converting_assignment(); - test_converting_assignment_for_noconst_const(); + test_converting_assignment(); + test_converting_assignment(); + test_converting_assignment(); } int main()