Fix compilation of CUDA code on Windows

[flowvision]

In pcl_macros.h the attribute [[deprecated]] is used. It seems that
nvcc doesn't support this attribute under Windows at the moment.
I tried it with CUDA Toolkit 10.1 with Windows 10. I also checked the
release notes of 10.2 but they didn't address it there.

To fix this I omit the attribute under Windows when nvcc is used using
preprocessor directives. This is the simplest way of avoiding the
issue. A more elegant solution would be if we added a macro similar to
how it is done at the bottom of pcl_macros.h with [[fallthrough]] and
[[nodiscard]].

Let me know if this is ok or if we should use a macro.

[Morwenn]

Adding a macro instead would for sure reduce the code duplication.

[kunaltyagi]

The best way would be to actually remove deprecated usage in the library itself, rather than patch over. That's not to say that the macro is a bad addition since several other headers in common use deprecated attribute.

[flowvision]

I checked the code of the repository and it seems that the deprecated types are not used anymore so I think we can remove them.
The only problem I see in doing this would be if other projects that use the PCL depend on these types.

We should probably do the stuff with the macro in a separate PR.
Maybe we should create an issue out of it because I don't know if I have time for this.
It would also probably be a good first issue.

[larshg]

I fetched this macro from somewhere which does the trick on windows.

if of interest.

[kunaltyagi]

I think we can remove them

They were deprecated for the stability of projects using them. As such, their removal isn't scheduled before 1.12 release.

Your fork seems to have a nice macro suite of macros.

[flowvision]

So should we use the macros from @larshg?

[kunaltyagi]

I liked this version better since it fits in the syntax better IMO. But it'll need to be modified to work with nvcc

[flowvision]

Ok, then I'll use this version.

[flowvision]

I introduced the macros for the different versions of 'deprecated' and replaced all occurences with the macros.
I took the code from @larshg and refactored it a bit so that it is more concise.

To be able to correctly support older versions of the Visual Studio compiler I needed a macro PCL_DEPRECATED_USING specifically for declaring using directives as deprecated. Additionally I also needed to introduce the macro PCL_SINGLE_ARG to be able to pass templated types to this macro (see surface/include/pcl/surface/mls.h) otherwise the comma in the template type interferes with the macro expansion.

If we would ignore the deprecated attribute for Visual Studio compilers older than 2015 we could remove the macros PCL_DEPRECATED_USING and PCL_SINGLE_ARG.
Let me know if you would prefer this.

[kunaltyagi]

Additionally I also needed to introduce the macro PCL_SINGLE_ARG

Did parentheses not work instead of a delay macro? Maybe use a more descriptive name instead of PCL_SINGLE_ARG. PCL_DEFER is a descriptive alternative that I'd prefer personally, but this is bike-shedding. Overall the approach and PR looks good to me. Maybe in future, we might consider using CMake's internal compiler compat code generation.

If we would ignore the deprecated attribute for Visual Studio compilers older than 2015

It's not a lot of code so doesn't matter to me. Others might have differing opinion though. You should however put this as a comment in the code so that in future when 2015 is definitely old enough, then the "dead code can be removed"

[taketwo]

If we would ignore the deprecated attribute for Visual Studio compilers older than 2015 we could remove the macros PCL_DEPRECATED_USING and PCL_SINGLE_ARG

Do I understand your message correctly that Visual Studio 2015 and above do not need PCL_SINGLE_ARG? If so, we can remove it because 2015 is the baseline version.

I think macros PCL_DEPRECATED_USING and PCL_DEPRECATED_MESSAGE are not consistently named. The first one reads like it is deprecating a using (which is correct) and, following the same logic, the second one reads like it is deprecating a... message (which does not make sense). So the latter should rather be PCL_DEPRECATED_WITH_MESSAGE. This is awkward, of course. I think every deprecation has to have a message anyway. So I'd propose to get rid of message-less PCL_DEPRECATED macro and rename PCL_DEPRECATED_MESSAGE into it.

[flowvision]

@kunaltyagi: I tried using parentheses but it didn't work because the will end up with something like
`using identifier = (type)' but (at least on my compiler) you mustn't have parantheses around the type.

@taketwo: No quite. Visual Studio 2015 and above don't need the PCL_DEPRECATED_USING because we could just write the using lines like this:
using identifier PCL_DEPRECATED_MESSAGE("Message") = type<T1,T2>;
When writing it like that we don't need PCL_SINGLE_ARG because the templated type isn't inside a macro and so we don't have the problem with the comma.

I like the version where we don't have a special macro for the using keyword better because you can than use the macro in the same way as you would use the deprecated attribute.

So taking the things into account @taketwo wrote we would end up with only a single macro PCL_DEPRECATED which has a message as parameter and for Visual Studio compilers below 2015 we just ignore the deprecation.

Is this Ok for everyone if I do the changes like that?

[flowvision]

I made the changes as discussed and rebased the branch on the new master because there was a merge conflict.

I'll just wait for the CI to see if the changes work on all platforms.

[kunaltyagi]

Squash & Merge pending CI 🚀 (1 commit is sufficient, 2 at max: adding new macro and replacement)

Thanks @office-florian-hubner 😄

[larshg]

Hi @office-florian-hubner

I had to change:
#if defined(__has_cpp_attribute) && __has_cpp_attribute(deprecated)
to
#if defined(__has_cpp_attribute) && __has_cpp_attribute(deprecated) && !defined(__CUDACC__)

or I'd still get an error:
attribute does not apply to any entity

regards, Lars

[kunaltyagi]

That's interesting. How does nvcc claim truth and yet throw an error? Let's add that patch with a comment related to nvcc's quirk

PS: any way to compile CUDA on windows on the CI?

[flowvision]

I added the check for CUDACC and added a comment.
For me it worked without it. Maybe it depends on the version of nvcc.

I squashed the commits so if the CI passes I think the branch can be integrated.

Thank you all for your feedback.

[kunaltyagi]

Pending CI