Skip to content
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

[libc++] Remove abandoned __append declaration in vector<bool> #121673

Merged
merged 1 commit into from
Jan 5, 2025

Conversation

winner245
Copy link
Contributor

@winner245 winner245 commented Jan 5, 2025

The vector<bool> implementation in libcxx contains a declaration of a private __append function, which is neither defined nor used anywhere in the codebase. This PR aims to remove this abandoned declaration, as its presence is misleading and could lead to confusion during future maintenance.

I have no idea why we have a declaration without a definition. My guess is that the declaration might be inherited from the implementation of vector<T>, where __append is both necessary and properly defined. The declaration may have been inadvertently copied from vector<T> to vector<bool> and subsequently abandoned, as vector<bool> never needs it.

@winner245 winner245 changed the title [libc++] Remove redundant __append declaration in vector<bool> [libc++] Remove abandoned __append declaration in vector<bool> Jan 5, 2025
@winner245 winner245 marked this pull request as ready for review January 5, 2025 13:03
@winner245 winner245 requested a review from a team as a code owner January 5, 2025 13:03
@llvmbot llvmbot added the libc++ libc++ C++ Standard Library. Not GNU libstdc++. Not libc++abi. label Jan 5, 2025
@llvmbot
Copy link
Member

llvmbot commented Jan 5, 2025

@llvm/pr-subscribers-libcxx

Author: Peng Liu (winner245)

Changes

The vector&lt;bool&gt; implementation in libcxx contains a declaration of a private __append function, which is neither defined nor used anywhere in the codebase. This PR aims to remove this abandoned declaration, as its presence is misleading and could lead to confusion during future maintenance.

I have no idea why we have a declaration without a definition. My guess is that the declaration might be inherited from the implementation of vector&lt;T&gt;, where __append is both necessary and properly defined. The declaration may have been inadvertently copied from vector&lt;T&gt; to vector&lt;bool&gt; and subsequently abandoned, as vector&lt;bool&gt; never needs it.


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

1 Files Affected:

  • (modified) libcxx/include/__vector/vector_bool.h (-1)
diff --git a/libcxx/include/__vector/vector_bool.h b/libcxx/include/__vector/vector_bool.h
index 525fc35b26cc9e..8658745b8a8f9e 100644
--- a/libcxx/include/__vector/vector_bool.h
+++ b/libcxx/include/__vector/vector_bool.h
@@ -442,7 +442,6 @@ class _LIBCPP_TEMPLATE_VIS vector<bool, _Allocator> {
   template <class _InputIterator, class _Sentinel>
   _LIBCPP_HIDE_FROM_ABI _LIBCPP_CONSTEXPR_SINCE_CXX20 void
   __construct_at_end(_InputIterator __first, _Sentinel __last, size_type __n);
-  _LIBCPP_HIDE_FROM_ABI _LIBCPP_CONSTEXPR_SINCE_CXX20 void __append(size_type __n, const_reference __x);
   _LIBCPP_HIDE_FROM_ABI _LIBCPP_CONSTEXPR_SINCE_CXX20 reference __make_ref(size_type __pos) _NOEXCEPT {
     return reference(__begin_ + __pos / __bits_per_word, __storage_type(1) << __pos % __bits_per_word);
   }

Copy link
Contributor

@philnik777 philnik777 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Looks like it's been there since the initial import and was never defined.

@philnik777 philnik777 merged commit 1d15541 into llvm:main Jan 5, 2025
66 checks passed
@winner245 winner245 deleted the remove-unused-declaration branch January 5, 2025 13:11
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
libc++ libc++ C++ Standard Library. Not GNU libstdc++. Not libc++abi.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants