Skip to content

Remove XSIMD_IF_CONSTEXPR macros, use if constexpr instead#1382

Draft
JohanMabille wants to merge 4 commits into
xtensor-stack:masterfrom
JohanMabille:macro_cleanup
Draft

Remove XSIMD_IF_CONSTEXPR macros, use if constexpr instead#1382
JohanMabille wants to merge 4 commits into
xtensor-stack:masterfrom
JohanMabille:macro_cleanup

Conversation

@JohanMabille

@JohanMabille JohanMabille commented Jul 15, 2026 •
edited
Loading

Copy link
Copy Markdown
Member

Cleanup PR, mechanical replacement.

@JohanMabille
JohanMabille marked this pull request as ready for review July 15, 2026 14:57
@DiamonDinoia

DiamonDinoia commented Jul 15, 2026

Copy link
Copy Markdown
Contributor

We are moving to c++17? Happy to review. Are we going to do the full transition in this PR or the plan is to adopt if constexpr only?

@JohanMabille

JohanMabille commented Jul 16, 2026

Copy link
Copy Markdown
Member Author

I was pretty sure that there was an issue where we agreed to migrate to C++17 Oo I must be confusing it with another project. Anyway, Given that the current standard is 26, and that all compilers tested in the CI support it, I think we can move to C++17.

@serge-sans-paille @AntoinePrv any objection for this migration? I'm pretty sure we already discussed this.

@AntoinePrv

AntoinePrv commented Jul 16, 2026 •
edited
Loading

Copy link
Copy Markdown
Contributor

Yes we had decided to move to 17. @JohanMabille mabye let's make a first isolated PR where we move just the Cmake?
Just so we can find the commit "Move to C++17" in the history (or rename this one?).

AntoinePrv
AntoinePrv approved these changes Jul 16, 2026
View reviewed changes

@AntoinePrv AntoinePrv left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Looks mechanical, I trust you

@JohanMabille

JohanMabille commented Jul 16, 2026

Copy link
Copy Markdown
Member Author

Migration to C++17 done in #1383, I'll rebase this one when #1383 is merged.

@JohanMabille
JohanMabille marked this pull request as draft July 16, 2026 08:46
@DiamonDinoia

DiamonDinoia commented Jul 16, 2026

Copy link
Copy Markdown
Contributor

This PR looks correct!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@DiamonDinoia DiamonDinoia DiamonDinoia approved these changes
@AntoinePrv AntoinePrv AntoinePrv approved these changes
@serge-sans-paille serge-sans-paille Awaiting requested review from serge-sans-paille

Assignees

No one assigned

Labels

None yet

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

3 participants

@JohanMabille @DiamonDinoia @AntoinePrv