search

mpark / variant

C++17 `std::variant` for C++11/14/17
https://mpark.github.io/variant
Boost Software License 1.0
659 stars 88 forks source link

now compiles in nvcc 10.2 #73

Closed gridley closed 4 years ago

gridley commented 4 years ago

So, using some xtensor stuff which uses variant.hpp, I was trying to unable to compile stuff with NVCC 10.2 (latest version), and encountered some parsing error in nvcc that is described in this issue and this PR. It seems that part of the issue lies in xtensor-stack's xtl library and also this variant.hpp header file.

This PR just slightly changes an expression in variant.hpp so that the nvcc parser doesn't screw up. I think the code written here is valid beforehand, it's just that nvcc chokes on the parameter pack expansion somewhere.

The error which is encountered before this change, when including variant.hpp is:

gavin@gpad:~/scratch/variant/include/mpark$ nvcc nothing.cu 
variant.hpp: In function ‘constexpr decltype(auto) mpark::visit(Visitor&&, Vs&& ...)’:
variant.hpp:1967:96: error: parameter packs not expanded with ‘...’:
     return (detail::all({!vs.valueless_by_exception()...})
        ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~                                     ^                                                                                                      
variant.hpp:1967:96: note:         ‘vs’

No clue why this happens, but the changes here fix it.

mpark commented 4 years ago

What the heck...? Not sure why/how this got closed...

mpark commented 4 years ago

Oh~ it's because it's trying to merge into the dev branch, and I've been messing with the dev branch! @gridley Could you target this for master please?

gridley commented 4 years ago

Whoops, on it in a bit! My apologies!

gridley commented 4 years ago

@mpark OK, it's master now.

mpark commented 4 years ago

@gridley the changes seem much bigger than it was before. seems like there may have been formatting applied or something?

gridley commented 4 years ago

OK, so, maybe now this is ready once tests run.

mpark commented 4 years ago

Okay, I've fixed master and re-run the tests for this PR. @ax3l Could you confirm whether this works for you please?

ax3l commented 4 years ago

I am trying to confirm this fixes my issue as well, but have troubles since the HPC machine (x86) this was found with changed (broke) its software environment recently and I cannot reproduce the original issue on other machines yet (tried on power). Trying to setup another x86 machine/environment today...

ax3l commented 4 years ago

Ok, I was now able to spin up another env that can reproduce the problem I reported. Sorry for the delay that this caused. I can confirm the proposed fix in this PR works for me as well and we can close #71 in favour of it. Thanks a lot!

As a side note: I reported the NVCC compiler frontend issue in #70 to Nvidia too and it will be fixed in the next CUDA release as well (latest today: NVCC 10.2.89).