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

Conflicts with multiple versions of nanoflann #212

Open
teub opened this issue Oct 4, 2023 · 3 comments
Open

Conflicts with multiple versions of nanoflann #212

teub opened this issue Oct 4, 2023 · 3 comments

Comments

@teub
Copy link

teub commented Oct 4, 2023

I spent some time debugging an access violation, only to find out the executable wasn't instantiating the correct classes for nanoflann.
Due to how c++ seems to work, the same template class could be defined at different places and there's no compiler error of any sort.

Anyway, long story short : could there be some define to specify a namespace and allow two versions of nanoflann to exist in the same program ?
Of course I tried to do
namespace something
{
#include "nanoflann.hpp"
}
and it doesn't work for various reasons.

Also to preemptively answer why there are 2 different versions in my program : one of them comes from another 3rd party static library, which I'd rather not change.

Thoughts ?

@jlblancoc
Copy link
Owner

I think the main problem is having the two instances of the same library... :-) And more importantly, perhaps even using different build flags (gcc flags... see the compile full command line, e.g. VERBOSE=1 make). If it's not ensured that nanoflann is included with the same compiler flags (those regarding optimizations in particular), there is no way to make it safe to pass a reference or pointer of a kd-tree from one translation unit to another. If using different versions of nanoflann, things may be even worse if in between the number of fields in any of the used classes changed.

In short: I think using two namespaces would not be a solution, unless you can put together a toy example of what's your exact situation and why it's useful. The same header can be included in several files (translation units), and the instantiated objects passed from one to another, as long as the header is actually the same, and the compiler flags do not lead to change in the memory layout.

@teub
Copy link
Author

teub commented Oct 6, 2023

In short: I think using two namespaces would not be a solution, unless you can put together a toy example of what's your exact situation and why it's useful.

Well I did : nanoflann is used by a 3rd party library I'm using(which I didn't know), and I'm also using it.
I don't want to modify the 3rd party library's build system, but I can modify my own one : hence the need for changing the namespace.

I'm using the latest version of nanoflann but the 3rd party library is not.

I think it's a known pattern to allow changing the namespace of a library, see for example how they do it in MathGeoLib :
https://github.com/juj/MathGeoLib/blob/master/src/MathBuildConfig.h (note : it could be done better, by allowing the define to be set by the build system : #ifndef MATH_NAMESPACE_NAME ...).

IMO the fact that a 3rd party lib is using nanoflann without me knowing is good, it means this repository is very popular :-) (and therefore the issue I raised is more likely to happen to others).

I have my own patch to handle the problem now, which I'll remove if you provide a configuration option for it.
Thank you !

@jlblancoc
Copy link
Owner

Feel free of proposing a Pull Request with this feature and we'll review and iterate on top of it, please :-)

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