paleolimbot commented on code in PR #14530:
URL: https://github.com/apache/arrow/pull/14530#discussion_r1009396026
##########
r/src/altrep.cpp:
##########
@@ -1098,9 +1098,9 @@ void test_arrow_altrep_set_string_elt(sexp x, int i,
std::string value) {
}
// [[arrow::export]]
-logicals test_arrow_altrep_is_materialized(sexp x) {
+sexp test_arrow_altrep_is_materialized(sexp x) {
Review Comment:
One of the things that has changed as cpp11 evolves is the constructor
methods...there's a lot of templating in the actual version but it's something
like:
```cpp
namespace writeable {
class logicals {
public:
logicals(); // creates an empty vector
logicals(R_xlen_t); // creates a vector with a length
logicals(int value); // maybe creates a vector with a literal value
};
}
```
Depending on exactly what type `R_xlen_t` resolves to and which compiler
you're on, the constructor that gets picked is at best unclear and at worst the
wrong one. In this case, something about the constructors meant that the
compiler couldn't figure out *any* suitable constructor to use. Instead of
trying to debut that, I just returned `Rf_ScalarLogical(value)` which is maybe
also a little more clear. That just returns a `SEXP`, so I changed the output
type to match (little case `sexp` is a C++-safe version of the `SEXP` that
makes sure it is not garbage collected as long as the `sexp` variable persists.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
