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

is_constructible has unexpected results #36

Open
kaihowl opened this issue Feb 20, 2019 · 2 comments
Open

is_constructible has unexpected results #36

kaihowl opened this issue Feb 20, 2019 · 2 comments

Comments

@kaihowl
Copy link

kaihowl commented Feb 20, 2019

#include <boost/units/base_units/metric/hour.hpp>
#include <boost/units/base_units/metric/liter.hpp>
#include <boost/units/quantity.hpp>

#include <boost/type_traits/is_constructible.hpp>

using TLiter = boost::units::quantity<boost::units::metric::liter_base_unit::unit_type>;
using THour = boost::units::quantity<boost::units::metric::hour_base_unit::unit_type>;

static_assert(!boost::is_constructible<TLiter, THour>::value, "Should not be constructible from unrelated units");

https://godbolt.org/z/W-7Nvg

This fails to compile before it is able to check the static assertion with the following error:

error: no matching function for call to 'conversion_factor(boost::units::unit<boost::units::list<boost::units::dim<boost::units::time_base_dimension, boost::units::static_rational<1l> >, boost::units::dimensionless_type>, boost::units::heterogeneous_system<boost::units::heterogeneous_system_impl<boost::units::list<boost::units::heterogeneous_system_dim<boost::units::scaled_base_unit<boost::units::si::second_base_unit, boost::units::scale<60l, boost::units::static_rational<2l> > >, boost::units::static_rational<1l> >, boost::units::dimensionless_type>, boost::units::list<boost::units::dim<boost::units::time_base_dimension, boost::units::static_rational<1l> >, boost::units::dimensionless_type>, boost::units::dimensionless_type> >, void>, boost::units::unit<boost::units::list<boost::units::dim<boost::units::length_base_dimension, boost::units::static_rational<3l> >, boost::units::dimensionless_type>, boost::units::heterogeneous_system<boost::units::heterogeneous_system_impl<boost::units::list<boost::units::heterogeneous_system_dim<boost::units::metric::liter_base_unit, boost::units::static_rational<1l> >, boost::units::dimensionless_type>, boost::units::list<boost::units::dim<boost::units::length_base_dimension, boost::units::static_rational<3l> >, boost::units::dimensionless_type>, boost::units::dimensionless_type> >, void>)'

         return(destination_type::from_value(static_cast<T2>(source.value() * conversion_factor(Unit1(), Unit2()))));

This has consequences for using boost::units inside a boost::variant as the constructor of the variant cannot reliably detect if the given source operand can be converted to any of the contained storage types of the variant.

In our source code base with a vendored-in copy of Boost, I fixed the issue by adding a check that conversion_factor is a defined function for the given source and target units of a quantity. If that is not the case, the constructor for explicit conversions is disabled. This does not work with an output test case at this point in time.

Furthermore, the current solution only fixes the problem in C++11 and beyond.

Before digging deeper and creating a proper PR, I wanted to know if I am looking at the right approach or if I am off on the wrong foot.

@swatanabe
Copy link
Contributor

swatanabe commented Feb 20, 2019 via email

@kaihowl
Copy link
Author

kaihowl commented Feb 20, 2019

@swatanabe You are right that makes sense. I came to the same conclusion now. Let's see if I can spin a PR out of this.

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 a pull request may close this issue.

2 participants