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

fix(r/adbcdrivermanager): Use std::vector<uint8_t> instead of std::basic_string<uint8_t> #1453

Merged
merged 2 commits into from
Jan 11, 2024

Conversation

paleolimbot
Copy link
Member

Because it makes more sense, but also with clang18, CRAN reports a warning:

In file included from driver_test.cc:24:
In file included from ./driver_base.h:19:
In file included from /usr/local/clang-trunk/bin/../include/c++/v1/memory:939:
In file included from /usr/local/clang-trunk/bin/../include/c++/v1/__memory/shared_ptr.h:21:
In file included from /usr/local/clang-trunk/bin/../include/c++/v1/__fwd/ostream.h:13:
/usr/local/clang-trunk/bin/../include/c++/v1/__fwd/string.h:45:41: warning: 'char_traits<unsigned char>' is deprecated: char_traits<T> for T not equal to char, wchar_t, char8_t, char16_t or char32_t is non-standard and is provided for a temporary period. It will be removed in LLVM 19, so please migrate off of it. [-Wdeprecated-declarations]
   45 | template <class _CharT, class _Traits = char_traits<_CharT>, class _Allocator = allocator<_CharT> >
      |                                         ^
./driver_base.h:168:20: note: in instantiation of default argument for 'basic_string<uint8_t>' required here
  168 |         const std::basic_string<uint8_t>& value = GetBytesUnsafe();
      |                    ^~~~~~~~~~~~~~~~~~~~~
/usr/local/clang-trunk/bin/../include/c++/v1/__string/char_traits.h:81:8: note: 'char_traits<unsigned char>' has been explicitly marked deprecated here
   81 | struct _LIBCPP_DEPRECATED_(
      |        ^
/usr/local/clang-trunk/bin/../include/c++/v1/__config:933:53: note: expanded from macro '_LIBCPP_DEPRECATED_'
  933 | #      define _LIBCPP_DEPRECATED_(m) __attribute__((__deprecated__(m)))
      |                                                     ^

@github-actions github-actions bot added this to the ADBC Libraries 0.10.0 milestone Jan 10, 2024
@paleolimbot paleolimbot marked this pull request as ready for review January 10, 2024 20:27
Comment on lines 269 to 270
std::vector<uint8_t> cppvalue(length);
memcpy(cppvalue.data(), value, length);
Copy link
Member

Choose a reason for hiding this comment

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

FWIW, I think it should work to write

std::vector<uint8_t> cppvalue(value, value + length);

(constructor (5) here https://en.cppreference.com/w/cpp/container/vector/vector)

Copy link
Member Author

Choose a reason for hiding this comment

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

That is way better (thank you!)

@paleolimbot paleolimbot merged commit c92f7de into apache:main Jan 11, 2024
21 of 22 checks passed
@paleolimbot paleolimbot deleted the r-test-driver-traits branch January 11, 2024 20:55
paleolimbot added a commit to paleolimbot/arrow-adbc that referenced this pull request Jan 11, 2024
…sic_string<uint8_t> (apache#1453)

Because it makes more sense, but also with clang18, CRAN reports a
warning:

```
In file included from driver_test.cc:24:
In file included from ./driver_base.h:19:
In file included from /usr/local/clang-trunk/bin/../include/c++/v1/memory:939:
In file included from /usr/local/clang-trunk/bin/../include/c++/v1/__memory/shared_ptr.h:21:
In file included from /usr/local/clang-trunk/bin/../include/c++/v1/__fwd/ostream.h:13:
/usr/local/clang-trunk/bin/../include/c++/v1/__fwd/string.h:45:41: warning: 'char_traits<unsigned char>' is deprecated: char_traits<T> for T not equal to char, wchar_t, char8_t, char16_t or char32_t is non-standard and is provided for a temporary period. It will be removed in LLVM 19, so please migrate off of it. [-Wdeprecated-declarations]
   45 | template <class _CharT, class _Traits = char_traits<_CharT>, class _Allocator = allocator<_CharT> >
      |                                         ^
./driver_base.h:168:20: note: in instantiation of default argument for 'basic_string<uint8_t>' required here
  168 |         const std::basic_string<uint8_t>& value = GetBytesUnsafe();
      |                    ^~~~~~~~~~~~~~~~~~~~~
/usr/local/clang-trunk/bin/../include/c++/v1/__string/char_traits.h:81:8: note: 'char_traits<unsigned char>' has been explicitly marked deprecated here
   81 | struct _LIBCPP_DEPRECATED_(
      |        ^
/usr/local/clang-trunk/bin/../include/c++/v1/__config:933:53: note: expanded from macro '_LIBCPP_DEPRECATED_'
  933 | #      define _LIBCPP_DEPRECATED_(m) __attribute__((__deprecated__(m)))
      |                                                     ^
```
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants