Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Add BOOST_CXX14_CONSTEXPR to CV assign() #161

Conversation

rogerorr
Copy link
Contributor

@rogerorr rogerorr commented Jul 8, 2020

Without this change the BOOST_CXX14_CONSTEXPR on the constructors has no
effect, as they invoke assign()

rogerorr and others added 2 commits July 8, 2020 14:32
Without this change the BOOST_CXX14_CONSTEXPR on the constructors has no
effect, as they invoke assign()
@JeffGarland
Copy link
Collaborator

Hi Roger -- I did try this out and I was unable to get this to resolve at compile time with gcc at least. I believe that is a result of the exception paths that are used by the higher order objects for handling out of range values. I think to make this really work there needs to be if constexpr logic to avoid exceptions in the compile time case -- which is a bit more involved. So I can merge the change, but I don't think it's having the impact you're looking for.

@rogerorr
Copy link
Contributor Author

rogerorr commented Jul 8, 2020

That's odd. The execption paths shouldn't matter when being evaluated in a constexpr context (as long as they're not taken of course.)

My trivial test case:

#include <boost/date_time.hpp>

static constexpr boost::posix_time::ptime UNIX_EPOCH(boost::gregorian::date(1970, 1, 1));

Using g++ -std=c++17 trivial_test.cpp

Without this change:

constrained_value.hpp:48:13: error: call to non-'constexpr' function 'void boost::CV::constrained_value<value_policies>::assign(boost::CV::constrained_value<value_policies>::value_type) [with value_policies = boost::CV::simple_exception_policy<short unsigned int, 1400, 9999, boost::gregorian::bad_year>; boost::CV::constrained_value<value_policies>::value_type = short unsigned int]'
   48 |       assign(value);
      |       ~~~~~~^~~~~~~

With this change it compiles successfully.

(I'm using gcc trunk from 20200601)

@mclow
Copy link
Contributor

mclow commented Jul 8, 2020

What Roger said (about constexpr and exceptions).
I wrote about this on SO: https://stackoverflow.com/questions/33716507/how-can-this-code-be-constexpr-stdchrono

@JeffGarland
Copy link
Collaborator

Ok interesting - today I learned again :)

Looking back on my changes I was testing a bit more directly.

  //test_constrained_value.cpp

 class day_value_policies
 {
  public:
    typedef unsigned int value_type;
    static unsigned int min BOOST_PREVENT_MACRO_SUBSTITUTION () { return 0; };
    static unsigned int max BOOST_PREVENT_MACRO_SUBSTITUTION () { return 31;};
    static void on_error(unsigned int&, unsigned int, boost::CV::violation_enum)
    {
      throw bad_day();
   }
  };
...
  //check constexpr case
  constexpr constrained_value<day_value_policies> cv3(1);
  static_assert(cv3 == 1, "constexpr construction/conversion");
  check("constexpr constrained value construct and equal", true);

And now that I'm looking again I see the compiler is telling me exactly the problem here: min and max, which are executed need to be constexpr. So looking at greg_month and how this is used upstream it indeed looks like it will indeed work.

@rogerorr
Copy link
Contributor Author

rogerorr commented Jul 8, 2020

Good -- that all makes sense. And you've already learned something today so you're ahead :)

@JeffGarland JeffGarland merged commit 7749580 into boostorg:develop Jul 8, 2020
@JeffGarland
Copy link
Collaborator

lol - c++ is a life long learning project :)

JeffGarland added a commit that referenced this pull request Jul 16, 2020
* Change greg_month and greg_weekday to be not marked as exported/imported (#146)

* Changed greg_month and greg_weekday to be not marked as exported/imported.

Both greg_month and greg_weekday classes are implemented completely in headers,
so they need not be marked with BOOST_DATE_TIME_DECL. For consistency with other
similar classes, they are now marked with BOOST_SYMBOL_VISIBLE.

This should fix linking errors on Windows/MSVC, where all members of greg_month
and greg_weekday classes remain unresolved as they are expected to be
implemented in a shared library.

* Enabled shared and static linking in AppVeyor CI.

* fix new msvc warning in date_parsing with Warn4 - issue #148

* Cast month numbers to the correct type in map init (#152)

Since the map contains unsigned short for month numbers, it is more correct to explicitly cast constants to that type rather than short.

* Changes for Embarcadero C++ clang-based compilers, targeting Boost 1.74 (#150)

* Change __BORLANDC__ to BOOST_BORLANDC, which is defined in Boost config for the Embarcadero non-clang-based compilers.

* Change BOOST_BORLANDC back to __BORLANDC__ for non-posix functionality.

* Include the header file.

* Inline friend function definitions for exported/imported classes must become declarations and inline definitions outside the class for Embarcadero C++ clang-based compilers. This bug has been reported to Embarcadero.

* Revert "Inline friend function definitions for exported/imported classes must become declarations and inline definitions outside the class for Embarcadero C++ clang-based compilers. This bug has been reported to Embarcadero."

This reverts commit 88e45e9.

* Inline friend function definitions for exported/imported classes must become declarations and inline definitions outside the class for Embarcadero C++ clang-based compilers. This bug has been reported to Embarcadero.

* Remove separate outside of template class friend functions.

* Define all the friend functions outside the class to conform with the necessity of Embarcadero C++ clang-based compilers that need the friend functions in exported/imported classes to be defined outside the class. This is an Embarcadero C++ clang-based compiler bug which i reported to Embarcadero.

Co-authored-by: Jeff Garland <jeff@crystalclearsoftware.com>

* tune: add initial cmake configuration (#151)

* tune: add initial cmake configuration

* bugfix: Incorrect library alias

Co-authored-by: Jeff Garland <jeff@crystalclearsoftware.com>

* Use BOOST_OVERRIDE to fix GCC -Wsuggest-override and Clang-tidy modernize-use-override warnings. (#155)

Also fix Clang -Wextra-semi-stmt and Clang-tidy readability-container-size-empty warnings.
Alphabetical order of STL headers.
Fix some misspellings.

Co-authored-by: Eugene Zelenko <eugene@diakopto.com>

* Break DateTime <-> Serialization circular dependency (#154)

Breaks DateTime -> Serialization -> Spirit -> Thread -> DateTime chain.

The Serialization documentation seems to be explicitly allow this:
https://www.boost.org/doc/libs/1_73_0/libs/serialization/doc/traits.html#version
https://www.boost.org/doc/libs/1_73_0/libs/serialization/doc/serialization.html#splitting

Co-authored-by: Jeff Garland <jeff@crystalclearsoftware.com>

* Fix clang 10 C++17 warnings about deprecated implicit assignment operators. (#158)

C++17 deprecates implicit generation of assignment operator in presence of
explicitly declared copy constructor. In future standards this behavior may
be removed.

In the affected Boost.DateTime classes, the explicitly defined copy
constructors are functionally equivalent to those would be generated
implicitly, so we can just remove them. The additional benefit is that the
implicitly generated ones will have constexpr and noexcept specifications,
and will also allow implicit move constructors and assignment operators.

* Suppress MSVC CRT deprecations on Clang (#160)

Co-authored-by: Jeff Garland <jeff@crystalclearsoftware.com>

* Add BOOST_CXX14_CONSTEXPR to CV assign() (#161)

Without this change the BOOST_CXX14_CONSTEXPR on the constructors has no
effect, as they invoke assign()

Co-authored-by: Jeff Garland <jeff@crystalclearsoftware.com>

* add constexpr tests to date, ptime, constrained_value (#162)

* merge some doc changes for constexpr fix (#163)

* add constexpr tests to date, ptime, constrained_value

* add some docs for 1.74 release

Co-authored-by: Andrey Semashev <Lastique@users.noreply.github.com>
Co-authored-by: Edward Diener <eldlistmailingz@tropicsoft.com>
Co-authored-by: tapika <tapika@yahoo.com>
Co-authored-by: EugeneZelenko <eugene.zelenko@gmail.com>
Co-authored-by: Eugene Zelenko <eugene@diakopto.com>
Co-authored-by: Nikita Kniazev <nok.raven@gmail.com>
Co-authored-by: Roger Orr <rogero@howzatt.co.uk>
JeffGarland added a commit that referenced this pull request Mar 5, 2021
* Change greg_month and greg_weekday to be not marked as exported/imported (#146)

* Changed greg_month and greg_weekday to be not marked as exported/imported.

Both greg_month and greg_weekday classes are implemented completely in headers,
so they need not be marked with BOOST_DATE_TIME_DECL. For consistency with other
similar classes, they are now marked with BOOST_SYMBOL_VISIBLE.

This should fix linking errors on Windows/MSVC, where all members of greg_month
and greg_weekday classes remain unresolved as they are expected to be
implemented in a shared library.

* Enabled shared and static linking in AppVeyor CI.

* fix new msvc warning in date_parsing with Warn4 - issue #148

* Cast month numbers to the correct type in map init (#152)

Since the map contains unsigned short for month numbers, it is more correct to explicitly cast constants to that type rather than short.

* Changes for Embarcadero C++ clang-based compilers, targeting Boost 1.74 (#150)

* Change __BORLANDC__ to BOOST_BORLANDC, which is defined in Boost config for the Embarcadero non-clang-based compilers.

* Change BOOST_BORLANDC back to __BORLANDC__ for non-posix functionality.

* Include the header file.

* Inline friend function definitions for exported/imported classes must become declarations and inline definitions outside the class for Embarcadero C++ clang-based compilers. This bug has been reported to Embarcadero.

* Revert "Inline friend function definitions for exported/imported classes must become declarations and inline definitions outside the class for Embarcadero C++ clang-based compilers. This bug has been reported to Embarcadero."

This reverts commit 88e45e9.

* Inline friend function definitions for exported/imported classes must become declarations and inline definitions outside the class for Embarcadero C++ clang-based compilers. This bug has been reported to Embarcadero.

* Remove separate outside of template class friend functions.

* Define all the friend functions outside the class to conform with the necessity of Embarcadero C++ clang-based compilers that need the friend functions in exported/imported classes to be defined outside the class. This is an Embarcadero C++ clang-based compiler bug which i reported to Embarcadero.

Co-authored-by: Jeff Garland <jeff@crystalclearsoftware.com>

* tune: add initial cmake configuration (#151)

* tune: add initial cmake configuration

* bugfix: Incorrect library alias

Co-authored-by: Jeff Garland <jeff@crystalclearsoftware.com>

* Use BOOST_OVERRIDE to fix GCC -Wsuggest-override and Clang-tidy modernize-use-override warnings. (#155)

Also fix Clang -Wextra-semi-stmt and Clang-tidy readability-container-size-empty warnings.
Alphabetical order of STL headers.
Fix some misspellings.

Co-authored-by: Eugene Zelenko <eugene@diakopto.com>

* Break DateTime <-> Serialization circular dependency (#154)

Breaks DateTime -> Serialization -> Spirit -> Thread -> DateTime chain.

The Serialization documentation seems to be explicitly allow this:
https://www.boost.org/doc/libs/1_73_0/libs/serialization/doc/traits.html#version
https://www.boost.org/doc/libs/1_73_0/libs/serialization/doc/serialization.html#splitting

Co-authored-by: Jeff Garland <jeff@crystalclearsoftware.com>

* Fix clang 10 C++17 warnings about deprecated implicit assignment operators. (#158)

C++17 deprecates implicit generation of assignment operator in presence of
explicitly declared copy constructor. In future standards this behavior may
be removed.

In the affected Boost.DateTime classes, the explicitly defined copy
constructors are functionally equivalent to those would be generated
implicitly, so we can just remove them. The additional benefit is that the
implicitly generated ones will have constexpr and noexcept specifications,
and will also allow implicit move constructors and assignment operators.

* Suppress MSVC CRT deprecations on Clang (#160)

Co-authored-by: Jeff Garland <jeff@crystalclearsoftware.com>

* Add BOOST_CXX14_CONSTEXPR to CV assign() (#161)

Without this change the BOOST_CXX14_CONSTEXPR on the constructors has no
effect, as they invoke assign()

Co-authored-by: Jeff Garland <jeff@crystalclearsoftware.com>

* add constexpr tests to date, ptime, constrained_value (#162)

* merge some doc changes for constexpr fix (#163)

* add constexpr tests to date, ptime, constrained_value

* add some docs for 1.74 release

* const-qualify retrieval and comparison methods of time_itr and date_itr_base (#170)

* [skip ci] Add "cxxstd" json field. The "cxxstd" json field is being added to each Boost library's meta json information for libraries in order to specify the minumum C++ standard compilation level. The value of this field matches one of the values for 'cxxstd' in Boost.Build. The purpose of doing this is to provide information for the Boost website documentation for each library which will specify the minimum C++ standard compilation that an end-user must employ in order to use the particular library. This will aid end-users who want to know if they can successfully use a Boost library based on their C++ compiler's  compilation level, without having to search the library's documentation to find this out. (#178)

* add drone config [ci skip] (#180)

* Add GitHub Actions config [ci skip] (#182)

Co-authored-by: Andrey Semashev <Lastique@users.noreply.github.com>
Co-authored-by: Edward Diener <eldlistmailingz@tropicsoft.com>
Co-authored-by: tapika <tapika@yahoo.com>
Co-authored-by: EugeneZelenko <eugene.zelenko@gmail.com>
Co-authored-by: Eugene Zelenko <eugene@diakopto.com>
Co-authored-by: Nikita Kniazev <nok.raven@gmail.com>
Co-authored-by: Roger Orr <rogero@howzatt.co.uk>
Co-authored-by: klaus triendl <klaus@triendl.eu>
Co-authored-by: Sam Darwin <samuel.d.darwin@gmail.com>
JeffGarland added a commit that referenced this pull request Jun 28, 2021
* Change greg_month and greg_weekday to be not marked as exported/imported (#146)

* Changed greg_month and greg_weekday to be not marked as exported/imported.

Both greg_month and greg_weekday classes are implemented completely in headers,
so they need not be marked with BOOST_DATE_TIME_DECL. For consistency with other
similar classes, they are now marked with BOOST_SYMBOL_VISIBLE.

This should fix linking errors on Windows/MSVC, where all members of greg_month
and greg_weekday classes remain unresolved as they are expected to be
implemented in a shared library.

* Enabled shared and static linking in AppVeyor CI.

* fix new msvc warning in date_parsing with Warn4 - issue #148

* Cast month numbers to the correct type in map init (#152)

Since the map contains unsigned short for month numbers, it is more correct to explicitly cast constants to that type rather than short.

* Changes for Embarcadero C++ clang-based compilers, targeting Boost 1.74 (#150)

* Change __BORLANDC__ to BOOST_BORLANDC, which is defined in Boost config for the Embarcadero non-clang-based compilers.

* Change BOOST_BORLANDC back to __BORLANDC__ for non-posix functionality.

* Include the header file.

* Inline friend function definitions for exported/imported classes must become declarations and inline definitions outside the class for Embarcadero C++ clang-based compilers. This bug has been reported to Embarcadero.

* Revert "Inline friend function definitions for exported/imported classes must become declarations and inline definitions outside the class for Embarcadero C++ clang-based compilers. This bug has been reported to Embarcadero."

This reverts commit 88e45e9.

* Inline friend function definitions for exported/imported classes must become declarations and inline definitions outside the class for Embarcadero C++ clang-based compilers. This bug has been reported to Embarcadero.

* Remove separate outside of template class friend functions.

* Define all the friend functions outside the class to conform with the necessity of Embarcadero C++ clang-based compilers that need the friend functions in exported/imported classes to be defined outside the class. This is an Embarcadero C++ clang-based compiler bug which i reported to Embarcadero.

Co-authored-by: Jeff Garland <jeff@crystalclearsoftware.com>

* tune: add initial cmake configuration (#151)

* tune: add initial cmake configuration

* bugfix: Incorrect library alias

Co-authored-by: Jeff Garland <jeff@crystalclearsoftware.com>

* Use BOOST_OVERRIDE to fix GCC -Wsuggest-override and Clang-tidy modernize-use-override warnings. (#155)

Also fix Clang -Wextra-semi-stmt and Clang-tidy readability-container-size-empty warnings.
Alphabetical order of STL headers.
Fix some misspellings.

Co-authored-by: Eugene Zelenko <eugene@diakopto.com>

* Break DateTime <-> Serialization circular dependency (#154)

Breaks DateTime -> Serialization -> Spirit -> Thread -> DateTime chain.

The Serialization documentation seems to be explicitly allow this:
https://www.boost.org/doc/libs/1_73_0/libs/serialization/doc/traits.html#version
https://www.boost.org/doc/libs/1_73_0/libs/serialization/doc/serialization.html#splitting

Co-authored-by: Jeff Garland <jeff@crystalclearsoftware.com>

* Fix clang 10 C++17 warnings about deprecated implicit assignment operators. (#158)

C++17 deprecates implicit generation of assignment operator in presence of
explicitly declared copy constructor. In future standards this behavior may
be removed.

In the affected Boost.DateTime classes, the explicitly defined copy
constructors are functionally equivalent to those would be generated
implicitly, so we can just remove them. The additional benefit is that the
implicitly generated ones will have constexpr and noexcept specifications,
and will also allow implicit move constructors and assignment operators.

* Suppress MSVC CRT deprecations on Clang (#160)

Co-authored-by: Jeff Garland <jeff@crystalclearsoftware.com>

* Add BOOST_CXX14_CONSTEXPR to CV assign() (#161)

Without this change the BOOST_CXX14_CONSTEXPR on the constructors has no
effect, as they invoke assign()

Co-authored-by: Jeff Garland <jeff@crystalclearsoftware.com>

* add constexpr tests to date, ptime, constrained_value (#162)

* merge some doc changes for constexpr fix (#163)

* add constexpr tests to date, ptime, constrained_value

* add some docs for 1.74 release

* const-qualify retrieval and comparison methods of time_itr and date_itr_base (#170)

* [skip ci] Add "cxxstd" json field. The "cxxstd" json field is being added to each Boost library's meta json information for libraries in order to specify the minumum C++ standard compilation level. The value of this field matches one of the values for 'cxxstd' in Boost.Build. The purpose of doing this is to provide information for the Boost website documentation for each library which will specify the minimum C++ standard compilation that an end-user must employ in order to use the particular library. This will aid end-users who want to know if they can successfully use a Boost library based on their C++ compiler's  compilation level, without having to search the library's documentation to find this out. (#178)

* add drone config [ci skip] (#180)

* Add GitHub Actions config [ci skip] (#182)

* Update README.md

* Update README.md

* Update README.md

fixup comments on readme to match header_only status

* fix #189 for autolink -- driveby change to remove gcc2.95 support (#191)

* ptime from iso error string error with date only (#192)

* fix #189 for autolink -- driveby change to remove gcc2.95 support

* fix #187 date-only case to avoid string exception, beef up error case test

* ci updates (#194)

* fix #189 for autolink -- driveby change to remove gcc2.95 support

* fix #187 date-only case to avoid string exception, beef up error case test

* attempted fix for #193 - ci issues

* attempted fix for #193 - ci issues try 2

* attempted fix for #193 - ci issues try 3

* attempted fix for #193 - ci issues try 4

* attempted fix for #193 - ci issues try 4a

* attempted fix for #193 - ci issues try 5

* attempted fix for #193 - ci issues try 6

* attempted fix for #193 - ci issues try 7 - rm clang 3.8

* Regenerate CMakeLists.txt

* Add stub source file

Co-authored-by: Andrey Semashev <Lastique@users.noreply.github.com>
Co-authored-by: Edward Diener <eldlistmailingz@tropicsoft.com>
Co-authored-by: tapika <tapika@yahoo.com>
Co-authored-by: EugeneZelenko <eugene.zelenko@gmail.com>
Co-authored-by: Eugene Zelenko <eugene@diakopto.com>
Co-authored-by: Nikita Kniazev <nok.raven@gmail.com>
Co-authored-by: Roger Orr <rogero@howzatt.co.uk>
Co-authored-by: klaus triendl <klaus@triendl.eu>
Co-authored-by: Sam Darwin <samuel.d.darwin@gmail.com>
Co-authored-by: Peter Dimov <pdimov@gmail.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants