Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Compilation fix: Remove redefinition for std::is_same_v() #8369

Merged
merged 2 commits into from
May 27, 2021

Conversation

mythrocks
Copy link
Contributor

With nvcc-11.2 + gcc-10.2, one sees the following compilation errors in type_list_tests.cpp:

../tests/utilities_tests/type_list_tests.cpp:24:16: error: redefinition of ‘template<class T, class U> constexpr const bool std::is_same_v’
   24 | constexpr bool is_same_v = std::is_same<T, U>::value;
      |                ^~~~~~~~~
In file included from /usr/include/c++/10/bits/move.h:57,
                 from /usr/include/c++/10/bits/stl_pair.h:59,
                 from /usr/include/c++/10/bits/stl_algobase.h:64,
                 from /usr/include/c++/10/memory:63,
                 from _deps/gtest-src/googletest/include/gtest/internal/gtest-port.h:251,
                 from _deps/gtest-src/googletest/include/gtest/internal/gtest-type-util.h:47,
                 from ../include/cudf_test/cudf_gtest.hpp:42,
                 from ../tests/utilities_tests/type_list_tests.cpp:17:
/usr/include/c++/10/type_traits:3182:25: note: ‘template<class _Tp, class _Up> constexpr const bool std::is_same_v<_Tp, _Up>’ previously declared here
 3182 |   inline constexpr bool is_same_v = _GLIBCXX_BUILTIN_IS_SAME_AS(_Tp, _Up);
      |                         ^~~~~~~~~

I'm not sure why this doesn't turn up with gcc-9, even though we're using --std=c++17 on both.

This commit removes the redefinition of is_same_v().

@mythrocks mythrocks requested a review from a team as a code owner May 26, 2021 16:50
@github-actions github-actions bot added the libcudf Affects libcudf (C++/CUDA) code. label May 26, 2021
@mythrocks mythrocks added bug Something isn't working non-breaking Non-breaking change labels May 26, 2021
@mythrocks
Copy link
Contributor Author

mythrocks commented May 26, 2021

Failure seems transient, and unrelated to the change:

09:58:18 Setting commit status on GitHub for https://github.com/rapidsai/cudf/commit/fc324412838ac413f898f1b639885f8522769150
09:58:55 ERROR: Build step failed with exception
09:58:55 java.net.SocketTimeoutException: timeout
09:58:55 	at okhttp3.internal.http2.Http2Stream$StreamTimeout.newTimeoutException(Http2Stream.java:678)
09:58:55 	at okhttp3.internal.http2.Http2Stream$StreamTimeout.exitAndThrowIfTimedOut(Http2Stream.java:686)
09:58:55 	at okhttp3.internal.http2.Http2Stream.takeHeaders(Http2Stream.java:154)
09:58:55 	at okhttp3.internal.http2.Http2ExchangeCodec.readResponseHeaders(Http2ExchangeCodec.java:136)
09:58:55 	at okhttp3.internal.connection.Exchange.readResponseHeaders(Exchange.java:115)
09:58:55 	at okhttp3.internal.http.CallServerInterceptor.intercept(CallServerInterceptor.java:94)
...

Re-kicking... Will re-kick after higher priority bugs have been merged.

@mythrocks mythrocks self-assigned this May 26, 2021
Copy link
Contributor

@rgsl888prabhu rgsl888prabhu 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, update copyright.

@kkraus14 kkraus14 added the 5 - Ready to Merge Testing and reviews complete, ready to merge label May 26, 2021
@kkraus14
Copy link
Collaborator

@gpucibot merge

@kkraus14
Copy link
Collaborator

rerun tests

@mythrocks
Copy link
Contributor Author

Looks good, update copyright

Argh! Sorry I missed the copyright. Any objection to my pushing a fix to this?

@codecov
Copy link

codecov bot commented May 27, 2021

Codecov Report

❗ No coverage uploaded for pull request base (branch-21.06@cbbcba7). Click here to learn what that means.
The diff coverage is n/a.

Impacted file tree graph

@@               Coverage Diff               @@
##             branch-21.06    #8369   +/-   ##
===============================================
  Coverage                ?   82.86%           
===============================================
  Files                   ?      106           
  Lines                   ?    17874           
  Branches                ?        0           
===============================================
  Hits                    ?    14811           
  Misses                  ?     3063           
  Partials                ?        0           

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update cbbcba7...e629099. Read the comment docs.

@rapids-bot rapids-bot bot merged commit 50c0335 into rapidsai:branch-21.06 May 27, 2021
@mythrocks
Copy link
Contributor Author

Thanks for the reviews, all.

@mythrocks mythrocks deleted the remove-is_same_v branch May 27, 2021 03:13
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
5 - Ready to Merge Testing and reviews complete, ready to merge bug Something isn't working libcudf Affects libcudf (C++/CUDA) code. non-breaking Non-breaking change
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants