[clang] ba5628f - ADT: Use 'using' to inherit assign and append in SmallString
Duncan P. N. Exon Smith via cfe-commits
cfe-commits at lists.llvm.org
Fri Jan 22 16:18:13 PST 2021
Author: Duncan P. N. Exon Smith
Date: 2021-01-22T16:17:58-08:00
New Revision: ba5628f2c2a9de049b80b3e276f7e05f481c49e7
URL: https://github.com/llvm/llvm-project/commit/ba5628f2c2a9de049b80b3e276f7e05f481c49e7
DIFF: https://github.com/llvm/llvm-project/commit/ba5628f2c2a9de049b80b3e276f7e05f481c49e7.diff
LOG: ADT: Use 'using' to inherit assign and append in SmallString
Rather than reimplement, use a `using` declaration to bring in
`SmallVectorImpl<char>`'s assign and append implementations in
`SmallString`.
The `SmallString` versions were missing reference invalidation
assertions from `SmallVector`. This patch also fixes a bug in
`llvm::FileCollector::addFileImpl`, which was a copy/paste from
`clang::ModuleDependencyCollector::copyToRoot`, both caught by the
no-longer-skipped assertions.
As a drive-by, this also sinks the `const SmallVectorImpl&` versions of
these methods down into `SmallVectorImpl`, since I imagine they'd be
useful elsewhere.
Differential Revision: https://reviews.llvm.org/D95202
Added:
Modified:
clang/lib/Frontend/ModuleDependencyCollector.cpp
llvm/include/llvm/ADT/SmallString.h
llvm/include/llvm/ADT/SmallVector.h
llvm/lib/Support/FileCollector.cpp
llvm/unittests/ADT/SmallVectorTest.cpp
Removed:
################################################################################
diff --git a/clang/lib/Frontend/ModuleDependencyCollector.cpp b/clang/lib/Frontend/ModuleDependencyCollector.cpp
index b54eb97d6c47..2b981224c7e0 100644
--- a/clang/lib/Frontend/ModuleDependencyCollector.cpp
+++ b/clang/lib/Frontend/ModuleDependencyCollector.cpp
@@ -190,17 +190,17 @@ std::error_code ModuleDependencyCollector::copyToRoot(StringRef Src,
// Canonicalize src to a native path to avoid mixed separator styles.
path::native(AbsoluteSrc);
// Remove redundant leading "./" pieces and consecutive separators.
- AbsoluteSrc = path::remove_leading_dotslash(AbsoluteSrc);
+ StringRef TrimmedAbsoluteSrc = path::remove_leading_dotslash(AbsoluteSrc);
// Canonicalize the source path by removing "..", "." components.
- SmallString<256> VirtualPath = AbsoluteSrc;
+ SmallString<256> VirtualPath = TrimmedAbsoluteSrc;
path::remove_dots(VirtualPath, /*remove_dot_dot=*/true);
// If a ".." component is present after a symlink component, remove_dots may
// lead to the wrong real destination path. Let the source be canonicalized
// like that but make sure we always use the real path for the destination.
SmallString<256> CopyFrom;
- if (!getRealPath(AbsoluteSrc, CopyFrom))
+ if (!getRealPath(TrimmedAbsoluteSrc, CopyFrom))
CopyFrom = VirtualPath;
SmallString<256> CacheDst = getDest();
diff --git a/llvm/include/llvm/ADT/SmallString.h b/llvm/include/llvm/ADT/SmallString.h
index c0e8fcd29461..5a56321ae492 100644
--- a/llvm/include/llvm/ADT/SmallString.h
+++ b/llvm/include/llvm/ADT/SmallString.h
@@ -40,35 +40,15 @@ class SmallString : public SmallVector<char, InternalLen> {
template<typename ItTy>
SmallString(ItTy S, ItTy E) : SmallVector<char, InternalLen>(S, E) {}
- // Note that in order to add new overloads for append & assign, we have to
- // duplicate the inherited versions so as not to inadvertently hide them.
-
/// @}
/// @name String Assignment
/// @{
- /// Assign from a repeated element.
- void assign(size_t NumElts, char Elt) {
- this->SmallVectorImpl<char>::assign(NumElts, Elt);
- }
-
- /// Assign from an iterator pair.
- template<typename in_iter>
- void assign(in_iter S, in_iter E) {
- this->clear();
- SmallVectorImpl<char>::append(S, E);
- }
+ using SmallVector<char, InternalLen>::assign;
/// Assign from a StringRef.
void assign(StringRef RHS) {
- this->clear();
- SmallVectorImpl<char>::append(RHS.begin(), RHS.end());
- }
-
- /// Assign from a SmallVector.
- void assign(const SmallVectorImpl<char> &RHS) {
- this->clear();
- SmallVectorImpl<char>::append(RHS.begin(), RHS.end());
+ SmallVectorImpl<char>::assign(RHS.begin(), RHS.end());
}
/// Assign from a list of StringRefs.
@@ -81,26 +61,13 @@ class SmallString : public SmallVector<char, InternalLen> {
/// @name String Concatenation
/// @{
- /// Append from an iterator pair.
- template<typename in_iter>
- void append(in_iter S, in_iter E) {
- SmallVectorImpl<char>::append(S, E);
- }
-
- void append(size_t NumInputs, char Elt) {
- SmallVectorImpl<char>::append(NumInputs, Elt);
- }
+ using SmallVector<char, InternalLen>::append;
/// Append from a StringRef.
void append(StringRef RHS) {
SmallVectorImpl<char>::append(RHS.begin(), RHS.end());
}
- /// Append from a SmallVector.
- void append(const SmallVectorImpl<char> &RHS) {
- SmallVectorImpl<char>::append(RHS.begin(), RHS.end());
- }
-
/// Append from a list of StringRefs.
void append(std::initializer_list<StringRef> Refs) {
size_t SizeNeeded = this->size();
diff --git a/llvm/include/llvm/ADT/SmallVector.h b/llvm/include/llvm/ADT/SmallVector.h
index dd72937c19e2..e960b272db04 100644
--- a/llvm/include/llvm/ADT/SmallVector.h
+++ b/llvm/include/llvm/ADT/SmallVector.h
@@ -664,6 +664,8 @@ class SmallVectorImpl : public SmallVectorTemplateBase<T> {
append(IL.begin(), IL.end());
}
+ void append(const SmallVectorImpl &RHS) { append(RHS.begin(), RHS.end()); }
+
void assign(size_type NumElts, ValueParamT Elt) {
// Note that Elt could be an internal reference.
if (NumElts > this->capacity()) {
@@ -698,6 +700,8 @@ class SmallVectorImpl : public SmallVectorTemplateBase<T> {
append(IL);
}
+ void assign(const SmallVectorImpl &RHS) { 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<iterator>(CI);
diff --git a/llvm/lib/Support/FileCollector.cpp b/llvm/lib/Support/FileCollector.cpp
index d0471ac8b66b..cb53878b7905 100644
--- a/llvm/lib/Support/FileCollector.cpp
+++ b/llvm/lib/Support/FileCollector.cpp
@@ -86,17 +86,18 @@ void FileCollector::addFileImpl(StringRef SrcPath) {
sys::path::native(AbsoluteSrc);
// Remove redundant leading "./" pieces and consecutive separators.
- AbsoluteSrc = sys::path::remove_leading_dotslash(AbsoluteSrc);
+ StringRef TrimmedAbsoluteSrc =
+ sys::path::remove_leading_dotslash(AbsoluteSrc);
// Canonicalize the source path by removing "..", "." components.
- SmallString<256> VirtualPath = AbsoluteSrc;
+ SmallString<256> VirtualPath = TrimmedAbsoluteSrc;
sys::path::remove_dots(VirtualPath, /*remove_dot_dot=*/true);
// If a ".." component is present after a symlink component, remove_dots may
// lead to the wrong real destination path. Let the source be canonicalized
// like that but make sure we always use the real path for the destination.
SmallString<256> CopyFrom;
- if (!getRealPath(AbsoluteSrc, CopyFrom))
+ if (!getRealPath(TrimmedAbsoluteSrc, CopyFrom))
CopyFrom = VirtualPath;
SmallString<256> DstPath = StringRef(Root);
diff --git a/llvm/unittests/ADT/SmallVectorTest.cpp b/llvm/unittests/ADT/SmallVectorTest.cpp
index b2cccc1a1e56..a533bb850a45 100644
--- a/llvm/unittests/ADT/SmallVectorTest.cpp
+++ b/llvm/unittests/ADT/SmallVectorTest.cpp
@@ -485,6 +485,15 @@ TYPED_TEST(SmallVectorTest, AppendRepeatedNonForwardIterator) {
this->assertValuesInOrder(this->theVector, 3u, 1, 7, 7);
}
+TYPED_TEST(SmallVectorTest, AppendSmallVector) {
+ SCOPED_TRACE("AppendSmallVector");
+
+ SmallVector<Constructable, 3> otherVector = {7, 7};
+ this->theVector.push_back(Constructable(1));
+ this->theVector.append(otherVector);
+ this->assertValuesInOrder(this->theVector, 3u, 1, 7, 7);
+}
+
// Assign test
TYPED_TEST(SmallVectorTest, AssignTest) {
SCOPED_TRACE("AssignTest");
@@ -513,6 +522,15 @@ TYPED_TEST(SmallVectorTest, AssignNonIterTest) {
this->assertValuesInOrder(this->theVector, 2u, 7, 7);
}
+TYPED_TEST(SmallVectorTest, AssignSmallVector) {
+ SCOPED_TRACE("AssignSmallVector");
+
+ SmallVector<Constructable, 3> otherVector = {7, 7};
+ this->theVector.push_back(Constructable(1));
+ this->theVector.assign(otherVector);
+ this->assertValuesInOrder(this->theVector, 2u, 7, 7);
+}
+
// Move-assign test
TYPED_TEST(SmallVectorTest, MoveAssignTest) {
SCOPED_TRACE("MoveAssignTest");
More information about the cfe-commits
mailing list