[ADT] Move llvm::identity into IndexedMap (NFC)

[kazutakahirata]

This patch moves llvm::identity to IndexedMap for two reasons:

• llvm::identity is used only IndexedMap.
• llvm::identity is not suitable for general use as it is not quite
  the same as std::identity despite the comments in identity.h.

Also, this patch renames the class to IdentityIndex and places it in
the "detail" namespace.

[llvmbot]

@llvm/pr-subscribers-llvm-adt

Author: Kazu Hirata (kazutakahirata)

Changes

The legacy llvm::identity differs from std::identity in C++20.
llvm::identity is a template struct with an ::argument_type member. In
contrast, llvm::identity_cxx20 (and std::identity) is a non-template
struct with a templated call operator and no ::argument_type.

This patch modernizes llvm::IndexedMap by updating its default functor
to llvm::identity_cxx20. A new template parameter IndexT now
fulfills the role of ::argument_type.

To avoid modifying numerous existing IndexedMap uses with custom
functors (e.g., VirtReg2IndexFunctor), the detail::DeduceIndexType
helper automatically deduces IndexT. It deduces ::argument_type for
legacy functors and unsigned for llvm::identity_cxx20.

---

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

1 Files Affected:

• (modified) llvm/include/llvm/ADT/IndexedMap.h (+22-3)

diff --git a/llvm/include/llvm/ADT/IndexedMap.h b/llvm/include/llvm/ADT/IndexedMap.h
index cda0316dc78fa..8526db9cdf4e8 100644
--- a/llvm/include/llvm/ADT/IndexedMap.h
+++ b/llvm/include/llvm/ADT/IndexedMap.h
@@ -21,14 +21,33 @@
 #define LLVM_ADT_INDEXEDMAP_H
 
 #include "llvm/ADT/STLExtras.h"
+#include "llvm/ADT/STLForwardCompat.h"
 #include "llvm/ADT/SmallVector.h"
-#include "llvm/ADT/identity.h"
 #include <cassert>
+#include <type_traits>
 
 namespace llvm {
 
-template <typename T, typename ToIndexT = identity<unsigned>> class IndexedMap {
-  using IndexT = typename ToIndexT::argument_type;
+namespace detail {
+// Helper to compute the IndexT for IndexedMap.
+template <typename ToIndexT, typename = void> struct DeduceIndexType {
+  using type =
+      std::conditional_t<std::is_same_v<ToIndexT, llvm::identity_cxx20>,
+                         unsigned, void>;
+};
+
+template <typename ToIndexT>
+struct DeduceIndexType<ToIndexT,
+                       std::void_t<typename ToIndexT::argument_type>> {
+  using type = typename ToIndexT::argument_type;
+};
+} // namespace detail
+
+template <typename T, typename ToIndexT = llvm::identity_cxx20,
+          typename IndexT = typename detail::DeduceIndexType<ToIndexT>::type>
+class IndexedMap {
+  static_assert(!std::is_same_v<IndexT, void>,
+                "Could not deduce index type from the provided functor.");
   // Prefer SmallVector with zero inline storage over std::vector. IndexedMaps
   // can grow very large and SmallVector grows more efficiently as long as T
   // is trivially copyable.

[kuhar]

Shouldn't we retain llvm::identity for cases like this? It seems much cleaner to me.

[kazutakahirata]

Shouldn't we retain llvm::identity for cases like this? It seems much cleaner to me.

@kuhar Well, C++ is moving away from argument_type. How about removing the dependence on argument_type completely? Please take a look at the latest iteration. Thanks!

[kuhar]

I understand c++ is doing its thing, but I think this is an example that shows it's useful to combine std::type_identity and std::identity, which is what llvm::identity does.

[kuhar]

I don't think we would be gaining much be removing llvm::identity in favor of std::identity + workarounds for lack of the argument type

[kazutakahirata]

Thanks for the review! I take your point. Given that IndexedMap.h is the only user of llvm::identity, would you be open to me moving llvm::identity to IndexedMap.h? Maybe rename it to IdentityIndex wrapped in namespace detail?

Otherwise, I am just happy to update comments in ADT/identity.h to clarify that it's not really the same thing as std::identity.

[kuhar]

yes, that makes sense, we can inline it there