Tests fail, but not due to the changes I belive

I am in doubt whether this is a proper fix for the problem. Following the same logic, every function should be inline because someone might want to use it in different compilation units.

Alternative option: instantiate the template with whatever point types you need into a library and then link your two c.u. against it.

Inlining it or not will depend in any case on the compiler, the keyword is just a suggestion, so I don't see any problem in this fix. If the inlined code block is long probably the compiler will leave the function call but will create a weak reference for the linker in order to allow to have multiple definitions of the same symbol.
http://stackoverflow.com/questions/10535667/does-it-make-any-sense-to-use-inline-keyword-with-templates

But I do not observe this pattern elsewhere in PCL. We user inline only to mark one-liners, at least that is my impression.

I would like to hear some other opinions before merging this.

Using inline to solve multiple definition errors is pretty common. I don't know whether or not it's a good practice though.

As @taketwo says, it's not done in most of the rest of library, but I think that's because much of the rest of the library is templated, making inline unnecessary. I might be wrong about that though.

Yeah, I also think that's the reason.

Mmm, I overlooked the fact that the class in question is not templated.

After having another look at it I think the problem is that this module does not follow the common PCL recipe: header (.h) + template definitions (.hpp) + explicit instantiations (.cpp). The latter is commented out. I think the proper fix would be to move implementation of PPFHashMapSearch into the .cpp file. Additionally, it would be nice to re-enable template instantiation for PPFRegistration like elsewhere in PCL.

@taketwo for sure it is good to make that change, but the problem remains.

Why? After you move definition of PPFHashMapSearch into .cpp it will be compiled into pcl_registration library. This will be the only definition because .hpp will no longer have it.

Yeah sorry, you are right @taketwo. I am doing the changes right away.

You did not enable explicit template instantiation, so the following lines may be left commented:

//#include <pcl/point_types.h>
//#include <pcl/impl/instantiate.hpp>
//#include <pcl/registration/impl/ppf_registration.hpp>

We also need to add inclusion guards in .h and .cpp files to account for PCL_NO_PRECOMPILE option. Just check how this is done in any other PCL class, e.g. bilateral filter.

Thanks for the comments, trying my best.

In .h file also, otherwise both template definition and pre-instantiated template specializations will be available to the user.

The guard in .h should not surround the whole class, just the inclusion of .hpp in the very end. Please check the link I provided for the bilateral filter. We need the same thing here.

Well... we are steadily going in the right direction :)
Please observe that the logic of the guard is opposite in .cpp and .h files. In header we want to include .hpp only if PCL_NO_PRECOMPILE is defined. (You did the opposite.)

Ah right! sorry for that!

Looks good now, but I thin in .cpp file

#include <pcl/registration/ppf_registration.h>

should be included unconditionally, because it is needed for PPFHashMapSearch.

Great! Could you please squash the commits into one and supply it with a sensible message?

mmm I think I am messing up everything since I have never done it and I am having some problems doing it. Should I use git rebase or git merge --squash?

Have a look here: http://justinhileman.info/article/git-pretty/

Is it ok now?

Almost ;). Do a git push -f, not a git push to get rid of the old commits (and the merge). Also, can you write a better commit message please? Thanks!

Hope now I fixed it :) I am quite new to git

Sorry to say, but it's not. Have a look at this site: https://github.com/PointCloudLibrary/pcl/pull/891/commits given the size of your changes, there should be one commit listed and not 11. You had one listed here: giacomodabisias@65b2cc1. Have a look at the link I provided: http://justinhileman.info/article/git-pretty/.

I think I finally did it

+1

There is something wrong about this pull request, see #906