Skip to content

[ADT] Declare replaceAllocation in SmallVector.cpp (NFC) #107469

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Conversation

kazutakahirata
Copy link
Contributor

This patch changes replaceAllocation to a static function while moving
the declaration to SmallVector.cpp. Note that:

  • replaceAllocation is used only within SmallVector.cpp.
  • replaceAllocation doesn't access any class members.

This patch changes replaceAllocation to a static function while moving
the declaration to SmallVector.cpp.  Note that:

- replaceAllocation is used only within SmallVector.cpp.
- replaceAllocation doesn't access any class members.
@llvmbot
Copy link
Member

llvmbot commented Sep 5, 2024

@llvm/pr-subscribers-llvm-adt

Author: Kazu Hirata (kazutakahirata)

Changes

This patch changes replaceAllocation to a static function while moving
the declaration to SmallVector.cpp. Note that:

  • replaceAllocation is used only within SmallVector.cpp.
  • replaceAllocation doesn't access any class members.

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

2 Files Affected:

  • (modified) llvm/include/llvm/ADT/SmallVector.h (-13)
  • (modified) llvm/lib/Support/SmallVector.cpp (+12-4)
diff --git a/llvm/include/llvm/ADT/SmallVector.h b/llvm/include/llvm/ADT/SmallVector.h
index 730f84ca038d60..bd3e887e36bce4 100644
--- a/llvm/include/llvm/ADT/SmallVector.h
+++ b/llvm/include/llvm/ADT/SmallVector.h
@@ -74,19 +74,6 @@ template <class Size_T> class SmallVectorBase {
   /// This function will report a fatal error if it cannot increase capacity.
   void grow_pod(void *FirstEl, size_t MinSize, size_t TSize);
 
-  /// If vector was first created with capacity 0, getFirstEl() points to the
-  /// memory right after, an area unallocated. If a subsequent allocation,
-  /// that grows the vector, happens to return the same pointer as getFirstEl(),
-  /// get a new allocation, otherwise isSmall() will falsely return that no
-  /// allocation was done (true) and the memory will not be freed in the
-  /// destructor. If a VSize is given (vector size), also copy that many
-  /// elements to the new allocation - used if realloca fails to increase
-  /// space, and happens to allocate precisely at BeginX.
-  /// This is unlikely to be called often, but resolves a memory leak when the
-  /// situation does occur.
-  void *replaceAllocation(void *NewElts, size_t TSize, size_t NewCapacity,
-                          size_t VSize = 0);
-
 public:
   size_t size() const { return Size; }
   size_t capacity() const { return Capacity; }
diff --git a/llvm/lib/Support/SmallVector.cpp b/llvm/lib/Support/SmallVector.cpp
index b6ce37842040b3..dceea4fbc630e8 100644
--- a/llvm/lib/Support/SmallVector.cpp
+++ b/llvm/lib/Support/SmallVector.cpp
@@ -108,10 +108,18 @@ static size_t getNewCapacity(size_t MinSize, size_t TSize, size_t OldCapacity) {
   return std::clamp(NewCapacity, MinSize, MaxSize);
 }
 
-template <class Size_T>
-void *SmallVectorBase<Size_T>::replaceAllocation(void *NewElts, size_t TSize,
-                                                 size_t NewCapacity,
-                                                 size_t VSize) {
+/// If vector was first created with capacity 0, getFirstEl() points to the
+/// memory right after, an area unallocated. If a subsequent allocation,
+/// that grows the vector, happens to return the same pointer as getFirstEl(),
+/// get a new allocation, otherwise isSmall() will falsely return that no
+/// allocation was done (true) and the memory will not be freed in the
+/// destructor. If a VSize is given (vector size), also copy that many
+/// elements to the new allocation - used if realloca fails to increase
+/// space, and happens to allocate precisely at BeginX.
+/// This is unlikely to be called often, but resolves a memory leak when the
+/// situation does occur.
+static void *replaceAllocation(void *NewElts, size_t TSize, size_t NewCapacity,
+                               size_t VSize = 0) {
   void *NewEltsReplace = llvm::safe_malloc(NewCapacity * TSize);
   if (VSize)
     memcpy(NewEltsReplace, NewElts, VSize * TSize);

@llvmbot
Copy link
Member

llvmbot commented Sep 5, 2024

@llvm/pr-subscribers-llvm-support

Author: Kazu Hirata (kazutakahirata)

Changes

This patch changes replaceAllocation to a static function while moving
the declaration to SmallVector.cpp. Note that:

  • replaceAllocation is used only within SmallVector.cpp.
  • replaceAllocation doesn't access any class members.

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

2 Files Affected:

  • (modified) llvm/include/llvm/ADT/SmallVector.h (-13)
  • (modified) llvm/lib/Support/SmallVector.cpp (+12-4)
diff --git a/llvm/include/llvm/ADT/SmallVector.h b/llvm/include/llvm/ADT/SmallVector.h
index 730f84ca038d60..bd3e887e36bce4 100644
--- a/llvm/include/llvm/ADT/SmallVector.h
+++ b/llvm/include/llvm/ADT/SmallVector.h
@@ -74,19 +74,6 @@ template <class Size_T> class SmallVectorBase {
   /// This function will report a fatal error if it cannot increase capacity.
   void grow_pod(void *FirstEl, size_t MinSize, size_t TSize);
 
-  /// If vector was first created with capacity 0, getFirstEl() points to the
-  /// memory right after, an area unallocated. If a subsequent allocation,
-  /// that grows the vector, happens to return the same pointer as getFirstEl(),
-  /// get a new allocation, otherwise isSmall() will falsely return that no
-  /// allocation was done (true) and the memory will not be freed in the
-  /// destructor. If a VSize is given (vector size), also copy that many
-  /// elements to the new allocation - used if realloca fails to increase
-  /// space, and happens to allocate precisely at BeginX.
-  /// This is unlikely to be called often, but resolves a memory leak when the
-  /// situation does occur.
-  void *replaceAllocation(void *NewElts, size_t TSize, size_t NewCapacity,
-                          size_t VSize = 0);
-
 public:
   size_t size() const { return Size; }
   size_t capacity() const { return Capacity; }
diff --git a/llvm/lib/Support/SmallVector.cpp b/llvm/lib/Support/SmallVector.cpp
index b6ce37842040b3..dceea4fbc630e8 100644
--- a/llvm/lib/Support/SmallVector.cpp
+++ b/llvm/lib/Support/SmallVector.cpp
@@ -108,10 +108,18 @@ static size_t getNewCapacity(size_t MinSize, size_t TSize, size_t OldCapacity) {
   return std::clamp(NewCapacity, MinSize, MaxSize);
 }
 
-template <class Size_T>
-void *SmallVectorBase<Size_T>::replaceAllocation(void *NewElts, size_t TSize,
-                                                 size_t NewCapacity,
-                                                 size_t VSize) {
+/// If vector was first created with capacity 0, getFirstEl() points to the
+/// memory right after, an area unallocated. If a subsequent allocation,
+/// that grows the vector, happens to return the same pointer as getFirstEl(),
+/// get a new allocation, otherwise isSmall() will falsely return that no
+/// allocation was done (true) and the memory will not be freed in the
+/// destructor. If a VSize is given (vector size), also copy that many
+/// elements to the new allocation - used if realloca fails to increase
+/// space, and happens to allocate precisely at BeginX.
+/// This is unlikely to be called often, but resolves a memory leak when the
+/// situation does occur.
+static void *replaceAllocation(void *NewElts, size_t TSize, size_t NewCapacity,
+                               size_t VSize = 0) {
   void *NewEltsReplace = llvm::safe_malloc(NewCapacity * TSize);
   if (VSize)
     memcpy(NewEltsReplace, NewElts, VSize * TSize);

@kazutakahirata kazutakahirata merged commit 169d453 into llvm:main Sep 5, 2024
8 of 10 checks passed
@kazutakahirata kazutakahirata deleted the cleanup_001_SmallVector_replaceAllocation branch September 5, 2024 22:47
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants