zmij / math

Small C++17 library for vector and matrix computations
Apache License 2.0
49 stars 9 forks source link

Example code in README.md does not compile with C++17 #9

Open jussihi opened 2 years ago

jussihi commented 2 years ago

In readme, it reads;

#include <psst/math/polar_coord.hpp>
#include <psst/math/spherical_coord.hpp>
#include <psst/math/cylindrical_coord.hpp>

using polar_c       = psst::math::polar_coord<double>;
using spherical_c   = psst::math::spherical_coord<double>;
using cylindrical_c = psst::math::cylindrical_coord<double>;
using vec3          = psst::math::vector<double, 3>;

using psst::math::operator "" _deg;

void a()
{
  polar_c p{10, 180_deg};
  spherical_c s = convert<spherical_c>(p);
  s.inclination() = 45_deg;
  cylindrical_c c = convert<cylindrical_c>(s);
  vec3 v = convert<vec3>(c);
}

However, this does not compile on C++17, at least on GCC 12.2.0.

error: variable 'polar_c p' has initializer but incomplete type
error: variable 'spherical_c s' has initializer but incomplete type
error: variable 'cylindrical_c c' has initializer but incomplete type
error: variable 'vec3 v' has initializer but incomplete type

Did I understand something wrong or is this a bug?

jussihi commented 2 years ago

Also, I would like to use a spherical coordinate in my class instances as a member variable, but including the following line in my class

psst::math::spherical_coord<double> m_coord;

results in

error: aggregate 'psst::math::polar_coord<double> m_coord' has incomplete type and cannot be defined
zmij commented 2 years ago

Hello @jussihi

Thanks for your report, I'll have a look into it

jussihi commented 2 years ago

@zmij ,

I got the incomplete type errors fixed by including the <psst/math/vector.hpp> header!

However, conversions still fail. Maybe some header needs to be included for them as well?

zmij commented 2 years ago

Сonversion is in <psst/math/detail/conversion.hpp> header. Thank you for pointing that out, I'll try and figure out how to make that more convenient

jussihi commented 2 years ago

Thanks for the tip @zmij . However, with this little sample:

#include <psst/math/vector.hpp>
#include <psst/math/detail/conversion.hpp>
#include <psst/math/spherical_coord.hpp>

void a()
{
  psst::math::spherical_coord<double> a{4.0, 1.0, 0.6};
  psst::math::vector<double, 3> v3 = psst::math::convert(a);
}

I still get the compiler error

error: no matching function for call to 'convert(psst::math::spherical_coord<double>&)'
   12 |   psst::math::vector<double, 3> v3 = psst::math::convert(a);
      |                                      ~~~~~~~~~~~~~~~~~~~^~~
...
.../math/include/psst/math/detail/conversion.hpp:90:1: note: candidate: 'template<class Target, class Expression> constexpr auto psst::math::convert(Expression&&)'
   90 | convert(Expression&& expr)
      | ^~~~~~~
.../math/include/psst/math/detail/conversion.hpp:90:1: note:   template argument deduction/substitution failed:
test.cpp:12:57: note:   couldn't deduce template parameter 'Target'
   12 |   psst::math::vector<double, 3> v3 = psst::math::convert(a);
      |                                      ~~~~~~~~~~~~~~~~~~~^~~

I guess it's still missing something..?

zmij commented 2 years ago

The convert function has two template parameters Source and Target, Source is deduced from the function's argument, and the target must be specified explicitly, e.g.

auto v3 = convert<psst::math::vector<double, 3>>(a);
jussihi commented 2 years ago

The convert function has two template parameters Source and Target, Source is deduced from the function's argument, and the target must be specified explicitly, e.g.

auto v3 = convert<psst::math::vector<double, 3>>(a);

Ah, stupid me. This fixes it - you can close this issue if you want to.

Thanks for the support!

jussihi commented 2 years ago

Actually, the conversion still fails:

#include <psst/math/vector.hpp>
#include <psst/math/detail/conversion.hpp>
#include <psst/math/spherical_coord.hpp>

void a()
{
  psst::math::spherical_coord<double> loc_s{4.0, 1.0, 0.6};
  auto loc_c = psst::math::convert<psst::math::vector<double, 3>>(loc_s);
}

results to

.../math/include/psst/math/vector.hpp:11,
                 from test.cpp:11:
.../math/detail/conversion.hpp: In instantiation of 'constexpr auto psst::math::convert(Expression&&) [with Target = vector<double, 3>; Expression = vector<double, 3, components::spherical>&]':
test:404:66:   required from here
.../math/detail/conversion.hpp:95:26: error: static assertion failed: Conversion between theses components is not defined
   95 |     static_assert((expr::conversion_exists_v<Expression, Target>),
      |                   ~~~~~~~^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
.../math/include/psst/math/detail/conversion.hpp:95:26: note: 'psst::math::expr::v::conversion_exists_v<psst::math::vector<double, 3, psst::math::components::spherical>&, psst::math::vector<double, 3> >' evaluates to false
In file included from .../math/include/psst/math/detail/vector_expressions.hpp:12,
                 from .../math/include/psst/math/detail/conversion.hpp:11:
.../math/include/psst/math/detail/expressions.hpp: In instantiation of 'constexpr auto psst::math::expr::make_unary_expression(Argument&&) [with ExpressionType = v::bind_conversion_args<psst::math::vector<double, 3, psst::math::components::spherical>, psst::math::vector<double, 3> >::type; Argument = psst::math::vector<double, 3, psst::math::components::spherical>&]':
.../math/include/psst/math/detail/conversion.hpp:102:90:   required from 'constexpr auto psst::math::convert(Expression&&) [with Target = vector<double, 3>; Expression = vector<double, 3, components::spherical>&]'
test.cpp:404:66:   required from here
.../math/include/psst/math/detail/expressions.hpp:93:60: error: invalid use of incomplete type 'psst::math::expr::v::bind_conversion_args<psst::math::vector<double, 3, psst::math::components::spherical>, psst::math::vector<double, 3> >::type<psst::math::vector<double, 3, psst::math::components::spherical> >' {aka 'struct psst::math::expr::v::conversion<psst::math::vector<double, 3, psst::math::components::spherical>, psst::math::vector<double, 3>, psst::math::vector<double, 3, psst::math::components::spherical> >'}
   93 |         static_cast<expression_argument_t<Argument&&>>(arg)};
      |                                                            ^
.../math/include/psst/math/detail/conversion.hpp:20:8: note: declaration of 'psst::math::expr::v::bind_conversion_args<psst::math::vector<double, 3, psst::math::components::spherical>, psst::math::vector<double, 3> >::type<psst::math::vector<double, 3, psst::math::components::spherical> >' {aka 'struct psst::math::expr::v::conversion<psst::math::vector<double, 3, psst::math::components::spherical>, psst::math::vector<double, 3>, psst::math::vector<double, 3, psst::math::components::spherical> >'}
   20 | struct conversion;
      |        ^~~~~~~~~~
jussihi commented 2 years ago

Ah, the right header to include was <psst/math/coordinate_conversion.hpp>, not <psst/math/detail/conversion.hpp>. Now I think this can be closed :-)