Skip to content

Comments

GH-48590: [C++] Modernize type traits using C++20 concepts (Preliminary)#49348

Open
abhishek593 wants to merge 1 commit intoapache:mainfrom
abhishek593:GH-48590
Open

GH-48590: [C++] Modernize type traits using C++20 concepts (Preliminary)#49348
abhishek593 wants to merge 1 commit intoapache:mainfrom
abhishek593:GH-48590

Conversation

@abhishek593
Copy link
Contributor

@abhishek593 abhishek593 commented Feb 20, 2026

Rationale for this change

This is a preliminary PR intended to demonstrate the scope and feasibility of modernizing the Arrow C++ codebase using C++20 Concepts.

What changes are included in this PR?

  • Added several concepts covering all Arrow type categories.
  • Updated some of the core files to demonstrate how concepts simplify template constraints.
  • Existing enable_if helpers in these files are preserved. We can remove them after full migration.

Are these changes tested?

Yes.

Are there any user-facing changes?

No.

Questions for the Reviewer

  1. Should we delete the enable_if_* trait definitions entirely since Arrow is now C++20-only?
  2. Are there specific areas where you would prefer to keep SFINAE for any compatibility reasons, or should we go all-in?

@abhishek593 abhishek593 changed the title MINOR: [C++] Modernize type traits using C++20 concepts GH-48590: [C++] Modernize type traits using C++20 concepts (Preliminary) Feb 20, 2026
@abhishek593
Copy link
Contributor Author

@pitrou I just want to post my analysis here.

The code so far what I have submitted is standard-compliant (compiles on gcc-14, gcc-15, clang). But the check C++ / C++ Minimal Build Example fails which uses gcc-13. This is because of a bug in gcc-13 when evaluating concept-constrained partial specialisation of nested templates. Hence, the error:

Error from CI:

/arrow/cpp/src/arrow/compute/kernels/codegen_internal.h:736:21: error: 'Arg0Value' has not been declared

  736 |           arg0, [&](Arg0Value v) { return builder.Append(functor.op.Call(ctx, v, &st)); },

Code from codegen_internal.h(present in my changes):

template <arrow_c_number_or_decimal Type>
struct ArrayExec<Type> {
  static Status Exec(...) {
    // GCC 13 throws an error on `Arg0Value` and `OutValue` inside this lambda
    VisitArrayValuesInline<Arg0Type>(arg0, [&](Arg0Value v) {
      *out_data++ = functor.op.template Call<OutValue, Arg0Value>(ctx, v, &st);
    });
  }
};

Since CI uses these older compilers, we may have a few options if we want to proceed:

  1. Explicit qualification: Explicitly qualify the types wherever they are used inside the specialisation
-   VisitArrayValuesInline<Arg0Type>(arg0, [&](Arg0Value v) {
-      *out_data++ = functor.op.template Call<OutValue, Arg0Value>(ctx, v, &st);
+   VisitArrayValuesInline<Arg0Type>(arg0, [&](typename ThisType::Arg0Value v) {
+      *out_data++ = functor.op.template Call<typename ThisType::OutValue, typename ThisType::Arg0Value>(ctx, v, &st);
  1. Identify such instances and fallback to SFINAE (lot of work).
  2. Upgrade CI compilers.

BTW, is there any reason why compilers are not being upgraded?

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

Labels

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant