Skip to content

Conversation

@zhjwpku
Copy link
Collaborator

@zhjwpku zhjwpku commented Jul 5, 2025

No description provided.

@zhjwpku
Copy link
Collaborator Author

zhjwpku commented Jul 5, 2025

Waiting apache/arrow#46998 to be resolved, but should be fine for review.

@zhjwpku zhjwpku force-pushed the cpp_23_standard branch from 6ebada5 to ff5cf81 Compare July 8, 2025 04:44
@wgtmac
Copy link
Member

wgtmac commented Jul 8, 2025

This is no longer a draft, right?

@zhjwpku zhjwpku marked this pull request as ready for review July 8, 2025 07:33
@zhjwpku
Copy link
Collaborator Author

zhjwpku commented Jul 8, 2025

This is no longer a draft, right?

Yeah, I just make it ready for review.

The commit includes apache/arrow#46912,
which are resolve our frequent CI failures.
@zhjwpku
Copy link
Collaborator Author

zhjwpku commented Jul 8, 2025

I just changed to a more recent arrow commit which contains the Apache Thrift update[1], which should fix our frequent CI failure.

[1] apache/arrow#46912

@mapleFU
Copy link
Member

mapleFU commented Jul 8, 2025

So should Result<> be removed after this is introduced?

Besides, C++23 has resize_and_overwrite, allowing zero-cost buffer initialization

@zhjwpku
Copy link
Collaborator Author

zhjwpku commented Jul 8, 2025

So should Result<> be removed after this is introduced?

Personally I think Result/Status are well-structured wrappers, but I'm glad to hear others' opinions.

Besides, C++23 has resize_and_overwrite, allowing zero-cost buffer initialization

Ah, I wasn't aware of this feature, could you suggest where it might be applied?

I think we can adopt more C++23 features in future PRs, assuming compiler support. I tried using views::join_with to replace JoinByDot, but it's only supported starting from Clang 21 (see [1]), so I gave it up.

[1] https://libcxx.llvm.org/Status/Cxx23.html

@wgtmac
Copy link
Member

wgtmac commented Jul 8, 2025

I just changed to a more recent arrow commit which contains the Apache Thrift update[1], which should fix our frequent CI failure.

[1] apache/arrow#46912

I think we need to update it to use Apache Arrow 21.0.0 once it is released. It is only a temporary state to pin to an unreleased version.

@zhjwpku
Copy link
Collaborator Author

zhjwpku commented Jul 9, 2025

I just changed to a more recent arrow commit which contains the Apache Thrift update[1], which should fix our frequent CI failure.
[1] apache/arrow#46912

I think we need to update it to use Apache Arrow 21.0.0 once it is released. It is only a temporary state to pin to an unreleased version.

Sure, will keep an eye on it.

Copy link
Member

@Xuanwo Xuanwo left a comment

Choose a reason for hiding this comment

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

Thank you for working on this, let's move!

@Xuanwo Xuanwo merged commit 0a807ca into apache:main Jul 14, 2025
7 checks passed
@zhjwpku zhjwpku deleted the cpp_23_standard branch July 14, 2025 10:36
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

5 participants