From bf2f5e70dd4035e265714d87c69d602b80e563be Mon Sep 17 00:00:00 2001 From: cyy Date: Thu, 30 May 2024 21:13:17 +0000 Subject: [PATCH] Fix warnings in SmallVector (#127250) Fixes #ISSUE_NUMBER Pull Request resolved: https://github.com/pytorch/pytorch/pull/127250 Approved by: https://github.com/ezyang --- c10/test/util/small_vector_test.cpp | 8 +++---- c10/util/SmallVector.cpp | 2 +- c10/util/SmallVector.h | 33 +++++++++++------------------ 3 files changed, 17 insertions(+), 26 deletions(-) diff --git a/c10/test/util/small_vector_test.cpp b/c10/test/util/small_vector_test.cpp index e05d21ce88f1..1efe4d4910e0 100644 --- a/c10/test/util/small_vector_test.cpp +++ b/c10/test/util/small_vector_test.cpp @@ -576,8 +576,8 @@ TYPED_TEST(SmallVectorTest, EraseTest) { SCOPED_TRACE("EraseTest"); this->makeSequence(this->theVector, 1, 3); - const auto& theConstVector = this->theVector; - this->theVector.erase(theConstVector.begin()); + auto& theVector = this->theVector; + this->theVector.erase(theVector.begin()); this->assertValuesInOrder(this->theVector, 2u, 2, 3); } @@ -586,8 +586,8 @@ TYPED_TEST(SmallVectorTest, EraseRangeTest) { SCOPED_TRACE("EraseRangeTest"); this->makeSequence(this->theVector, 1, 3); - const auto& theConstVector = this->theVector; - this->theVector.erase(theConstVector.begin(), theConstVector.begin() + 2); + auto& theVector = this->theVector; + this->theVector.erase(theVector.begin(), theVector.begin() + 2); this->assertValuesInOrder(this->theVector, 1u, 3); } diff --git a/c10/util/SmallVector.cpp b/c10/util/SmallVector.cpp index 14b2fa9eb671..e30cdbf8dd3b 100644 --- a/c10/util/SmallVector.cpp +++ b/c10/util/SmallVector.cpp @@ -123,7 +123,7 @@ void* SmallVectorBase::mallocForGrow( // Note: Moving this function into the header may cause performance regression. template void SmallVectorBase::grow_pod( - void* FirstEl, + const void* FirstEl, size_t MinSize, size_t TSize) { size_t NewCapacity = getNewCapacity(MinSize, TSize, this->capacity()); diff --git a/c10/util/SmallVector.h b/c10/util/SmallVector.h index 919553811454..cbcfbc52cb8a 100644 --- a/c10/util/SmallVector.h +++ b/c10/util/SmallVector.h @@ -38,11 +38,6 @@ #include #include -C10_CLANG_DIAGNOSTIC_PUSH() -#if C10_CLANG_HAS_WARNING("-Wshorten-64-to-32") -C10_CLANG_DIAGNOSTIC_IGNORE("-Wshorten-64-to-32") -#endif - namespace c10 { /// This is all the stuff common to all SmallVectors. @@ -75,7 +70,7 @@ class C10_API SmallVectorBase { /// This is an implementation of the grow() method which only works /// on POD-like data types and is out of line to reduce code duplication. /// This function will report a fatal error if it cannot increase capacity. - void grow_pod(void* FirstEl, size_t MinSize, size_t TSize); + void grow_pod(const void* FirstEl, size_t MinSize, size_t TSize); public: SmallVectorBase() = delete; @@ -112,8 +107,10 @@ using SmallVectorSizeType = /// Figure out the offset of the first element. template struct SmallVectorAlignmentAndSize { + // NOLINTNEXTLINE(*c-arrays*) alignas(SmallVectorBase>) char Base[sizeof( SmallVectorBase>)]; + // NOLINTNEXTLINE(*c-arrays*) alignas(T) char FirstEl[sizeof(T)]; }; @@ -246,7 +243,7 @@ class SmallVectorTemplateCommon bool ReferencesStorage = false; int64_t Index = -1; - if (!U::TakesParamByValue) { + if constexpr (!U::TakesParamByValue) { if (C10_UNLIKELY(This->isReferenceToStorage(&Elt))) { ReferencesStorage = true; Index = &Elt - This->begin(); @@ -306,7 +303,7 @@ class SmallVectorTemplateCommon size_type size_in_bytes() const { return size() * sizeof(T); } - size_type max_size() const { + constexpr size_type max_size() const { return std::min(this->SizeTypeMax(), size_type(-1) / sizeof(T)); } @@ -475,6 +472,7 @@ class SmallVectorTemplateBase : public SmallVectorTemplateCommon { this->set_size(this->size() + 1); } + // NOLINTNEXTLINE(cppcoreguidelines-rvalue-reference-param-not-moved) void push_back(T&& Elt) { T* EltPtr = reserveForParamAndGetAddress(Elt); ::new ((void*)this->end()) T(::std::move(*EltPtr)); @@ -788,13 +786,9 @@ class SmallVectorImpl : public SmallVectorTemplateBase { assign(RHS.begin(), RHS.end()); } - iterator erase(const_iterator CI) { - // Just cast away constness because this is a non-const member function. - iterator I = const_cast(CI); - + iterator erase(iterator I) { assert( - this->isReferenceToStorage(CI) && - "Iterator to erase is out of bounds."); + this->isReferenceToStorage(I) && "Iterator to erase is out of bounds."); iterator N = I; // Shift all elts down one. @@ -804,11 +798,7 @@ class SmallVectorImpl : public SmallVectorTemplateBase { return (N); } - iterator erase(const_iterator CS, const_iterator CE) { - // Just cast away constness because this is a non-const member function. - iterator S = const_cast(CS); - iterator E = const_cast(CE); - + iterator erase(iterator S, iterator E) { assert(this->isRangeInStorage(S, E) && "Range to erase is out of bounds."); iterator N = S; @@ -1402,6 +1392,7 @@ class /* LLVM_GSL_OWNER */ SmallVector : public SmallVectorImpl, .end())>::iterator_category, std::input_iterator_tag>, int> = 0> + // NOLINTNEXTLINE(cppcoreguidelines-missing-std-forward) SmallVector& operator=(Container&& C) { this->assign(C.begin(), C.end()); return *this; @@ -1439,6 +1430,7 @@ using ValueTypeFromRangeType = std::remove_const_t< /// SmallVector with elements of the vector. This is useful, for example, /// when you want to iterate a range and then sort the results. template +// NOLINTNEXTLINE(cppcoreguidelines-missing-std-forward) SmallVector, Size> to_vector(R&& Range) { return {std::begin(Range), std::end(Range)}; } @@ -1447,6 +1439,7 @@ SmallVector< ValueTypeFromRangeType, CalculateSmallVectorDefaultInlinedElements< ValueTypeFromRangeType>::value> +// NOLINTNEXTLINE(cppcoreguidelines-missing-std-forward) to_vector(R&& Range) { return {std::begin(Range), std::end(Range)}; } @@ -1472,5 +1465,3 @@ inline void swap( } } // end namespace std - -C10_CLANG_DIAGNOSTIC_POP()