@llvm/pr-subscribers-llvm-support

Author: Kazu Hirata (kazutakahirata)

diff --git a/llvm/include/llvm/Support/ReverseIteration.h b/llvm/include/llvm/Support/ReverseIteration.h
index 9e9411856369e..456d423600d6e 100644
--- a/llvm/include/llvm/Support/ReverseIteration.h
+++ b/llvm/include/llvm/Support/ReverseIteration.h
@@ -6,13 +6,11 @@
 
 namespace llvm {
 
-template<class T = void *>
-bool shouldReverseIterate() {
-#if LLVM_ENABLE_REVERSE_ITERATION
-  return detail::IsPointerLike<T>::value;
-#else
-  return false;
-#endif
+template <class T = void *> constexpr bool shouldReverseIterate() {
+  if constexpr (LLVM_ENABLE_REVERSE_ITERATION)
+    return detail::IsPointerLike<T>::value;
+  else
+    return false;
 }
 
 } // namespace llvm

Is this really a simplification? This is a macro, so I'm not sure what the benefit is. Is it for better IDE/lsp support?

@kuhar Maybe simplification is a wrong word here. Making this function constexpr can help simplify its users. It can be used as in std::conditional_t for example. I'm planning to use the constexpr version of this function to further simplify DenseMapIterator.

You can make it contexpr while still keeping the macro. I'm not saying the new implementation with if constexpr is worse but that I don't know if it's better.

You can make it contexpr while still keeping the macro. I'm not saying the new implementation with if constexpr is worse but that I don't know if it's better.

@kuhar I now retain the macro and just make the return type constexpr.