[ADT] Backport std::to_underlying from C++23

[Endilll]

This patch backports a one-liner std::to_underlying that came with C++23. This is useful for refactoring unscoped enums into scoped enums, because the latter are not implicitly convertible to integer types.

I followed libc++ implementation, but I consider their testing too heavy for us, so I wrote a simpler set of tests.

[llvmbot]

@llvm/pr-subscribers-llvm-adt

@llvm/pr-subscribers-llvm-support

Author: Vlad Serebrennikov (Endilll)

Changes

This patch backports a one-liner std::to_underlying that came with C++23. This is useful for refactoring unscoped enums into scoped enums, because the latter are not implicitly convertible to integer types.

I followed libc++ implementation, but I consider their testing too heavy for us, so I wrote a simpler set of tests.

---

Full diff: https://github.com/llvm/llvm-project/pull/70681.diff

2 Files Affected:

• (modified) llvm/include/llvm/ADT/STLExtras.h (+6)
• (modified) llvm/unittests/ADT/STLExtrasTest.cpp (+22)

diff --git a/llvm/include/llvm/ADT/STLExtras.h b/llvm/include/llvm/ADT/STLExtras.h
index 18bc4d108b156bf..98342bfc36e9f83 100644
--- a/llvm/include/llvm/ADT/STLExtras.h
+++ b/llvm/include/llvm/ADT/STLExtras.h
@@ -1535,6 +1535,12 @@ constexpr decltype(auto) makeVisitor(CallableTs &&...Callables) {
   return detail::Visitor<CallableTs...>(std::forward<CallableTs>(Callables)...);
 }
 
+/// Backport of C++23 std::to_underlying.
+template <typename Enum>
+[[nodiscard]] constexpr typename std::underlying_type_t<Enum> to_underlying(Enum E) {
+  return static_cast<typename std::underlying_type_t<Enum>>(E);
+}
+
 //===----------------------------------------------------------------------===//
 //     Extra additions to <algorithm>
 //===----------------------------------------------------------------------===//
diff --git a/llvm/unittests/ADT/STLExtrasTest.cpp b/llvm/unittests/ADT/STLExtrasTest.cpp
index 7db339e4ef31cdc..213b3e4b3d06fb6 100644
--- a/llvm/unittests/ADT/STLExtrasTest.cpp
+++ b/llvm/unittests/ADT/STLExtrasTest.cpp
@@ -1332,4 +1332,26 @@ struct Bar {};
 static_assert(is_incomplete_v<Foo>, "Foo is incomplete");
 static_assert(!is_incomplete_v<Bar>, "Bar is defined");
 
+TEST(STLExtrasTest, ToUnderlying) {
+  enum E {
+    A1 = 0, B1 = -1
+  };
+  static_assert(to_underlying(A1) == 0);
+  static_assert(to_underlying(B1) == -1);
+
+  enum E2 : unsigned char {
+    A2 = 0, B2
+  };
+  static_assert(std::is_same_v<unsigned char, decltype(to_underlying(A2))>);
+  static_assert(to_underlying(A2) == 0);
+  static_assert(to_underlying(B2) == 1);
+
+  enum class E3 {
+    A3 = -1, B3
+  };
+  static_assert(std::is_same_v<int, decltype(to_underlying(E3::A3))>);
+  static_assert(to_underlying(E3::A3) == -1);
+  static_assert(to_underlying(E3::B3) == 0);
+}
+
 } // namespace

[github-actions]

✅ With the latest revision this PR passed the C/C++ code formatter.

[AaronBallman]

The changes look reasonable to me, and I presume you intend to use this in-tree for the enum changes you've been working on in Clang. But I added a few more reviewers just to get more sets of eyes on the changes; please don't land until at least one more reviewer has accepted.

[Endilll]

Formatting is fine. I guess I pushed formatting commit too fast for bot to notice.

[slinder1]

The code LGTM, but with one small nit: as it is a backport of a C++23 feature please move it to include/llvm/ADT/STLForwardCompat.h

[kuhar]

Makes sense, I saw this implemented in half a dozen of downstream llvm-based projects.

Have you tried grepping the codebase and checking if there are some existing helpers that can be replaced by this?

[Endilll]

Have you tried grepping the codebase and checking if there are some existing helpers that can be replaced by this?

I can't say I performed an exhaustive search, but apparently there is only one helper somewhere deep in MLIR:

llvm-project/mlir/lib/Dialect/SparseTensor/IR/Detail/TemplateExtras.h

Lines 41 to 45 in 0f8615f

	/// Convert an enum to its underlying type.
	template <typename Enum>
	constexpr std::underlying_type_t<Enum> to_underlying(Enum e) noexcept {
	return static_cast<std::underlying_type_t<Enum>>(e);
	}

[kuhar]

Have you tried grepping the codebase and checking if there are some existing helpers that can be replaced by this?

I can't say I performed an exhaustive search, but apparently there is only one helper somewhere deep in MLIR:

llvm-project/mlir/lib/Dialect/SparseTensor/IR/Detail/TemplateExtras.h

Lines 41 to 45 in 0f8615f

	/// Convert an enum to its underlying type.
	template <typename Enum>
	constexpr std::underlying_type_t<Enum> to_underlying(Enum e) noexcept {
	return static_cast<std::underlying_type_t<Enum>>(e);
	}

Thanks for checking, @Endilll. Would be nice to replace this with the function from this PR.

[Endilll]

@slinder1 I didn't know STLForwardCompat.h even exists! Thank you for mentioning.

[Endilll]

@kuhar Would it be fine to leave such replacement out of scope of this PR?

[kuhar]

@kuhar Would it be fine to leave such replacement out of scope of this PR?

I'd land it together, otherwise this change has no uses in the codebase. And the change is very simple.

[Endilll]

I'd land it together, otherwise this change has no uses in the codebase. And the change is very simple.

This is driven by my work on Clang enums, so I intentionally factored this PR out so that this useful facility is not tied to any particular usage (that might get reverted). I'm going to commit several NFC patches tomorrow that use this.

[kuhar]

I'd land it together, otherwise this change has no uses in the codebase. And the change is very simple.

This is driven by my work on Clang enums, so I intentionally factored this PR out so that this useful facility is not tied to any particular usage (that might get reverted). I'm going to commit several NFC patches tomorrow that use this.

The clang part makes sense to me, but the other usage seems to have exactly the same API and implementation and should be completely safe to replace.

[AaronBallman]

@kuhar Would it be fine to leave such replacement out of scope of this PR?

I'd land it together, otherwise this change has no uses in the codebase. And the change is very simple.

FWIW, it's pretty typical for follow-up commits to start using the new ADT functionality; this separation makes it a bit easier to revert if necessary (when the ADT is fine in general but its specific use as a replacement is problematic for some reason).

[kuhar]

@kuhar Would it be fine to leave such replacement out of scope of this PR?

I'd land it together, otherwise this change has no uses in the codebase. And the change is very simple.

FWIW, it's pretty typical for follow-up commits to start using the new ADT functionality; this separation makes it a bit easier to revert if necessary (when the ADT is fine in general but its specific use as a replacement is problematic for some reason).

In this specific scenario we would be replacing the same implementation with the same API. Because this is a detail, I don't think it's worth arguing over and I approved the PR as-is.