remove the need to derive from thrust::unary_function/thrust::binary_function when c++11 is enabled

[schiller-manuel]

see #624

[jaredhoberock]

You need to #include <type_traits> for std::result_of, right?

Also, since we're using C++11, we should probably implement result_of with a using template.

[schiller-manuel]

• changed to using.
• included <type_traits>, it was working without when not using nvcc, e.g. g++ and clang would compile without error.

Do we need a unit test?

[schiller-manuel]

regarding the unit tests: how can I enable -std=c++11 when building the unit tests?

[jaredhoberock]

I don't think the unit tests know about c++11. You would need to introduce a command line option to pass something like -std=c++11 to the compiler.

There are dictionaries which map build options to compiler flags here:

https://github.com/thrust/thrust/blob/master/SConstruct#L28

I suggest introducing a new SCons ListVariable named std with the options c++03 and c++11.

[jaredhoberock]

I couldn't find any unit tests for unary_function or binary_function, so I think it's OK if there is no particular test for this feature. It would be a good idea to document that thrust::unary_function and thrust::binary_function are optional in C++11, though.

Regarding using -- now that we are not guarding its use with __cplusplus, I wonder if the implementation of result_of should be changed back to your original implementation. What do you think?

[schiller-manuel]

regarding using:

Do you think any compiler which defines __cpp_lib_result_of_sfinae would not support type aliases through using? I can hardly imagine this. But having a unit test then would make sense, if unit tests are run on all supported compilers. What is your usual workflow regarding the execution of unit tests? Do you run on Linux, Mac OSX, Windows and different compilers? Is there a list of supported compilers?

If we added a unit test for this, how would it be activated? Through that yet to be defined SCons ListVariable or using the same __cplusplus >= 201103L || defined(__cpp_lib_result_of_sfinae) check done in code?

[schiller-manuel]

Regarding documentation: Where should that hint go?

1. http://thrust.github.io/doc/classthrust_1_1permutation__iterator.html
2. http://thrust.github.io/doc/structthrust_1_1binary__function.html and http://thrust.github.io/doc/structthrust_1_1unary__function.html

?

[jaredhoberock]

It's difficult to predict the behavior of (buggy) compilers, so Thrust typically needs to be programmed as defensively as possible. Since it is a one-line change to change it back to a C++03-style typedef, we may as well do it. I note that it is not impossible for an enterprising user who knows about feature test macros to define that macro as well as <type_traits> herself, independently of a compiler.

Unit tests are run on a cross product of supported OSes and hardwares. Each particular OS version has a compiler version which is supported:

http://docs.nvidia.com/cuda/cuda-getting-started-guide-for-linux/index.html#system-requirements
http://docs.nvidia.com/cuda/cuda-getting-started-guide-for-mac-os-x/index.html#system-requirements
http://docs.nvidia.com/cuda/cuda-getting-started-guide-for-microsoft-windows/index.html#system-requirements

If we added a unit test for this feature, I would simply guard it with __cplusplus >= 201103L. However, because there are so many unit tests which are very expensive to build and execute, I do not think it is worth introducing a new one for what is a small quality of life feature.

For documentation, I think it would be sufficient to add a new note entry to unary_function and binary_function which indicates that they are optional with C++11. At the same time, I would eliminate the current notes discussing redundancy with std::unary_function and std::binary_function.

[schiller-manuel]

Would you still like the possibility to enable c++11 for the existing unit tests through the scons variable?

[jaredhoberock]

Finally, please add a new entry in the CHANGELOG for version 1.9.0. You can simply copy from 1.8.0 and fill in the sections with TODO. This feature should be mentioned under the "Other Enhancements" section. Also acknowledge yourself for a job well done!

Once we have some confidence that this change works, I'll merge it. Because the change is small, simply building and running the example programs on your system should be sufficient. I will do the same on my system (Ubuntu 14.04).

[jaredhoberock]

Yes, in order to test the example programs with this feature, it seems necessary to enable C++11 in the build.

In case it's not obvious, you can build & run the example programs automatically with

$ scons run_examples

[schiller-manuel]

You might add this command to the newly created wiki page.

[jaredhoberock]

I've added a section for building & running the example programs.

[schiller-manuel]

Could you please have a look at SConstruct? How can I avoid the warnings

"cc1: warning: command line option '-std=c++11' is valid for C++/ObjC++ but not for C [enabled by default]" ?

[schiller-manuel]

when not appending -std=c++11 to CCFLAGS I don't get the warnings. Is the change to SConstruct sufficient?

[jaredhoberock]

No, I don't believe the change is sufficient. With your current changes, the C++11 build won't work unless nvcc is used as the compiler.

I think the warnings were probably due to -Xcompiler -std=c++11 being passed to nvcc. Since nvcc supports -std=c++11 directly, this option doesn't need to be wrapped with -Xcompiler.

Perhaps there's a way to remove -std=c++11 from CCFLAGS when nvcc is used.

[schiller-manuel]

I thought nvcc is mandatory when compiling the tests/examples, regardless which host and device system is chosen? If this is assumption is wrong, then the information in the Developer-Information wiki page I wrote is also incorrect:

In order to run the tests, CUDA must be installed on your system because nvcc is used as the compiler driver.

[schiller-manuel]

Unit tests passed on my computer (Ubuntu 14.04) with std=c++11:

================================================================                                                                                                                                                                  |
KNOWN FAILURE: TestDeviceDeleteDestructorInvocation
[testing/device_delete.cu:29]
================================================================
KNOWN FAILURE: TestScanWithLargeTypes
[testing/scan.cu:504]
================================================================
Totals: 0 failures, 2 known failures, 0 errors, and 1374 passes.

Examples compiled and run after I fixed cuda/async_reduce.cu.

[jaredhoberock]

I edited that part of the wiki page to indicate that that comment only applies to the CUDA backend. Some of the source files used in the build have the extension .cpp. These are compiled with a normal C++ compiler, i.e., g++ or cl.exe. So the C++ compiler will also need to know about the std option.

[schiller-manuel]

warnings do not occur anymore because -std=c++11 is only set if not compiling via nvcc. Anything else to add/change?

[jaredhoberock]

C+11 -> C++11

I think I would change the note to say something like:

"Because C++11 language support makes the functionality of \c unary_function obsolete, it's use is optional if C++11 language features are enabled."

Note that the \c makes Doxygen print the word in typewriter font.

[jaredhoberock]

Thanks Manuel, your changes look good to me. I found a couple of typos in thrust/functional.h and suggested a tweak to the note.

If you make those last tweaks, I will test your changes with MSVC and merge them if the tests pass on my system.

[jaredhoberock]

Looks good, thanks for the hard work, Manuel!