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

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

Open
jussihi opened this issue Oct 11, 2022 · 9 comments
Open

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

jussihi opened this issue Oct 11, 2022 · 9 comments

Comments

@jussihi
Copy link

jussihi commented Oct 11, 2022

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
Copy link
Author

jussihi commented Oct 11, 2022

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
Copy link
Owner

zmij commented Oct 11, 2022

Hello @jussihi

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

@jussihi
Copy link
Author

jussihi commented Oct 11, 2022

@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
Copy link
Owner

zmij commented Oct 11, 2022

С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
Copy link
Author

jussihi commented Oct 12, 2022

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
Copy link
Owner

zmij commented Oct 12, 2022

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
Copy link
Author

jussihi commented Oct 12, 2022

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
Copy link
Author

jussihi commented Oct 12, 2022

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
Copy link
Author

jussihi commented Oct 13, 2022

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 :-)

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

No branches or pull requests

2 participants