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 2021/01/19 15:54:07 UTC

[GitHub] [arrow] bkietz opened a new pull request #9267: ARROW-11277: [C++] Workaround macOS 10.11: don't default construct consts

bkietz opened a new pull request #9267:
URL: https://github.com/apache/arrow/pull/9267


   ```c++
   /tmp/apache-arrow-20210116-6233-1jyrhk8/apache-arrow-3.0.0/cpp/src/arrow/dataset/expression.cc
   /tmp/apache-arrow-20210116-6233-1jyrhk8/apache-arrow-3.0.0/cpp/src/arrow/dataset/expression.cc:684:30:
   error: default initialization of an object of const type 'const arrow::Datum' without a user-provided default constructor
             static const Datum ignored_input;
   ```
   Datum defines a default constructor but it doesn't seem to be found for const/constexpr decls


----------------------------------------------------------------
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.

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



[GitHub] [arrow] bkietz commented on a change in pull request #9267: ARROW-11277: [C++] Workaround macOS 10.11: don't default construct consts

Posted by GitBox <gi...@apache.org>.
bkietz commented on a change in pull request #9267:
URL: https://github.com/apache/arrow/pull/9267#discussion_r560302275



##########
File path: cpp/src/arrow/dataset/expression.cc
##########
@@ -681,7 +681,7 @@ Result<Expression> FoldConstants(Expression expr) {
         if (std::all_of(call->arguments.begin(), call->arguments.end(),
                         [](const Expression& argument) { return argument.literal(); })) {
           // all arguments are literal; we can evaluate this subexpression *now*
-          static const Datum ignored_input;
+          static const Datum ignored_input = Datum{};

Review comment:
       https://github.com/bkietz/arrow/blob/17f2df78f1fa623d05dd01f245f8f65f828f510d/cpp/src/arrow/datum.h#L118-L119
   
   Datum has one already; resolution of that constructor is just failing here




----------------------------------------------------------------
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.

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



[GitHub] [arrow] pitrou commented on a change in pull request #9267: ARROW-11277: [C++] Workaround macOS 10.11: don't default construct consts

Posted by GitBox <gi...@apache.org>.
pitrou commented on a change in pull request #9267:
URL: https://github.com/apache/arrow/pull/9267#discussion_r560287406



##########
File path: cpp/src/arrow/dataset/expression.cc
##########
@@ -681,7 +681,7 @@ Result<Expression> FoldConstants(Expression expr) {
         if (std::all_of(call->arguments.begin(), call->arguments.end(),
                         [](const Expression& argument) { return argument.literal(); })) {
           // all arguments are literal; we can evaluate this subexpression *now*
-          static const Datum ignored_input;
+          static const Datum ignored_input = Datum{};

Review comment:
       Perhaps we can give Datum a default constructor?




----------------------------------------------------------------
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.

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



[GitHub] [arrow] jeroen commented on pull request #9267: ARROW-11277: [C++] Workaround macOS 10.11: don't default construct consts

Posted by GitBox <gi...@apache.org>.
jeroen commented on pull request #9267:
URL: https://github.com/apache/arrow/pull/9267#issuecomment-762966925


   Thanks I'm going to test this patch on top of rc2. Btw the MacOS 10.11 target be be EOL for us in april/may (but I imagine RHEL users will have the same issues....)


----------------------------------------------------------------
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.

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



[GitHub] [arrow] pitrou commented on a change in pull request #9267: ARROW-11277: [C++] Workaround macOS 10.11: don't default construct consts

Posted by GitBox <gi...@apache.org>.
pitrou commented on a change in pull request #9267:
URL: https://github.com/apache/arrow/pull/9267#discussion_r560303887



##########
File path: cpp/src/arrow/dataset/expression.cc
##########
@@ -681,7 +681,7 @@ Result<Expression> FoldConstants(Expression expr) {
         if (std::all_of(call->arguments.begin(), call->arguments.end(),
                         [](const Expression& argument) { return argument.literal(); })) {
           // all arguments are literal; we can evaluate this subexpression *now*
-          static const Datum ignored_input;
+          static const Datum ignored_input = Datum{};

Review comment:
       Uh.




----------------------------------------------------------------
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.

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



[GitHub] [arrow] jeroen commented on pull request #9267: ARROW-11277: [C++] Workaround macOS 10.11: don't default construct consts

Posted by GitBox <gi...@apache.org>.
jeroen commented on pull request #9267:
URL: https://github.com/apache/arrow/pull/9267#issuecomment-762975550


   Thanks confirmed this works. With this patch I can build rc2 on macos 10.11 


----------------------------------------------------------------
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.

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



[GitHub] [arrow] github-actions[bot] commented on pull request #9267: ARROW-11277: [C++] Workaround macOS 10.11: don't default construct consts

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


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


----------------------------------------------------------------
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.

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



[GitHub] [arrow] bkietz commented on a change in pull request #9267: ARROW-11277: [C++] Workaround macOS 10.11: don't default construct consts

Posted by GitBox <gi...@apache.org>.
bkietz commented on a change in pull request #9267:
URL: https://github.com/apache/arrow/pull/9267#discussion_r560349996



##########
File path: cpp/src/arrow/dataset/expression.cc
##########
@@ -681,7 +681,7 @@ Result<Expression> FoldConstants(Expression expr) {
         if (std::all_of(call->arguments.begin(), call->arguments.end(),
                         [](const Expression& argument) { return argument.literal(); })) {
           // all arguments are literal; we can evaluate this subexpression *now*
-          static const Datum ignored_input;
+          static const Datum ignored_input = Datum{};

Review comment:
       precisely




----------------------------------------------------------------
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.

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



[GitHub] [arrow] nealrichardson closed pull request #9267: ARROW-11277: [C++] Workaround macOS 10.11: don't default construct consts

Posted by GitBox <gi...@apache.org>.
nealrichardson closed pull request #9267:
URL: https://github.com/apache/arrow/pull/9267


   


----------------------------------------------------------------
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.

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