You are viewing a plain text version of this content. The canonical link for it is here.
Posted to github@arrow.apache.org by GitBox <gi...@apache.org> on 2022/10/27 12:29:07 UTC

[GitHub] [arrow] paleolimbot opened a new pull request, #14530: ARROW-18174: [R] Fix compile of altrep.cpp on some builds

paleolimbot opened a new pull request, #14530:
URL: https://github.com/apache/arrow/pull/14530

   After ARROW-17187 (#14271), we get at least two nightly build failures because of how cpp11 objects are constructed in some new test functions. This PR makes the constructors less ambiguous so that no compilers give us trouble!


-- 
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: github-unsubscribe@arrow.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


[GitHub] [arrow] github-actions[bot] commented on pull request #14530: ARROW-18174: [R] Fix compile of altrep.cpp on some builds

Posted by GitBox <gi...@apache.org>.
github-actions[bot] commented on PR #14530:
URL: https://github.com/apache/arrow/pull/14530#issuecomment-1295188405

   Revision: fc514d0ead58f34dfc609873017559f7db913d2d
   
   Submitted crossbow builds: [ursacomputing/crossbow @ actions-c5c1f0e15d](https://github.com/ursacomputing/crossbow/branches/all?query=actions-c5c1f0e15d)
   
   |Task|Status|
   |----|------|
   |homebrew-r-autobrew|[![Github Actions](https://github.com/ursacomputing/crossbow/workflows/Crossbow/badge.svg?branch=actions-c5c1f0e15d-github-homebrew-r-autobrew)](https://github.com/ursacomputing/crossbow/actions/runs/3346907349/jobs/5544287323)|
   |test-r-linux-valgrind|[![Azure](https://dev.azure.com/ursacomputing/crossbow/_apis/build/status/ursacomputing.crossbow?branchName=actions-c5c1f0e15d-azure-test-r-linux-valgrind)](https://github.com/ursacomputing/crossbow/runs/9168267553)|


-- 
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: github-unsubscribe@arrow.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


[GitHub] [arrow] paleolimbot commented on pull request #14530: ARROW-18174: [R] Fix compile of altrep.cpp on some builds

Posted by GitBox <gi...@apache.org>.
paleolimbot commented on PR #14530:
URL: https://github.com/apache/arrow/pull/14530#issuecomment-1328355616

   Done! See ARROW-18412


-- 
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: github-unsubscribe@arrow.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


[GitHub] [arrow] paleolimbot commented on a diff in pull request #14530: ARROW-18174: [R] Fix compile of altrep.cpp on some builds

Posted by GitBox <gi...@apache.org>.
paleolimbot commented on code in PR #14530:
URL: https://github.com/apache/arrow/pull/14530#discussion_r1008599486


##########
r/src/compute-exec.cpp:
##########
@@ -175,8 +175,8 @@ class ExecPlanReader : public arrow::RecordBatchReader {
     }
 
     plan_status_ = PLAN_FINISHED;
-    plan_.reset();
-    sink_gen_ = arrow::MakeEmptyGenerator<std::optional<compute::ExecBatch>>();
+    // plan_.reset();

Review Comment:
   Ok - I added a note here since this does seem to fix things (and because I can't think of a reason why keeping a reference to a completed or cancelled plan would be a problem).



-- 
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: github-unsubscribe@arrow.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


[GitHub] [arrow] paleolimbot commented on a diff in pull request #14530: ARROW-18174: [R] Fix compile of altrep.cpp on some builds

Posted by GitBox <gi...@apache.org>.
paleolimbot commented on code in PR #14530:
URL: https://github.com/apache/arrow/pull/14530#discussion_r1008178156


##########
r/src/compute-exec.cpp:
##########
@@ -175,8 +175,8 @@ class ExecPlanReader : public arrow::RecordBatchReader {
     }
 
     plan_status_ = PLAN_FINISHED;
-    plan_.reset();
-    sink_gen_ = arrow::MakeEmptyGenerator<std::optional<compute::ExecBatch>>();
+    // plan_.reset();

Review Comment:
   It's a temporary change to figure out if this is, indeed, what fixes one or more of the `head()` related failures! (It's looking promising so far although I'd like to understand why...)



-- 
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: github-unsubscribe@arrow.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


[GitHub] [arrow] paleolimbot commented on a diff in pull request #14530: ARROW-18174: [R] Fix compile of altrep.cpp on some builds

Posted by GitBox <gi...@apache.org>.
paleolimbot commented on code in PR #14530:
URL: https://github.com/apache/arrow/pull/14530#discussion_r1009409907


##########
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:
   Yes...pretty much always use `sexp` because it's safer. `SEXP` gets used a lot in our code base too...it can be safe to use that in C++ as well but you have to make sure the object is not going to get garbage collected (either by using `PROTECT()` or by making absolutely sure that it's something that's already protected by virtue of its container). The problem is, whenever you write `SEXP`, put it on the reader of your code to figure that out.



-- 
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: github-unsubscribe@arrow.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


[GitHub] [arrow] paleolimbot commented on pull request #14530: ARROW-18174: [R] Fix compile of altrep.cpp on some builds

Posted by GitBox <gi...@apache.org>.
paleolimbot commented on PR #14530:
URL: https://github.com/apache/arrow/pull/14530#issuecomment-1293456437

   @github-actions crossbow submit test-r-gcc-12 test-r-rhub-ubuntu-gcc-release-latest


-- 
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: github-unsubscribe@arrow.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


[GitHub] [arrow] github-actions[bot] commented on pull request #14530: ARROW-18174: [R] Fix compile of altrep.cpp on some builds

Posted by GitBox <gi...@apache.org>.
github-actions[bot] commented on PR #14530:
URL: https://github.com/apache/arrow/pull/14530#issuecomment-1328326268

   Revision: ec2452d911aff2df2cacdc1db890f9d929c1b845
   
   Submitted crossbow builds: [ursacomputing/crossbow @ actions-89486fcd3f](https://github.com/ursacomputing/crossbow/branches/all?query=actions-89486fcd3f)
   
   |Task|Status|
   |----|------|
   |test-r-devdocs|[![Github Actions](https://github.com/ursacomputing/crossbow/workflows/Crossbow/badge.svg?branch=actions-89486fcd3f-github-test-r-devdocs)](https://github.com/ursacomputing/crossbow/actions/runs/3559717769/jobs/5979255297)|


-- 
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: github-unsubscribe@arrow.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


[GitHub] [arrow] github-actions[bot] commented on pull request #14530: ARROW-18174: [R] Fix compile of altrep.cpp on some builds

Posted by GitBox <gi...@apache.org>.
github-actions[bot] commented on PR #14530:
URL: https://github.com/apache/arrow/pull/14530#issuecomment-1293512665

   Revision: 1e4de75dc08bd960a37ab152572d00e1ba977325
   
   Submitted crossbow builds: [ursacomputing/crossbow @ actions-034ae4ff37](https://github.com/ursacomputing/crossbow/branches/all?query=actions-034ae4ff37)
   
   |Task|Status|
   |----|------|
   |test-r-gcc-12|[![Github Actions](https://github.com/ursacomputing/crossbow/workflows/Crossbow/badge.svg?branch=actions-034ae4ff37-github-test-r-gcc-12)](https://github.com/ursacomputing/crossbow/actions/runs/3337713430/jobs/5524409577)|
   |test-r-rhub-ubuntu-gcc-release-latest|[![Azure](https://dev.azure.com/ursacomputing/crossbow/_apis/build/status/ursacomputing.crossbow?branchName=actions-034ae4ff37-azure-test-r-rhub-ubuntu-gcc-release-latest)](https://github.com/ursacomputing/crossbow/runs/9141789709)|


-- 
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: github-unsubscribe@arrow.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


[GitHub] [arrow] github-actions[bot] commented on pull request #14530: ARROW-18174: [R] Fix compile of altrep.cpp on some builds

Posted by GitBox <gi...@apache.org>.
github-actions[bot] commented on PR #14530:
URL: https://github.com/apache/arrow/pull/14530#issuecomment-1293503418

   :warning: Ticket **has not been started in JIRA**, please click 'Start Progress'.


-- 
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: github-unsubscribe@arrow.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


[GitHub] [arrow] paleolimbot commented on pull request #14530: ARROW-18174: [R] Fix compile of altrep.cpp on some builds

Posted by GitBox <gi...@apache.org>.
paleolimbot commented on PR #14530:
URL: https://github.com/apache/arrow/pull/14530#issuecomment-1295091354

   @github-actions crossbow submit test-r-linux-valgrind homebrew-r-autobrew


-- 
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: github-unsubscribe@arrow.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


[GitHub] [arrow] thisisnic commented on a diff in pull request #14530: ARROW-18174: [R] Fix compile of altrep.cpp on some builds

Posted by GitBox <gi...@apache.org>.
thisisnic commented on code in PR #14530:
URL: https://github.com/apache/arrow/pull/14530#discussion_r1009402154


##########
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:
   Is there generally a heuristic you use for when to use `SEXP` versus the cpp11 version of the variable, and vice versa?



-- 
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: github-unsubscribe@arrow.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


[GitHub] [arrow] thisisnic merged pull request #14530: ARROW-18174: [R] Fix compile of altrep.cpp on some builds

Posted by GitBox <gi...@apache.org>.
thisisnic merged PR #14530:
URL: https://github.com/apache/arrow/pull/14530


-- 
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: github-unsubscribe@arrow.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


[GitHub] [arrow] kou commented on pull request #14530: ARROW-18174: [R] Fix compile of altrep.cpp on some builds

Posted by GitBox <gi...@apache.org>.
kou commented on PR #14530:
URL: https://github.com/apache/arrow/pull/14530#issuecomment-1328341447

   @paleolimbot Could you open an issue for https://github.com/ursacomputing/crossbow/actions/runs/3559717769/jobs/5979255297#step:9:2818 ?
   
   ```text
   C:/rtools40/mingw64/bin/g++ -shared -s -static-libgcc -o arrow.dll tmp.def RTasks.o altrep.o array.o array_to_vector.o arraydata.o arrowExports.o bridge.o buffer.o chunkedarray.o compression.o compute-exec.o compute.o config.o csv.o dataset.o datatype.o expression.o extension-impl.o feather.o field.o filesystem.o imports.o io.o json.o memorypool.o message.o parquet.o r_to_arrow.o recordbatch.o recordbatchreader.o recordbatchwriter.o safe-call-into-r-impl.o scalar.o schema.o symbols.o table.o threadpool.o type_infer.o -LD:/a/crossbow/crossbow/dist/lib -LC:/rtools40/mingw64/lib -larrow_dataset -lparquet -larrow C:/rtools40/mingw64/lib/libre2.a -lthrift -lz -LC:/R/bin/x64 -lR
   C:/rtools40/mingw64/bin/../lib/gcc/x86_64-w64-mingw32/8.3.0/../../../../x86_64-w64-mingw32/bin/ld.exe: altrep.o:altrep.cpp:(.text+0x544d): undefined reference to `arrow::internal::ChunkResolver::ChunkResolver(std::vector<std::shared_ptr<arrow::Array>, std::allocator<std::shared_ptr<arrow::Array> > > const&)'
   C:/rtools40/mingw64/bin/../lib/gcc/x86_64-w64-mingw32/8.3.0/../../../../x86_64-w64-mingw32/bin/ld.exe: altrep.o:altrep.cpp:(.text+0x5898): undefined reference to `arrow::internal::ChunkResolver::ChunkResolver(std::vector<std::shared_ptr<arrow::Array>, std::allocator<std::shared_ptr<arrow::Array> > > const&)'
   C:/rtools40/mingw64/bin/../lib/gcc/x86_64-w64-mingw32/8.3.0/../../../../x86_64-w64-mingw32/bin/ld.exe: altrep.o:altrep.cpp:(.text+0x599c): undefined reference to `arrow::internal::ChunkResolver::ChunkResolver(std::vector<std::shared_ptr<arrow::Array>, std::allocator<std::shared_ptr<arrow::Array> > > const&)'
   C:/rtools40/mingw64/bin/../lib/gcc/x86_64-w64-mingw32/8.3.0/../../../../x86_64-w64-mingw32/bin/ld.exe: altrep.o:altrep.cpp:(.text+0x5a5b): undefined reference to `arrow::internal::ChunkResolver::ChunkResolver(std::vector<std::shared_ptr<arrow::Array>, std::allocator<std::shared_ptr<arrow::Array> > > const&)'
   C:/rtools40/mingw64/bin/../lib/gcc/x86_64-w64-mingw32/8.3.0/../../../../x86_64-w64-mingw32/bin/ld.exe: altrep.o:altrep.cpp:(.text+0x5ad7): undefined reference to `arrow::internal::ChunkResolver::ChunkResolver(std::vector<std::shared_ptr<arrow::Array>, std::allocator<std::shared_ptr<arrow::Array> > > const&)'
   collect2.exe: error: ld returned 1 exit status
   ```
   
   The following patch may fix the error:
   
   ```diff
   diff --git a/cpp/src/arrow/chunk_resolver.h b/cpp/src/arrow/chunk_resolver.h
   index 1a63d26c24..818070ffe3 100644
   --- a/cpp/src/arrow/chunk_resolver.h
   +++ b/cpp/src/arrow/chunk_resolver.h
   @@ -32,7 +32,7 @@ struct ChunkLocation {
    };
    
    // An object that resolves an array chunk depending on a logical index
   -struct ChunkResolver {
   +struct ARROW_EXPORT ChunkResolver {
      explicit ChunkResolver(const ArrayVector& chunks);
    
      explicit ChunkResolver(const std::vector<const Array*>& chunks);
   ```


-- 
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: github-unsubscribe@arrow.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


[GitHub] [arrow] paleolimbot commented on a diff in pull request #14530: ARROW-18174: [R] Fix compile of altrep.cpp on some builds

Posted by GitBox <gi...@apache.org>.
paleolimbot commented on code in PR #14530:
URL: https://github.com/apache/arrow/pull/14530#discussion_r1009416238


##########
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:
   (In case it's helpful: https://github.com/r-lib/cpp11/blob/main/inst/include/cpp11/sexp.hpp#L14-L58 )



-- 
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: github-unsubscribe@arrow.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


[GitHub] [arrow] nealrichardson commented on a diff in pull request #14530: ARROW-18174: [R] Fix compile of altrep.cpp on some builds

Posted by GitBox <gi...@apache.org>.
nealrichardson commented on code in PR #14530:
URL: https://github.com/apache/arrow/pull/14530#discussion_r1008174775


##########
r/src/compute-exec.cpp:
##########
@@ -175,8 +175,8 @@ class ExecPlanReader : public arrow::RecordBatchReader {
     }
 
     plan_status_ = PLAN_FINISHED;
-    plan_.reset();
-    sink_gen_ = arrow::MakeEmptyGenerator<std::optional<compute::ExecBatch>>();
+    // plan_.reset();

Review Comment:
   Is this change intentional? If so, delete the code? Add a TODO comment?



-- 
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: github-unsubscribe@arrow.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


[GitHub] [arrow] thisisnic commented on a diff in pull request #14530: ARROW-18174: [R] Fix compile of altrep.cpp on some builds

Posted by GitBox <gi...@apache.org>.
thisisnic commented on code in PR #14530:
URL: https://github.com/apache/arrow/pull/14530#discussion_r1009384870


##########
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:
   For my own learning, would be interested to hear why changing this from `logicals` to `sexp` mattered, and same with the return value.



-- 
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: github-unsubscribe@arrow.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


[GitHub] [arrow] paleolimbot commented on pull request #14530: ARROW-18174: [R] Fix compile of altrep.cpp on some builds

Posted by GitBox <gi...@apache.org>.
paleolimbot commented on PR #14530:
URL: https://github.com/apache/arrow/pull/14530#issuecomment-1329850642

   I can give it a go tomorrow (but all I know how to do here is copy/paste what you suggested above!)


-- 
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: github-unsubscribe@arrow.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


[GitHub] [arrow] github-actions[bot] commented on pull request #14530: ARROW-18174: [R] Fix compile of altrep.cpp on some builds

Posted by GitBox <gi...@apache.org>.
github-actions[bot] commented on PR #14530:
URL: https://github.com/apache/arrow/pull/14530#issuecomment-1294249146

   Revision: 8b2ae883d2a425b6a2528350435d6797220f7c51
   
   Submitted crossbow builds: [ursacomputing/crossbow @ actions-411e83beb7](https://github.com/ursacomputing/crossbow/branches/all?query=actions-411e83beb7)
   
   |Task|Status|
   |----|------|
   |test-r-gcc-12|[![Github Actions](https://github.com/ursacomputing/crossbow/workflows/Crossbow/badge.svg?branch=actions-411e83beb7-github-test-r-gcc-12)](https://github.com/ursacomputing/crossbow/actions/runs/3341783687/jobs/5533324721)|
   |test-r-rhub-ubuntu-gcc-release-latest|[![Azure](https://dev.azure.com/ursacomputing/crossbow/_apis/build/status/ursacomputing.crossbow?branchName=actions-411e83beb7-azure-test-r-rhub-ubuntu-gcc-release-latest)](https://github.com/ursacomputing/crossbow/runs/9153918991)|


-- 
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: github-unsubscribe@arrow.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


[GitHub] [arrow] github-actions[bot] commented on pull request #14530: ARROW-18174: [R] Fix compile of altrep.cpp on some builds

Posted by GitBox <gi...@apache.org>.
github-actions[bot] commented on PR #14530:
URL: https://github.com/apache/arrow/pull/14530#issuecomment-1295048688

   Revision: fc514d0ead58f34dfc609873017559f7db913d2d
   
   Submitted crossbow builds: [ursacomputing/crossbow @ actions-b8ea8d97c9](https://github.com/ursacomputing/crossbow/branches/all?query=actions-b8ea8d97c9)
   
   |Task|Status|
   |----|------|
   |test-r-gcc-12|[![Github Actions](https://github.com/ursacomputing/crossbow/workflows/Crossbow/badge.svg?branch=actions-b8ea8d97c9-github-test-r-gcc-12)](https://github.com/ursacomputing/crossbow/actions/runs/3346056131/jobs/5542377139)|
   |test-r-rhub-ubuntu-gcc-release-latest|[![Azure](https://dev.azure.com/ursacomputing/crossbow/_apis/build/status/ursacomputing.crossbow?branchName=actions-b8ea8d97c9-azure-test-r-rhub-ubuntu-gcc-release-latest)](https://github.com/ursacomputing/crossbow/runs/9165698129)|


-- 
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: github-unsubscribe@arrow.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


[GitHub] [arrow] paleolimbot commented on a diff in pull request #14530: ARROW-18174: [R] Fix compile of altrep.cpp on some builds

Posted by GitBox <gi...@apache.org>.
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: github-unsubscribe@arrow.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


[GitHub] [arrow] kou commented on pull request #14530: ARROW-18174: [R] Fix compile of altrep.cpp on some builds

Posted by GitBox <gi...@apache.org>.
kou commented on PR #14530:
URL: https://github.com/apache/arrow/pull/14530#issuecomment-1329777479

   Thanks!
   Do you want to fix this problem? Or do you want me to do it?


-- 
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: github-unsubscribe@arrow.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


[GitHub] [arrow] kou commented on pull request #14530: ARROW-18174: [R] Fix compile of altrep.cpp on some builds

Posted by GitBox <gi...@apache.org>.
kou commented on PR #14530:
URL: https://github.com/apache/arrow/pull/14530#issuecomment-1329931879

   OK!
   If there is a problem with the suggestion, please let me know.


-- 
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: github-unsubscribe@arrow.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


[GitHub] [arrow] ursabot commented on pull request #14530: ARROW-18174: [R] Fix compile of altrep.cpp on some builds

Posted by GitBox <gi...@apache.org>.
ursabot commented on PR #14530:
URL: https://github.com/apache/arrow/pull/14530#issuecomment-1297314386

   Benchmark runs are scheduled for baseline = c7d97307925b08c6573a5f14d8a2222c116a0f88 and contender = e91d228c8dae139664c4bfa78b86484a2cd2484e. e91d228c8dae139664c4bfa78b86484a2cd2484e is a master commit associated with this PR. Results will be available as each benchmark for each run completes.
   Conbench compare runs links:
   [Finished :arrow_down:0.0% :arrow_up:0.0%] [ec2-t3-xlarge-us-east-2](https://conbench.ursa.dev/compare/runs/8a347b794dc0441a89adf4153c89a242...29f480a5673a4144addd224e9c014c43/)
   [Failed :arrow_down:0.0% :arrow_up:0.0%] [test-mac-arm](https://conbench.ursa.dev/compare/runs/a5fb849aa26c4faf8bbac667fdf90674...80f8614d174f4328be3baf7bddf72e76/)
   [Failed :arrow_down:0.0% :arrow_up:0.0%] [ursa-i9-9960x](https://conbench.ursa.dev/compare/runs/e7bffd6ed3e5427088d079ab5e93822c...a2feb58a72c24f23a865d172ae3a46c7/)
   [Finished :arrow_down:0.29% :arrow_up:0.04%] [ursa-thinkcentre-m75q](https://conbench.ursa.dev/compare/runs/8a34b88eaffd4f36a1f093fdb28436f1...70812e0e361d49f29d7de0ab6f275be4/)
   Buildkite builds:
   [Finished] [`e91d228c` ec2-t3-xlarge-us-east-2](https://buildkite.com/apache-arrow/arrow-bci-benchmark-on-ec2-t3-xlarge-us-east-2/builds/1792)
   [Failed] [`e91d228c` test-mac-arm](https://buildkite.com/apache-arrow/arrow-bci-benchmark-on-test-mac-arm/builds/1813)
   [Failed] [`e91d228c` ursa-i9-9960x](https://buildkite.com/apache-arrow/arrow-bci-benchmark-on-ursa-i9-9960x/builds/1780)
   [Finished] [`e91d228c` ursa-thinkcentre-m75q](https://buildkite.com/apache-arrow/arrow-bci-benchmark-on-ursa-thinkcentre-m75q/builds/1805)
   [Finished] [`c7d97307` ec2-t3-xlarge-us-east-2](https://buildkite.com/apache-arrow/arrow-bci-benchmark-on-ec2-t3-xlarge-us-east-2/builds/1791)
   [Failed] [`c7d97307` test-mac-arm](https://buildkite.com/apache-arrow/arrow-bci-benchmark-on-test-mac-arm/builds/1812)
   [Finished] [`c7d97307` ursa-i9-9960x](https://buildkite.com/apache-arrow/arrow-bci-benchmark-on-ursa-i9-9960x/builds/1779)
   [Finished] [`c7d97307` ursa-thinkcentre-m75q](https://buildkite.com/apache-arrow/arrow-bci-benchmark-on-ursa-thinkcentre-m75q/builds/1804)
   Supported benchmarks:
   ec2-t3-xlarge-us-east-2: Supported benchmark langs: Python, R. Runs only benchmarks with cloud = True
   test-mac-arm: Supported benchmark langs: C++, Python, R
   ursa-i9-9960x: Supported benchmark langs: Python, R, JavaScript
   ursa-thinkcentre-m75q: Supported benchmark langs: C++, Java
   


-- 
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: github-unsubscribe@arrow.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


[GitHub] [arrow] paleolimbot commented on pull request #14530: ARROW-18174: [R] Fix compile of altrep.cpp on some builds

Posted by GitBox <gi...@apache.org>.
paleolimbot commented on PR #14530:
URL: https://github.com/apache/arrow/pull/14530#issuecomment-1294247825

   @github-actions crossbow submit test-r-gcc-12 test-r-rhub-ubuntu-gcc-release-latest


-- 
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: github-unsubscribe@arrow.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


[GitHub] [arrow] paleolimbot commented on pull request #14530: ARROW-18174: [R] Fix compile of altrep.cpp on some builds

Posted by GitBox <gi...@apache.org>.
paleolimbot commented on PR #14530:
URL: https://github.com/apache/arrow/pull/14530#issuecomment-1294965111

   @github-actions crossbow submit test-r-gcc-12 test-r-rhub-ubuntu-gcc-release-latest


-- 
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: github-unsubscribe@arrow.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


[GitHub] [arrow] github-actions[bot] commented on pull request #14530: ARROW-18174: [R] Fix compile of altrep.cpp on some builds

Posted by GitBox <gi...@apache.org>.
github-actions[bot] commented on PR #14530:
URL: https://github.com/apache/arrow/pull/14530#issuecomment-1293503381

   https://issues.apache.org/jira/browse/ARROW-18174


-- 
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: github-unsubscribe@arrow.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


[GitHub] [arrow] kou commented on pull request #14530: ARROW-18174: [R] Fix compile of altrep.cpp on some builds

Posted by GitBox <gi...@apache.org>.
kou commented on PR #14530:
URL: https://github.com/apache/arrow/pull/14530#issuecomment-1328325872

   @github-actions crossbow submit test-r-devdocs


-- 
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: github-unsubscribe@arrow.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org