I don't know if PCL is supporting ARM, but CMake 3.15 will introduce ARMClang. Maybe we should add this, too.

//Edit: Your commit message contains a typo (just in case you want to fix it before merge)

No opinion against.

I noticed the following piece of code in the main CMakeLists:

It is nested under an if that checks for Clang. So currently this is never executed. After merging your fix, -stdlib=libstdc++ will get appended on Mac. Do you know why? Do we need this given that this did not work as expected before?

Do you know why? Do we need this given that this did not work as expected before?

The only reason to do so is if you're depending on a 3rd party which was compiled with libstdc++. That's not the default case on OS X. I suspect this is a remnant from the macports age. These days there's no reason to unconditionally set the standard and if there is one, libc++ should be the default on OS X.

Edit: I just noticed the CUDA there... this is an NVCC issue. I don't have a way to test this at the moment. and I'm reading mixed reports from here. I suspect this should no longer be an issue in more recent nvcc versions.

Do you have any experts in mind whom we could summon to get an opinion?

@claudiofantacci do you still have a working cuda toolkit on your mac? Do you mind testing if:

1. currently you can build pcl CUDA code (with the current master branch)
2. if -stdlib=libstdc++ is being added or not as a flag

@SergioRAgostinho yes I do 🚀

I'll try it out tomorrow and should you not hear anything from me anytime soon, just ping me 👍

Thank you.

It feels like we are dealing with same issue in both. The std=c++14 flag is likely not being passed to nvcc. Something that needs to be fixed first in order to test the original case further.

Edit: There's also the issue of has having to propagate the same user selected standard from the host compiler (clang/gcc) to nvcc, i.e., a user can select c++17 and that should probably be consistent between both compilers.

It feels like we are dealing with same issue in both. The std=c++14 flag is likely not being passed to nvcc. Something that needs to be fixed first in order to test the original case further.

By my experience, that kind of flag has always been a pain on UNIX systems.
I usually add something like this in my projects

if (UNIX)
  list(APPEND CUDA_NVCC_FLAGS "-std=c++11")
endif()

We could, of course, do something smarter by, e.g., get the flag from some CMake variable.

We could, of course, do something smarter by, e.g., get the flag from some CMake variable.

There exists a smarter way... starting from CMake 3.8 (we are limited to 3.5 because of Ubuntu 16.04) ;)

There exists a smarter way... starting from CMake 3.8 (we are limited to 3.5 because of Ubuntu 16.04) ;)

Still, the variable seems to be used only if you add CUDA as a language of the project. The variable may not work properly if you do find_package(CUDA) (which unfortunately is still necessary if you want CMake to create IDE projects).

But anyway, good to know! Was unaware of the CUDA_STANDARD variable 👍

The variable may not work properly if you do find_package(CUDA) (which unfortunately is still necessary if you want CMake to create IDE projects).

Mhm? We are using project(... CXX CUDA) in our project and still have an IDE project. So I don't see any reason why someone should still use find_package(CUDA) after CMake 3.8.

Mhm? We are using project(... CXX CUDA) in our project and still have an IDE project. So I don't see any reason why someone should still use find_package(CUDA) after CMake 3.8.

Not all IDE generators support CUDA as a language. For example Xcode does not support it, MSVC does support it, and so forth. PCL project does not set CUDA as a language so you can generate an IDE project out of it for (theoretically) any IDE.

In the particular case of XCode, you can have a look at this issue on CMake's GitLab.

Good to know, seems we have luck, that we don't use XCode ;).

As of now, I see no way around other than adding the explicit flag to NVCC. As you can see, I've added directly the C++14 standard which from what I can tell was only added in CUDA 9.0. I haven't tried yet to see if thing compile simply with C++11. I just wanted to know what's the general opinion on deprecating everything below CUDA 9 if needed.

I just wanted to know what's the general opinion on deprecating everything below CUDA 9 if needed.

As far as I remember you have still Ubuntu 16.04 as lowest compile target and there is only CUDA 7.5 in default repository. So we have following choice:

• Drop CUDA support below 9.0 => requires using repository of Nvidia on older Linux distributions (=> could affects your Azure builds).
• Only allow C++11 in *.cu-files => CUDA 7.5 still supported

@ your latest changes: Does this works?

  if(CMAKE_VERSION VERSION_LESS 3.10)
    list(APPEND CUDA_NVCC_FLAGS "-D_FORCE_INLINES" $<$<OR:$<BOOL:CMAKE_COMPILER_IS_GNUXX>,$<BOOL:CMAKE_COMPILER_IS_CLANG>>:-std=c++14>)
  else()
    set(CMAKE_CUDA_STANDARD 14)
  endif()

Or does CMAKE_CUDA_STANDARD just work in case you are working with enable_language(CUDA)?

Apologies for the massive derailing this PR took. Quick update, just setting the flag to c++11 won't work. Some cuda code includes indirectly pcl_config.h and then

/Users/neglective/Development/3rdparty/pcl/build/include/pcl/pcl_config.h:7:4: error: PCL requires C++14 or above
  #error PCL requires C++14 or above

Or does CMAKE_CUDA_STANDARD just work in case you are working with enable_language(CUDA)?

No idea. I'll give it a try once I manage to compile all cuda code. At this moment only pcl_cuda_io is failing.

Edit:

Drop CUDA support below 9.0 => requires using repository of Nvidia on older Linux distributions (=> could affects your Azure builds).

NVidia has their own ppa which can be added to the current docker image.

Even I like 38a30fa - isn't it something for a separate PR, because it does not have anything to do with Clang?

Even I like 38a30fa - isn't it something for a separate PR, because it does not have anything to do with Clang?

Totally. I'll pull it out and open a separate PR. Don't be lazy like me.

Big request: in case any of you has some apps done with pcl_cuda code, could you test it against this PR. As you can see, host code is being compiled against c++14 and cuda code is being compiled with c++11. From I could tell, GCC should raise no issues as long as everything is compiled with the same compiler, or with a compiler version in which the C++11 standard features were already stable. I suspect the same will be applicable to clang. Running kinfu should (hopefully) be a good test to validate if things are alright.

In summary:

1. Compile pcl with everything related to cuda and gpu active, using this branch
2. Plug in an openni compatible depth camera, or something which works with the OpenNi viewer out of the box
3. Run pcl_kinfu.
4. If you have custom consumer code making use of classes in gpu or cuda, see if they run without issues.

Help \o/

1. Compile pcl with everything related to cuda and gpu active, using this branch

I can just do this as I don't have any code using pcl_cuda. Do you want me to give it a shot?

I wanted to test this PR on Ubuntu 18.04. The first step was to test whether kinect_viewer_cuda works as is, without this change. Unfortunately, the app uses the old OpenNI grabber (not OpenNI2), and at least on my machine it fails to capture data from Asus Xtion. (The NiViewer app shipped with OpeNI also does not work.) So I tried to change kinect_viewer_cuda to use OpenNI2 grabber. It's not as trivial as adding 2 to the name, because it uses different image structures. And the methods in CUDA modules are not specialized for the new image structures. I gave a quick shot at updating those, but hit a problem similar to this, which can not be resolved without updating to new Eigen version.

Unfortunately I don't have much free time now, so have to get out of this rabbit hole until it's too late. Personally, I'd just avoid touching all this stuff. I never used it and don't know the status, but it seems to be totally outdated. Maybe we just set the flags as proposed such that the beast compiles, but add a warning "use at your own risk, might not work properly".

Just to remember as 3.15 is now official:

I don't know if PCL is supporting ARM, but CMake 3.15 will introduce ARMClang. Maybe we should add this, too.

2. Plug in an openni compatible depth camera, or something which works with the OpenNi viewer out of the box

I don't have any device on my PC like this, so I can't test it :(

I don't know if PCL is supporting ARM, but CMake 3.15 will introduce ARMClang. Maybe we should add this, too.

I incorporated a change that @Morwenn proposed in another PR. It should cover that case as well. See https://stackoverflow.com/questions/10046114/in-cmake-how-can-i-test-if-the-compiler-is-clang/10055571#10055571

I also haven't forgotten about this

 if(CMAKE_VERSION VERSION_LESS 3.10)
    list(APPEND CUDA_NVCC_FLAGS "-D_FORCE_INLINES" $<$<OR:$<BOOL:CMAKE_COMPILER_IS_GNUXX>,$<BOOL:CMAKE_COMPILER_IS_CLANG>>:-std=c++14>)
  else()
    set(CMAKE_CUDA_STANDARD 14)
  endif()

I will test a modified version of this, bearing in mind the info from @claudiofantacci that it isn't working with XCode.

I can just do this as I don't have any code using pcl_cuda. Do you want me to give it a shot?

I believe I have the same dev env as you currently, so I expect no issues.

$ clang --version
Apple LLVM version 9.0.0 (clang-900.0.39.2)
Target: x86_64-apple-darwin17.7.0
Thread model: posix
InstalledDir: /Library/Developer/CommandLineTools/usr/bin
$ nvcc --version
nvcc: NVIDIA (R) Cuda compiler driver
Copyright (c) 2005-2018 NVIDIA Corporation
Built on Tue_Jun_12_23:08:12_CDT_2018
Cuda compilation tools, release 9.2, V9.2.148

If something is different and/or you feel bored, by all means give it a go :)

This pull request has been automatically marked as stale because it hasn't had
any activity in the past 60 days. Commenting or adding a new commit to the
pull request will revert this.

Come back whenever you have time. We look forward to your contribution.

Are you still working on this, or should I remove the milestone of 1.11 from this PR?

Remove from the milestone. I can no longer work on this because I decided to upgrade OS and Catalina still doesn't have CUDA support, nor I expect to to have it anytime soon.

Marking this as stale due to 30 days of inactivity. Commenting or adding a new commit to the pull request will revert this.

This fixes #5120, would be great to have this merged, or use the fix in #5120.