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

[R] r-arrow cannot be compiled with clang #43398

Closed
h-vetinari opened this issue Jul 23, 2024 · 9 comments
Closed

[R] r-arrow cannot be compiled with clang #43398

h-vetinari opened this issue Jul 23, 2024 · 9 comments

Comments

@h-vetinari
Copy link
Contributor

h-vetinari commented Jul 23, 2024

Describe the bug, including details regarding any error messages, version, and platform.

In conda-forge, we didn't have windows builds for R>4.1 for a while (needed large infrastructural changes), but now that it's sorted, we're having trouble getting the builds for r-arrow back on track. They were dropped completely for v15 & v16, and now, trying to re-add them runs into compilation errors:

In file included from altrep.cpp:18:
In file included from .\./arrow_types.h:22:
In file included from .\.\./arrow_cpp11.h:20:
In file included from C:\Program Files\Microsoft Visual Studio\2022\Enterprise\VC\Tools\MSVC\14.40.33807\include\memory:14:
In file included from C:\Program Files\Microsoft Visual Studio\2022\Enterprise\VC\Tools\MSVC\14.40.33807\include\xmemory:15:
C:\Program Files\Microsoft Visual Studio\2022\Enterprise\VC\Tools\MSVC\14.40.33807\include\xutility:1408:13: error: object of type 'cpp11::writable::r_vector<double>::iterator' cannot be assigned because its copy assignment operator is implicitly deleted
 1408 |         _It = _STD forward<_UIter>(_UIt);
      |             ^
C:\Program Files\Microsoft Visual Studio\2022\Enterprise\VC\Tools\MSVC\14.40.33807\include\algorithm:3556:10: note: in instantiation of function template specialization 'std::_Seek_wrapped<cpp11::writable::r_vector<double>::iterator, cpp11::writable::r_vector<double>::iterator &>' requested here
 3556 |     _STD _Seek_wrapped(_Dest, _UDest);
      |          ^
D:/bld/r-arrow_1721706669778/_h_env/lib/R/library/cpp11/include\cpp11/doubles.hpp:149:10: note: in instantiation of function template specialization 'std::transform<cpp11::r_vector<int>::const_iterator, cpp11::writable::r_vector<double>::iterator, (lambda at D:\bld\r-arrow_1721706669778\_h_env\lib\R\library\cpp11\include\cpp11\doubles.hpp:149:55)>' requested here
  149 |     std::transform(xn.begin(), xn.end(), ret.begin(), [](int value) {
      |          ^
D:/bld/r-arrow_1721706669778/_h_env/lib/R/library/cpp11/include\cpp11/r_vector.hpp:263:21: note: copy assignment operator of 'iterator' is implicitly deleted because field 'data_' is of reference type 'const r_vector<double> &'
  263 |     const r_vector& data_;
      |                     ^

This is with the combination of clang (more specifically r_clang_win-64) + MSVC's C++ STL.

Component(s)

Packaging, R

@assignUser
Copy link
Member

assignUser commented Jul 24, 2024

This must be an issue with cpp11 and the clang+msvc stl, the relevant code is untouched for 4 years and both our CI and CRAN builds with clang. So I don't quite understand why this error would only happen for you (afaik your clang is correct here). edit: A quick repro of the issue errors in both clang and gcc https://godbolt.org/z/oGPqc9rxr 🤷

I don't know if there is a way to solve this without patching cpp11

@h-vetinari
Copy link
Contributor Author

Thanks a lot for the quick response and the godbolt example. It seems this is failing even with the older compiler versions that we used. Perhaps an issue in https://github.com/r-lib/cpp11 is warranted?

I also ended up looking through the changes from r-cpp11 and couldn't find something that stood out to me. However, the last passing windows build we have in conda-forge used

    r-cpp11:                 0.4.3-r41hc72bb7e_0         conda-forge

Perhaps something regressed between then and 0.4.7? Can you check what version you're building against?

@assignUser
Copy link
Member

Hm yeah I don't see anything here either: r-lib/cpp11@ada0d7c...ff363d7#diff-65ceb5633686ca7a39f13ea1af039ba5b7bbc7bd225ca4eedbe181cf097ab0e6

Can you try the arrow build with that version pinned?

@h-vetinari
Copy link
Contributor Author

Looking at the diff, I think it's this change that is responsible, which landed in 0.4.4

@h-vetinari
Copy link
Contributor Author

Can you try the arrow build with that version pinned?

Not easily, but I'll try

@h-vetinari
Copy link
Contributor Author

So I don't quite understand why this error would only happen for you (afaik your clang is correct here).

Since this is happening in std::transform, it could be down to implementation details of the C++ standard library, i.e. perhaps libc++ and libstd++ can manage even if there's no copy assignment operator...? What C++ stdlib are the CRAN builds using?

@h-vetinari
Copy link
Contributor Author

Can confirm that things pass with r-cpp11 v0.4.3

@jakirkham
Copy link

jakirkham commented Jul 24, 2024

What C++ stdlib are the CRAN builds using?

Generally MinGW GCC compiler and MSYS2 build tools, which versions exactly depends on the version of R. Here is a full listing. For our case R 4.4 is most relevant, which uses GCC 13

Edit: Also some potentially useful (older) references: conda-forge/conda-forge.github.io#1654 (comment)

@assignUser
Copy link
Member

Looking at the diff, I think it's this change that is responsible, which landed in 0.4.4

Ok, as noted in the linked issue you are not going to use rtools for conda, so an issue on cpp11 seems to be the right recourse + patching it as a workaround?
In any case it's not an Arrow issue, so I am closing the issue but happy I could help :)

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

3 participants