Handle ServiceDescriptor::FindMethodByName() taking std::string_view

[acozzette]

The ServiceDescriptor::FindMethodByName() method will hopefully soon accept std::string_view instead of const std::string&, so this commit updates RProtoBuf to be compatible with that change. I created a new macro RPB_XP_METHOD_CAST_1_STRING which explicitly converts to std::string before calling FindMethodByName().

[eddelbuettel]

Looks good in principle. And that should in 'both types' of environments, those absl types and those without?

[eddelbuettel]

And there goes Travis for the CI check. It's a fairly rigid (but reliable) setup where all we need is in the Docker container. If it failed it means that we may need something else...

[eddelbuettel]

Or it may be your code. If I try R CMD build in your branch it ends in tears on

[....other stuff omitted...]
In file included from wrapper_ServiceDescriptor.cpp:3:
wrapper_ServiceDescriptor.cpp: In function ‘SEXPREC* rprotobuf::ServiceDescriptor__getMethodByName(SEXP, SEXP)’:
RcppMacros.h:158:60: error: call of overloaded ‘basic_string(Rcpp::internal::converter)’ is ambiguous
  158 |             std::string( ::Rcpp::internal::converter( x0 ) ) ) ) ) ;         \
      |                                                            ^
wrapper_ServiceDescriptor.cpp:15:1: note: in expansion of macro ‘RPB_XP_METHOD_CAST_1_STRING’
   15 | RPB_XP_METHOD_CAST_1_STRING(METHOD(getMethodByName), GPB::ServiceDescriptor, FindMethodByName,
      | ^~~~~~~~~~~~~~~~~~~~~~~~~~~
In file included from /usr/include/c++/9/string:55,
                 from rprotobuf.h:28,
                 from wrapper_ServiceDescriptor.cpp:2:
/usr/include/c++/9/bits/basic_string.h:579:7: note: candidate: ‘std::__cxx11::basic_string<_CharT, _Traits, _Alloc>::basic_string(std::initializer_list<_Tp>, const _Alloc&) [with _CharT = char; _Traits = std::char_traits<char>; _Alloc = std::allocator<char>]’
  579 |       basic_string(initializer_list<_CharT> __l, const _Alloc& __a = _Alloc())
      |       ^~~~~~~~~~~~
/usr/include/c++/9/bits/basic_string.h:552:7: note: candidate: ‘std::__cxx11::basic_string<_CharT, _Traits, _Alloc>::basic_string(std::__cxx11::basic_string<_CharT, _Traits, _Alloc>&&) [with _CharT = char; _Traits = std::char_traits<char>; _Alloc = std::allocator<char>]’
  552 |       basic_string(basic_string&& __str) noexcept
      |       ^~~~~~~~~~~~
/usr/include/c++/9/bits/basic_string.h:525:7: note: candidate: ‘std::__cxx11::basic_string<_CharT, _Traits, _Alloc>::basic_string(const _CharT*, const _Alloc&) [with _CharT = char; _Traits = std::char_traits<char>; _Alloc = std::allocator<char>]’
  525 |       basic_string(const _CharT* __s, const _Alloc& __a = _Alloc())
      |       ^~~~~~~~~~~~
/usr/include/c++/9/bits/basic_string.h:448:7: note: candidate: ‘std::__cxx11::basic_string<_CharT, _Traits, _Alloc>::basic_string(const std::__cxx11::basic_string<_CharT, _Traits, _Alloc>&) [with _CharT = char; _Traits = std::char_traits<char>; _Alloc = std::allocator<char>]’
  448 |       basic_string(const basic_string& __str)
      |       ^~~~~~~~~~~~
/usr/include/c++/9/bits/basic_string.h:440:7: note: candidate: ‘std::__cxx11::basic_string<_CharT, _Traits, _Alloc>::basic_string(const _Alloc&) [with _CharT = char; _Traits = std::char_traits<char>; _Alloc = std::allocator<char>]’
  440 |       basic_string(const _Alloc& __a) _GLIBCXX_NOEXCEPT
      |       ^~~~~~~~~~~~
make: *** [/usr/lib/R/etc/Makeconf:176: wrapper_ServiceDescriptor.o] Error 1

Ubuntu 20.04, g++ 9.3

[acozzette]

Sorry about that, the build initially succeeded for me using Google's internal build system but I was able to reproduce the failure with R CMD build. I found that I could fix the error by static_casting to const std::string&.

[acozzette]

This should work with both absl::string_view and std::string_view.

[eddelbuettel]

It looks almost the same and as you force-pushed I can't tell what the second attempt does better, but hey, it works now :) Sorry about the pain of having to do in two environments; out here in the open we only have one.

[acozzette]

I just changed it from std::string( ... ) to static_cast<const std::string&>( ... ).

[eddelbuettel]

Thanks for the clarification. I think I can probably take this as is, especially as there aren't too many other packages riding on RProtoBuf.

[eddelbuettel]

Thanks again for the PR. I just marked it as 0.4.17.1 following the merge.

[acozzette]

Thanks, @eddelbuettel!