[llvm] [LLVM][ADT] Explicitly convert size_t values to SmallVector's size type (PR #77939)

Andrei Golubev via llvm-commits llvm-commits at lists.llvm.org
Wed Jan 17 07:34:46 PST 2024


https://github.com/andrey-golubev updated https://github.com/llvm/llvm-project/pull/77939

>From df7078fc428ce2a169643df971abb13eebdba308 Mon Sep 17 00:00:00 2001
From: "Golubev, Andrey" <andrey.golubev at intel.com>
Date: Fri, 29 Dec 2023 15:53:17 +0000
Subject: [PATCH 1/3] [LLVM][ADT] Explicitly convert size_t values to
 SmallVector's size type

Multiple places rely on implicit conversion when assigning 'size_t' values
to the member fields (size or capacity) of SmallVector.

Depending on the platform / compiler configuration, this may result in
narrowing conversion warnings (especially given that the size type of
SmallVector's member fields is determined based on type T - in SmallVector<T>).
To avoid the problem altogether, make the conversions explicit.

Co-authored-by: Orest Chura <orest.chura at intel.com>
---
 llvm/include/llvm/ADT/SmallVector.h | 9 +++++----
 1 file changed, 5 insertions(+), 4 deletions(-)

diff --git a/llvm/include/llvm/ADT/SmallVector.h b/llvm/include/llvm/ADT/SmallVector.h
index 2e6d2dc6ce90a2..65e862911522a8 100644
--- a/llvm/include/llvm/ADT/SmallVector.h
+++ b/llvm/include/llvm/ADT/SmallVector.h
@@ -61,7 +61,7 @@ template <class Size_T> class SmallVectorBase {
 
   SmallVectorBase() = delete;
   SmallVectorBase(void *FirstEl, size_t TotalCapacity)
-      : BeginX(FirstEl), Capacity(TotalCapacity) {}
+      : BeginX(FirstEl), Capacity(static_cast<Size_T>(TotalCapacity)) {}
 
   /// This is a helper for \a grow() that's out of line to reduce code
   /// duplication.  This function will report a fatal error if it can't grow at
@@ -99,8 +99,9 @@ template <class Size_T> class SmallVectorBase {
   ///
   /// This does not construct or destroy any elements in the vector.
   void set_size(size_t N) {
-    assert(N <= capacity());
-    Size = N;
+    auto n = static_cast<Size_T>(N);
+    assert(n <= capacity());
+    Size = n;
   }
 };
 
@@ -468,7 +469,7 @@ void SmallVectorTemplateBase<T, TriviallyCopyable>::takeAllocationForGrow(
     free(this->begin());
 
   this->BeginX = NewElts;
-  this->Capacity = NewCapacity;
+  this->Capacity = static_cast<SmallVectorSizeType<T>>(NewCapacity);
 }
 
 /// SmallVectorTemplateBase<TriviallyCopyable = true> - This is where we put

>From a254543df36d22c79d3c824a343c6082fbb4c312 Mon Sep 17 00:00:00 2001
From: "Golubev, Andrey" <andrey.golubev at intel.com>
Date: Mon, 15 Jan 2024 10:53:55 +0000
Subject: [PATCH 2/3] fixup! [LLVM][ADT] Explicitly convert size_t values to
 SmallVector's size type

---
 llvm/include/llvm/ADT/SmallVector.h | 18 ++++++++++++++----
 1 file changed, 14 insertions(+), 4 deletions(-)

diff --git a/llvm/include/llvm/ADT/SmallVector.h b/llvm/include/llvm/ADT/SmallVector.h
index 65e862911522a8..b1d447a22df411 100644
--- a/llvm/include/llvm/ADT/SmallVector.h
+++ b/llvm/include/llvm/ADT/SmallVector.h
@@ -99,9 +99,19 @@ template <class Size_T> class SmallVectorBase {
   ///
   /// This does not construct or destroy any elements in the vector.
   void set_size(size_t N) {
-    auto n = static_cast<Size_T>(N);
-    assert(n <= capacity());
-    Size = n;
+    assert(N <= capacity());
+    Size = static_cast<Size_T>(N);
+    assert(size() == N);
+  }
+
+  /// Set the array capacity to \p M, which the current array size must
+  /// not be greater than.
+  ///
+  /// This does not construct or destroy any elements in the vector.
+  void set_capacity(size_t M) {
+    assert(size() <= M);
+    Capacity = static_cast<Size_T>(M);
+    assert(capacity() == M);
   }
 };
 
@@ -469,7 +479,7 @@ void SmallVectorTemplateBase<T, TriviallyCopyable>::takeAllocationForGrow(
     free(this->begin());
 
   this->BeginX = NewElts;
-  this->Capacity = static_cast<SmallVectorSizeType<T>>(NewCapacity);
+  this->set_capacity(NewCapacity);
 }
 
 /// SmallVectorTemplateBase<TriviallyCopyable = true> - This is where we put

>From 9eebfbcb9d3538bb2b062350c7ae3276462ec51c Mon Sep 17 00:00:00 2001
From: "Golubev, Andrey" <andrey.golubev at intel.com>
Date: Wed, 17 Jan 2024 14:47:06 +0000
Subject: [PATCH 3/3] fixup #2! [LLVM][ADT] Explicitly convert size_t values to
 SmallVector's size type

---
 llvm/include/llvm/ADT/SmallVector.h | 16 +++++++---------
 llvm/lib/Support/SmallVector.cpp    |  3 +--
 2 files changed, 8 insertions(+), 11 deletions(-)

diff --git a/llvm/include/llvm/ADT/SmallVector.h b/llvm/include/llvm/ADT/SmallVector.h
index b1d447a22df411..f7a2dc51e26291 100644
--- a/llvm/include/llvm/ADT/SmallVector.h
+++ b/llvm/include/llvm/ADT/SmallVector.h
@@ -101,17 +101,16 @@ template <class Size_T> class SmallVectorBase {
   void set_size(size_t N) {
     assert(N <= capacity());
     Size = static_cast<Size_T>(N);
-    assert(size() == N);
   }
 
-  /// Set the array capacity to \p M, which the current array size must
-  /// not be greater than.
+  /// Set the array data pointer to \p Begin and capacity to \p N.
   ///
   /// This does not construct or destroy any elements in the vector.
-  void set_capacity(size_t M) {
-    assert(size() <= M);
-    Capacity = static_cast<Size_T>(M);
-    assert(capacity() == M);
+  //  This does not clean up any existing allocation.
+  void set_allocation_range(void *Begin, size_t N) {
+    assert(N <= SizeTypeMax());
+    BeginX = Begin;
+    Capacity = static_cast<Size_T>(N);
   }
 };
 
@@ -478,8 +477,7 @@ void SmallVectorTemplateBase<T, TriviallyCopyable>::takeAllocationForGrow(
   if (!this->isSmall())
     free(this->begin());
 
-  this->BeginX = NewElts;
-  this->set_capacity(NewCapacity);
+  this->set_allocation_range(NewElts, NewCapacity);
 }
 
 /// SmallVectorTemplateBase<TriviallyCopyable = true> - This is where we put
diff --git a/llvm/lib/Support/SmallVector.cpp b/llvm/lib/Support/SmallVector.cpp
index f7e7e80332cc33..b6ce37842040b3 100644
--- a/llvm/lib/Support/SmallVector.cpp
+++ b/llvm/lib/Support/SmallVector.cpp
@@ -153,8 +153,7 @@ void SmallVectorBase<Size_T>::grow_pod(void *FirstEl, size_t MinSize,
       NewElts = replaceAllocation(NewElts, TSize, NewCapacity, size());
   }
 
-  this->BeginX = NewElts;
-  this->Capacity = NewCapacity;
+  this->set_allocation_range(NewElts, NewCapacity);
 }
 
 template class llvm::SmallVectorBase<uint32_t>;



More information about the llvm-commits mailing list