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

string proxy buglet #552

Closed
eddelbuettel opened this Issue Sep 8, 2016 · 7 comments

Comments

Projects
None yet
4 participants
@eddelbuettel
Member

eddelbuettel commented Sep 8, 2016

Assigning from a CharacterVector to a std::string fails:

R> cppFunction("std::string foo(CharacterVector V) { std::string s = V[0]; return s; }")
file1d288e26d1.cpp: In functionstd::__cxx11::string foo(Rcpp::CharacterVector)’:
file1d288e26d1.cpp:6:57: error: conversion fromRcpp::Vector<16>::Proxy \
  {aka Rcpp::internal::string_proxy<16>}’  to non-scalar typestd::__cxx11::string \
  {aka std::__cxx11::basic_string<char>}’ requested
 std::string foo(CharacterVector V) { std::string s = V[0]; return s; }
                                                         ^
make: *** [file1d288e26d1.o] Error 1
ccache g++ -I/usr/share/R/include -DNDEBUG    \
  -I"/usr/local/lib/R/site-library/Rcpp/include" \
  -I"/tmp/Rtmp2FcziP/sourceCpp-x86_64-pc-linux-gnu-0.12.7"    \
  -fpic  -g -O3 -Wall -pipe -Wno-unused -pedantic -c file1d288e26d1.cpp \
  -o file1d288e26d1.o
/usr/lib/R/etc/Makeconf:141: recipe for target 'file1d288e26d1.o' failed
Error in sourceCpp(code = code, env = env, rebuild = rebuild, cacheDir = cacheDir,  : 
  Error 1 occurred building shared library.
R>

where as this passes:

R> cppFunction("std::string foo(CharacterVector V) { std::string s(V[0]); return s; }")
R> 
@kevinushey

This comment has been minimized.

Show comment
Hide comment
@kevinushey

kevinushey Sep 8, 2016

Contributor

The clang error:

> cppFunction("std::string foo(CharacterVector V) { std::string s = V[0]; return s; }")
file154bf381b52f7.cpp:6:50: error: no viable conversion from 'Proxy' (aka 'string_proxy<16>') to 'std::string' (aka 'basic_string<char, char_traits<char>, allocator<char> >')
std::string foo(CharacterVector V) { std::string s = V[0]; return s; }
                                                 ^   ~~~~
/Users/kevin/clang/3.8.0/bin/../include/c++/v1/string:1373:5: note: candidate constructor not viable: no known conversion from 'Proxy' (aka 'string_proxy<16>') to 'const std::__1::basic_string<char> &' for 1st argument
    basic_string(const basic_string& __str);
    ^
/Users/kevin/clang/3.8.0/bin/../include/c++/v1/string:1388:31: note: candidate constructor not viable: no known conversion from 'Proxy' (aka 'string_proxy<16>') to 'const value_type *' (aka 'const char *') for 1st argument
    _LIBCPP_INLINE_VISIBILITY basic_string(const value_type* __s);
                              ^
/Users/kevin/Library/R/3.3/library/Rcpp/include/Rcpp/vector/string_proxy.h:109:3: note: candidate function
                operator SEXP() const {
                ^
/Users/kevin/Library/R/3.3/library/Rcpp/include/Rcpp/vector/string_proxy.h:118:4: note: candidate function
                 operator const char*() const {
                 ^
1 error generated.

I'm a little surprised that operator const char*() isn't selected here.

Contributor

kevinushey commented Sep 8, 2016

The clang error:

> cppFunction("std::string foo(CharacterVector V) { std::string s = V[0]; return s; }")
file154bf381b52f7.cpp:6:50: error: no viable conversion from 'Proxy' (aka 'string_proxy<16>') to 'std::string' (aka 'basic_string<char, char_traits<char>, allocator<char> >')
std::string foo(CharacterVector V) { std::string s = V[0]; return s; }
                                                 ^   ~~~~
/Users/kevin/clang/3.8.0/bin/../include/c++/v1/string:1373:5: note: candidate constructor not viable: no known conversion from 'Proxy' (aka 'string_proxy<16>') to 'const std::__1::basic_string<char> &' for 1st argument
    basic_string(const basic_string& __str);
    ^
/Users/kevin/clang/3.8.0/bin/../include/c++/v1/string:1388:31: note: candidate constructor not viable: no known conversion from 'Proxy' (aka 'string_proxy<16>') to 'const value_type *' (aka 'const char *') for 1st argument
    _LIBCPP_INLINE_VISIBILITY basic_string(const value_type* __s);
                              ^
/Users/kevin/Library/R/3.3/library/Rcpp/include/Rcpp/vector/string_proxy.h:109:3: note: candidate function
                operator SEXP() const {
                ^
/Users/kevin/Library/R/3.3/library/Rcpp/include/Rcpp/vector/string_proxy.h:118:4: note: candidate function
                 operator const char*() const {
                 ^
1 error generated.

I'm a little surprised that operator const char*() isn't selected here.

@eddelbuettel

This comment has been minimized.

Show comment
Hide comment
@eddelbuettel

eddelbuettel Sep 8, 2016

Member

It's actually via clang-3.8 that we found it (on OS X) as a colleague couldn't compile my budding anytime package.

Member

eddelbuettel commented Sep 8, 2016

It's actually via clang-3.8 that we found it (on OS X) as a colleague couldn't compile my budding anytime package.

@kevinushey

This comment has been minimized.

Show comment
Hide comment
@kevinushey

kevinushey Sep 8, 2016

Contributor

Attempting to add a std::string implicit conversion operator just causes a load of other compiler errors, so it seems like this one will be difficult / impossible to fix.

(I think the reason why const char* isn't used here is because it would involve more than one user-defined conversion in constructing the std::string object (string_proxy -> const char* -> std::string), which is prohibited by the C++ standards)

Contributor

kevinushey commented Sep 8, 2016

Attempting to add a std::string implicit conversion operator just causes a load of other compiler errors, so it seems like this one will be difficult / impossible to fix.

(I think the reason why const char* isn't used here is because it would involve more than one user-defined conversion in constructing the std::string object (string_proxy -> const char* -> std::string), which is prohibited by the C++ standards)

@thirdwing

This comment has been minimized.

Show comment
Hide comment
@thirdwing

thirdwing Sep 8, 2016

Member

I agree with @kevinushey

And this also works:

> cppFunction("std::string foo(CharacterVector V) { std::string s;s = V[0]; return s; }")
>
Member

thirdwing commented Sep 8, 2016

I agree with @kevinushey

And this also works:

> cppFunction("std::string foo(CharacterVector V) { std::string s;s = V[0]; return s; }")
>
@eddelbuettel

This comment has been minimized.

Show comment
Hide comment
@eddelbuettel

eddelbuettel Sep 8, 2016

Member

I wasn't arguing that we must fix it, I just thought I'd bring it up. I was surprised by as previously-compiling code suddenly stopped / stopped to compile on another platform.

What might be the best way to document it? Short new entry / paragraph in the FAQ? I think the whole topic of 'dis-entangle a complex assignment statement' might be worthy but I am not sure how to do it concisely and convincingly.

Member

eddelbuettel commented Sep 8, 2016

I wasn't arguing that we must fix it, I just thought I'd bring it up. I was surprised by as previously-compiling code suddenly stopped / stopped to compile on another platform.

What might be the best way to document it? Short new entry / paragraph in the FAQ? I think the whole topic of 'dis-entangle a complex assignment statement' might be worthy but I am not sure how to do it concisely and convincingly.

@coatless

This comment has been minimized.

Show comment
Hide comment
@coatless

coatless Mar 26, 2017

Contributor

Fifth entry in Section: Known Issues

Title: Issues with implicit conversion from an Rcpp object to a scalar or other Rcpp object

Text: Not all Rcpp expressions are directly compatible with \code{operator=}. Compability issues stem from many Rcpp objects and functions returning an intermediary result which requires an explicit conversion. In such cases, the user may need to assist the compiler with the conversion.

There are two ways to assist with the conversion. The first is to construct storage variable for a result, calculate the result, and then store a value into it. This is typically what is needed when working with \code{Character} and \code{String} in Rcpp due to the \code{Rcpp::internal::string_proxy} class. Within the following code snippet, the aforementioned approach is emphasized:

#include<Rcpp.h>
// [[Rcpp::export]]
std::string explicit_string_conv(Rcpp::CharacterVector X) { 
    std::string s;  // define storage
    s = X[0];       // assign from CharacterVector
    return s; 
}

If one were to use a direct allocation and assignment strategy, e.g. \code{std::string s = X[0]}, this would result in the compiler triggering a conversion error on \textit{some} platforms. The error would be similar to:

error: no viable conversion from 'Proxy' (aka 'string_proxy<16>') 
to 'std::string' (aka 'basic_string<char, char_traits<char>, allocator<char> >')

The second way to help the compiler is to use an explicit Rcpp type conversion function, if one were to exist. Examples of Rcpp type conversion functions include \code{as<T>()}, \code{.get()} for \code{cumsum()}, \code{is_true()} and \code{is_false()} for \code{any()} or \code{all()}.

Contributor

coatless commented Mar 26, 2017

Fifth entry in Section: Known Issues

Title: Issues with implicit conversion from an Rcpp object to a scalar or other Rcpp object

Text: Not all Rcpp expressions are directly compatible with \code{operator=}. Compability issues stem from many Rcpp objects and functions returning an intermediary result which requires an explicit conversion. In such cases, the user may need to assist the compiler with the conversion.

There are two ways to assist with the conversion. The first is to construct storage variable for a result, calculate the result, and then store a value into it. This is typically what is needed when working with \code{Character} and \code{String} in Rcpp due to the \code{Rcpp::internal::string_proxy} class. Within the following code snippet, the aforementioned approach is emphasized:

#include<Rcpp.h>
// [[Rcpp::export]]
std::string explicit_string_conv(Rcpp::CharacterVector X) { 
    std::string s;  // define storage
    s = X[0];       // assign from CharacterVector
    return s; 
}

If one were to use a direct allocation and assignment strategy, e.g. \code{std::string s = X[0]}, this would result in the compiler triggering a conversion error on \textit{some} platforms. The error would be similar to:

error: no viable conversion from 'Proxy' (aka 'string_proxy<16>') 
to 'std::string' (aka 'basic_string<char, char_traits<char>, allocator<char> >')

The second way to help the compiler is to use an explicit Rcpp type conversion function, if one were to exist. Examples of Rcpp type conversion functions include \code{as<T>()}, \code{.get()} for \code{cumsum()}, \code{is_true()} and \code{is_false()} for \code{any()} or \code{all()}.

@coatless

This comment has been minimized.

Show comment
Hide comment
@coatless

coatless Mar 31, 2017

Contributor

@eddelbuettel: This issue can now be closed.

Contributor

coatless commented Mar 31, 2017

@eddelbuettel: This issue can now be closed.

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