I'm also wondering whether we could perhaps approach this differently: perhaps we could remove the #include of the generated .cpp from the template, make it a separate compilation target in the generated CMakeLists.txt and use set_target_properties(..) / target_compile_definitions(..) to alias isnan with std::isnan only in that particular target.

That would localise the effects of this as much as possible.

I'm not entirely sure why the .cpp was #included the way it is right now though. If it is for performance reasons (ie: inlining) other compiler/linker features could perhaps be used now that they have become available.

@davetcoleman: do you know why the generated source file is #included in the template?

I have another fix/work-around for this that is a little less hacky and more maintainable.

I'll submit a PR for that later this week.

@gavanderhoorn I was about to +1 this when you posted the above comment just now.
Given the circumstances, I believe this is a reasonable solution and it's just 3LOC.
What do you propose instead?

@v4hn wrote:

What do you propose instead?

to make the create_ikfast_moveit_plugin.py post-process the generated c++ source and replace unqualified isnan calls with std::isnan where appropriate. That is a lot less of a shotgun approach to this, as #define will also override all other occurences of isnan, not just those local to the file.

to make the create_ikfast_moveit_plugin.py post-process the generated c++ source and replace unqualified isnan calls with std::isnan where appropriate. That is a lot less of a shotgun approach to this

You are right. I'm looking ahead to merge your working request :)

@gavanderhoorn I do not know why it was done this way. I once had a branch for the ikfast plugin that made all generated IKFast plugins depend upon moveit_kinematics package for all wrapper code around the one robot-specific generated file. If implemented, this would allow all IKFast plugins to be automatically updated whenever moveit_kinematics is updated. Just a slightly off-topic thought that involved moving this #include directive.

I remember discussing that approach before, yes.

The current way of doing things does mean that we have almost total freedom when it comes to breaking things, as existing plugins will not be affected in any way - they are completely stand-alone (as far as that is possible for plugins).

It does mean that upgrades - or desirable changes - do not get propagated easily.

@gavanderhoorn ping

I'll get back to this, had a EU project deadline.

closed in favor of #619