Skip to content

ARROW-11277: [C++] Workaround macOS 10.11: don't default construct consts #9267

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.

Sign up for GitHub

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

Closed
bkietz wants to merge 1 commit into from
Closed

ARROW-11277: [C++] Workaround macOS 10.11: don't default construct consts #9267

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 1 addition & 1 deletion cpp/src/arrow/dataset/expression.cc
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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{};
Copy link
Member

@pitrou pitrou Jan 19, 2021

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Perhaps we can give Datum a default constructor?

Copy link
Member Author

@bkietz bkietz Jan 19, 2021

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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

Copy link
Member

@pitrou pitrou Jan 19, 2021

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Uh.

Copy link
Member Author

@bkietz bkietz Jan 19, 2021

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

precisely

ARROW_ASSIGN_OR_RAISE(Datum constant,
ExecuteScalarExpression(expr, ignored_input));

Expand Down
9 changes: 5 additions & 4 deletions cpp/src/arrow/util/future.h
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -54,7 +54,7 @@ struct is_future<Future<T>> : std::true_type {};
template <typename Signature>
using result_of_t = typename std::result_of<Signature>::type;

constexpr struct ContinueFuture {
struct ContinueFuture {
template <typename Return>
struct ForReturnImpl;

Expand Down Expand Up @@ -99,7 +99,7 @@ constexpr struct ContinueFuture {

signal_to_complete_next.AddCallback(MarkNextFinished{std::move(next)});
}
} Continue;
};

template <>
struct ContinueFuture::ForReturnImpl<void> {
Expand Down Expand Up @@ -438,13 +438,14 @@ class ARROW_MUST_USE_TYPE Future {

struct Callback {
void operator()(const Result<T>& result) && {
detail::ContinueFuture continue_future;
if (ARROW_PREDICT_TRUE(result.ok())) {
// move on_failure to a(n immediately destroyed) temporary to free its resources
ARROW_UNUSED(OnFailure(std::move(on_failure)));
detail::Continue(std::move(next), std::move(on_success), result.ValueOrDie());
continue_future(std::move(next), std::move(on_success), result.ValueOrDie());
} else {
ARROW_UNUSED(OnSuccess(std::move(on_success)));
detail::Continue(std::move(next), std::move(on_failure), result.status());
continue_future(std::move(next), std::move(on_failure), result.status());
}
}

Expand Down
4 changes: 2 additions & 2 deletions cpp/src/arrow/util/thread_pool.h
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -95,8 +95,8 @@ class ARROW_EXPORT Executor {
Result<FutureType> Submit(TaskHints hints, Function&& func, Args&&... args) {
auto future = FutureType::Make();

auto task = std::bind(::arrow::detail::Continue, future, std::forward<Function>(func),
std::forward<Args>(args)...);
auto task = std::bind(::arrow::detail::ContinueFuture{}, future,
std::forward<Function>(func), std::forward<Args>(args)...);
ARROW_RETURN_NOT_OK(SpawnReal(hints, std::move(task)));

return future;
Expand Down