Skip to content
This repository has been archived by the owner on Mar 21, 2024. It is now read-only.

Deprecate C++03, C++11, MSVC < 2017, GCC < 5.0 #1089

Merged

Conversation

alliepiper
Copy link
Collaborator

@alliepiper alliepiper commented Apr 3, 2020

Deprecate everything before C++14, MSVC 2017, and GCC 5.0.

Includes many fixes to get things building on MSVC and assorted config consistency / modernization updates. See commit message for full details.

griwes
griwes previously requested changes Apr 4, 2020
Copy link
Collaborator

@griwes griwes left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Please rename that one macro with a problematic name and this will be good to go.

(...I realize that there are already some reserved names used in Thrust, we should probably get rid of those one day also.)

thrust/detail/config/cpp_dialect.h Outdated Show resolved Hide resolved
thrust/detail/config/exec_check_disable.h Show resolved Hide resolved
@alliepiper alliepiper force-pushed the feature/static_config_consistency branch from e67ca5f to 4c6c8aa Compare April 4, 2020 14:32
@alliepiper
Copy link
Collaborator Author

Renamed the macro and made the same change in cub as part of thrust/cub#21.

@alliepiper alliepiper requested a review from griwes April 4, 2020 15:03
@alliepiper
Copy link
Collaborator Author

New shelve 28219812

Copy link
Collaborator

@brycelelbach brycelelbach left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Looks good once the comments are addressed; the only changes I've asked for are minor.

internal/benchmark/bench.cu Outdated Show resolved Hide resolved
testing/mr_pool.cu Show resolved Hide resolved
thrust/detail/complex/catrig.h Outdated Show resolved Hide resolved
thrust/detail/complex/catrig.h Outdated Show resolved Hide resolved
thrust/detail/complex/catrig.h Outdated Show resolved Hide resolved
thrust/detail/complex/catrigf.h Outdated Show resolved Hide resolved
thrust/detail/complex/catrigf.h Outdated Show resolved Hide resolved
thrust/detail/config/namespace.h Outdated Show resolved Hide resolved
thrust/detail/contiguous_storage.h Outdated Show resolved Hide resolved
thrust/detail/preprocessor.h Outdated Show resolved Hide resolved
@alliepiper alliepiper force-pushed the feature/static_config_consistency branch from 4c6c8aa to e9d521e Compare April 6, 2020 21:30
@alliepiper
Copy link
Collaborator Author

New shelve 28221412

@alliepiper alliepiper force-pushed the feature/static_config_consistency branch from e9d521e to f18ef8b Compare April 8, 2020 21:11
@alliepiper
Copy link
Collaborator Author

Updated with a ton of changes that get this to build on MSVC2015.

A commit with just the new changes can be found here: alliepiper@1c9440e

Relaunched CI under shelve 28231909.

@alliepiper alliepiper force-pushed the feature/static_config_consistency branch from f18ef8b to 149ef5a Compare April 9, 2020 00:09
@alliepiper alliepiper force-pushed the feature/static_config_consistency branch from 149ef5a to e58d51f Compare April 9, 2020 00:17
@alliepiper alliepiper dismissed griwes’s stale review April 9, 2020 00:19

Addressed issue (removed __ from identifier)

@alliepiper
Copy link
Collaborator Author

Merged with the -Werror and deprecation PRs to check for incompatible CI builds. New shelve is 28232673.

@alliepiper alliepiper changed the title Build infrastructure and static configuration fixes. Deprecate C++03, C++11, MSVC < 2015u3 Apr 9, 2020
@alliepiper alliepiper force-pushed the feature/static_config_consistency branch from e58d51f to c5504c3 Compare April 9, 2020 18:39
@brycelelbach
Copy link
Collaborator

brycelelbach commented Apr 9, 2020

It turns out that MSVC 2015 lies about being C++14 compliant. It doesn't have extended constexpr, but _MSVC_LANG does report C++14.

So what do we want to do about this?

  • Use "C++14 minus extended constexpr" as our base
  • Deprecate MSVC 2015 as well and fix THRUST_CPP_DIALECT to report MSVC as C++11 instead of C++14

@alliepiper
Copy link
Collaborator Author

I vote for the second option (and am currently testing MSVC 2017 and 2019).

@alliepiper alliepiper force-pushed the feature/static_config_consistency branch 2 times, most recently from b4ec71e to 1c0452b Compare April 11, 2020 01:30
@alliepiper alliepiper changed the title Deprecate C++03, C++11, MSVC < 2015u3 Deprecate C++03, C++11, MSVC < 2017, GCC < 5.0 Apr 11, 2020
@alliepiper
Copy link
Collaborator Author

This last push updates the deprecation checks to give an #error for C++ < 14, MSVC < 2015, Clang < 6.0, GCC < 5.0. These can all be ignored by defining THRUST_IGNORE_DEPRECATED_CPP_DIALECT and CUB_IGNORE_DEPRECATED_CPP_DIALECT.

This patch also gets everything building on MSVC 2017 and 2019 using C++14 with the async algorithms enabled, and all tests pass (with the exception of #1098, that still needs to be tracked down).

New shelve: 28243163

@alliepiper alliepiper force-pushed the feature/static_config_consistency branch 3 times, most recently from 786457e to 2cc6847 Compare April 15, 2020 19:08
@alliepiper alliepiper force-pushed the feature/static_config_consistency branch 6 times, most recently from b756453 to 72e7f5e Compare April 18, 2020 14:52
thrust/detail/config/cpp_dialect.h Outdated Show resolved Hide resolved
thrust/detail/config/cpp_dialect.h Outdated Show resolved Hide resolved
thrust/detail/config/cpp_dialect.h Outdated Show resolved Hide resolved
thrust/detail/config/cpp_dialect.h Outdated Show resolved Hide resolved
testing/alignment.cu Outdated Show resolved Hide resolved
thrust/detail/complex/catrig.h Show resolved Hide resolved
thrust/detail/execute_with_dependencies.h Show resolved Hide resolved
@brycelelbach
Copy link
Collaborator

Make sure to put the "Reviewed-by:" line in this commit (one for each reviewer).

@brycelelbach brycelelbach added this to the 1.9.9 milestone Apr 20, 2020
@alliepiper alliepiper force-pushed the feature/static_config_consistency branch from 72e7f5e to 086193d Compare April 20, 2020 18:30
Build infrastructure and static configuration fixes:

- Bump CMAKE_CXX_STANDARD to 14
- Add `-Werror all-warnings` to NVCC to promote warnings to errors
- Add `-Xcudafe --display_error_number` to get useful diagnositics from
    cudafe.
- Clean up cub include dir spec in CMake.
- Move THRUST_DEPRECATED logic out of compiler.h and into new header.
- Fix CPP dialect detection on newer MSVC.
- Remove raw `__cplusplus` checks.
- Use `_Pragma`/`__pragma` instead of `#pragma` in macro.
- Remove THRUST_BEGIN/END_NS macros.
  - These were used inconsistently, rendering them non-functional. Removing
    to prevent people from trying to use them.

Workarounds for msvc:

- MSVC isn't a fan of `decltype(...)::some_member` syntax.
  - WAR by aliasing the `decltype(...)` and doing `NewAlias::some_member`
- Missing `template` keyword when rebinding pointer in `async/reduce.h`
- Silence warning C4494 `declspec(allocator) used on non-pointer/ref type`
  - Bug in MSVC STL: microsoft/STL#696
- Disable async sort test on MSVC
  - Triage. Looks like a bug in cudafe? See NVIDIA#1098.
- Add pointer<T>::pointer_to(reference)
  - Required for C++11, hard compile error on MSVC.
- Bring a definition of `atanh` into scope for complex number impl
- Fix floating point literals be declared as floats instead of doubles
- Replace `std::remove_reference<T>::type&` with
  `std::add_lvalue_reference`.
  - Same behavior, and MSVC chokes on the other syntax when followed by
    `__host__`.
- Remove constexpr markup from defaulted functions.
  - These are constexpr by default when possible, and the compilers were
    complaining about the markup in places.
- Use `thrust::detail::integer_traits` instead of `std::numeric_limits`
  in device code.
- Avoid aligning beyond platform limits in alignment.cu.
- Pass /bigobj to MSVC so it can handle the async tests
- Work around MSVC compiler bug by replacing SFINAE with static dispatch

Bug 2865172
Bug 2880936

Reviewed-by:  Bryce Adelstein Lelbach aka wash <[email protected]>
Reviewed-by: Michał 'Griwes' Dominiak <[email protected]>
@alliepiper alliepiper force-pushed the feature/static_config_consistency branch from 086193d to 66d0110 Compare April 20, 2020 19:49
@alliepiper alliepiper merged commit 372dea5 into NVIDIA:master Apr 20, 2020
@alliepiper alliepiper deleted the feature/static_config_consistency branch April 20, 2020 20:09
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants