[ADT] Refactor SmallVector::assertSafeToAddRange with "constexpr if"

[kazutakahirata]

This patch consolidates two implementations of assertSafeToAddRange
into a single template function.

The new implementation uses "constexpr if" to check for pointer
iterators, preserving the original behavior while simplifying the
code.

[llvmbot]

@llvm/pr-subscribers-llvm-adt

Author: Kazu Hirata (kazutakahirata)

Changes

This patch consolidates two implementations of assertSafeToAddRange
into a single template function.

The new implementation uses "constexpr if" to check for pointer
iterators, preserving the original behavior while simplifying the
code.

---

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

1 Files Affected:

• (modified) llvm/include/llvm/ADT/SmallVector.h (+9-10)

diff --git a/llvm/include/llvm/ADT/SmallVector.h b/llvm/include/llvm/ADT/SmallVector.h
index 80f7734b86907..5577b09fee89c 100644
--- a/llvm/include/llvm/ADT/SmallVector.h
+++ b/llvm/include/llvm/ADT/SmallVector.h
@@ -212,17 +212,16 @@ class SmallVectorTemplateCommon
   void assertSafeToReferenceAfterClear(ItTy, ItTy) {}
 
   /// Check whether any part of the range will be invalidated by growing.
-  void assertSafeToAddRange(const T *From, const T *To) {
-    if (From == To)
-      return;
-    this->assertSafeToAdd(From, To - From);
-    this->assertSafeToAdd(To - 1, To - From);
+  template <class ItTy> void assertSafeToAddRange(ItTy From, ItTy To) {
+    if constexpr (std::is_pointer_v<ItTy> &&
+                  std::is_same_v<std::remove_cv_t<std::remove_pointer_t<ItTy>>,
+                                 T>) {
+      if (From == To)
+        return;
+      this->assertSafeToAdd(From, To - From);
+      this->assertSafeToAdd(To - 1, To - From);
+    }
   }
-  template <
-      class ItTy,
-      std::enable_if_t<!std::is_same<std::remove_const_t<ItTy>, T *>::value,
-                       bool> = false>
-  void assertSafeToAddRange(ItTy, ItTy) {}
 
   /// Reserve enough space to add one element, and return the updated element
   /// pointer in case it was a reference to the storage.

[klausler]

This commit is breaking my flang builds.

In file included from /home/pklausler/llvm-project/llvm/include/llvm/ADT/ArrayRef.h:13,
                 from /home/pklausler/llvm-project/llvm/include/llvm/Support/MemoryBuffer.h:17,
                 from /home/pklausler/llvm-project/flang/include/flang/Parser/source.h:19,
                 from /home/pklausler/llvm-project/flang/include/flang/Parser/provenance.h:15,
                 from /home/pklausler/llvm-project/flang/include/flang/Parser/message.h:17,
                 from /home/pklausler/llvm-project/flang/include/flang/Evaluate/common.h:19,
                 from /home/pklausler/llvm-project/flang/include/flang/Evaluate/expression.h:19,
                 from /home/pklausler/llvm-project/flang/include/flang/Semantics/type.h:13,
                 from /home/pklausler/llvm-project/flang/include/flang/Semantics/symbol.h:12,
                 from /home/pklausler/llvm-project/flang/include/flang/Support/OpenMP-utils.h:12,
                 from /home/pklausler/llvm-project/flang/lib/Support/OpenMP-utils.cpp:9:
/home/pklausler/llvm-project/llvm/include/llvm/ADT/SmallVector.h: In instantiation of ‘void llvm::SmallVectorTemplateCommon<T, <template-parameter-1-2> >::asser
tSafeToAddRange(ItTy, ItTy) [with ItTy = llvm::mapped_iterator<const mlir::AffineExpr*, mlir::AffineMap::shiftDims(unsigned int, unsigned int) const::<lambda(ml
ir::AffineExpr)>, mlir::AffineExpr>; T = mlir::AffineExpr; <template-parameter-1-2> = void]’:
/home/pklausler/llvm-project/llvm/include/llvm/ADT/SmallVector.h:684:5:   required from ‘void llvm::SmallVectorImpl<T>::append(ItTy, ItTy) [with ItTy = llvm::ma
pped_iterator<const mlir::AffineExpr*, mlir::AffineMap::shiftDims(unsigned int, unsigned int) const::<lambda(mlir::AffineExpr)>, mlir::AffineExpr>; <template-pa
rameter-2-2> = void; T = mlir::AffineExpr]’            
/home/pklausler/llvm-project/llvm/include/llvm/ADT/SmallVector.h:1217:5:   required from ‘llvm::SmallVector<T, N>::SmallVector(ItTy, ItTy) [with ItTy = llvm::ma
pped_iterator<const mlir::AffineExpr*, mlir::AffineMap::shiftDims(unsigned int, unsigned int) const::<lambda(mlir::AffineExpr)>, mlir::AffineExpr>; <template-pa
rameter-2-2> = void; T = mlir::AffineExpr; unsigned int N = 4]’
/home/pklausler/llvm-project/llvm/include/llvm/ADT/SmallVector.h:1300:45:   required from ‘llvm::SmallVector<typename std::remove_const<typename std::remove_ref
erence<decltype (* std::begin(declval<R&>()))>::type>::type, Size> llvm::to_vector(R&&) [with unsigned int Size = 4; R = llvm::iterator_range<llvm::mapped_itera
tor<const mlir::AffineExpr*, mlir::AffineMap::shiftDims(unsigned int, unsigned int) const::<lambda(mlir::AffineExpr)>, mlir::AffineExpr> >; typename std::remove
_const<typename std::remove_reference<decltype (* std::begin(declval<R&>()))>::type>::type = mlir::AffineExpr]’
/home/pklausler/llvm-project/llvm/include/llvm/ADT/SmallVectorExtras.h:39:25:   required from ‘auto llvm::map_to_vector(ContainerTy&&, FuncTy&&) [with unsigned
int Size = 4; ContainerTy = llvm::ArrayRef<mlir::AffineExpr>; FuncTy = mlir::AffineMap::shiftDims(unsigned int, unsigned int) const::<lambda(mlir::AffineExpr)>]
’                                                        
/home/pklausler/llvm-project/mlir/include/mlir/IR/AffineMap.h:274:32:   required from here
/home/pklausler/llvm-project/llvm/include/llvm/ADT/SmallVector.h:215:56: error: parameter ‘From’ set but not used [-Werror=unused-but-set-parameter]
  215 |   template <class ItTy> void assertSafeToAddRange(ItTy From, ItTy To) {
      |                                                   ~~~~~^~~~
/home/pklausler/llvm-project/llvm/include/llvm/ADT/SmallVector.h:215:67: error: parameter ‘To’ set but not used [-Werror=unused-but-set-parameter]
  215 |   template <class ItTy> void assertSafeToAddRange(ItTy From, ItTy To) {
      |                                                              ~~~~~^~

This is on x86-64 with GCC 9.3.0.

Please advise, fix, or revert.

[kuhar]

Please advise, fix, or revert.

You can add (void)From, To; after the if constexpr statement

[klausler]

Is that a permanent change that you're telling me to make, or a temporary patch until you can resolve this build failure?

[kuhar]

This is a proper fix to the warning you pasted. The code is fine in general, it's just this warning is a bit overzealous while you run with warnings as errors.

[kuhar]

Here you go: 2b5e29e