[libcxx-commits] [libcxxabi] 1cea7ab - [demangler] Use standard semantics for StringView::substr

[Mark de Wever via libcxx-commits]

Author: Tomasz Miąsko
Date: 2021-04-25T13:56:41+02:00
New Revision: 1cea7ab4ba13346745d6aa6e117ee2e77995fb07

URL: https://github.com/llvm/llvm-project/commit/1cea7ab4ba13346745d6aa6e117ee2e77995fb07
DIFF: https://github.com/llvm/llvm-project/commit/1cea7ab4ba13346745d6aa6e117ee2e77995fb07.diff

LOG: [demangler] Use standard semantics for StringView::substr

The StringView::substr now accepts a substring starting position and its
length instead of previous non-standard `from` & `to` positions.

All uses of two argument StringView::substr are in MicrosoftDemangler
and have 0 as a starting position, so no changes are necessary.

This also fixes a bug where attempting to extract a suffix with substr
(a `to` position equal to size) would return a substring without the
last character.

Fixing the issue should not introduce observable changes in the
demangler, since as currently used, a second argument to
StringView::substr is either: 1) a result of a successful call to
StringView::find and so necessarily smaller than size., or 2) in the
case of Demangler::demangleCharLiteral potentially equal to size, but
with demangler expecting more data to follow later on and failing either
way.

Reviewed By: #libc_abi, ldionne, erik.pilkington

Differential Revision: https://reviews.llvm.org/D100246

Added: 
    llvm/unittests/Demangle/StringViewTest.cpp

Modified: 
    libcxxabi/src/demangle/StringView.h
    llvm/include/llvm/Demangle/StringView.h
    llvm/unittests/Demangle/CMakeLists.txt

Removed: 
    


################################################################################
diff  --git a/libcxxabi/src/demangle/StringView.h b/libcxxabi/src/demangle/StringView.h
index ceb6c7958066..1e4d3803f06c 100644
--- a/libcxxabi/src/demangle/StringView.h
+++ b/libcxxabi/src/demangle/StringView.h
@@ -36,8 +36,9 @@ class StringView {
   StringView(const char *Str) : First(Str), Last(Str + std::strlen(Str)) {}
   StringView() : First(nullptr), Last(nullptr) {}
 
-  StringView substr(size_t From) const {
-    return StringView(begin() + From, size() - From);
+  StringView substr(size_t Pos, size_t Len = npos) const {
+    assert(Pos <= size());
+    return StringView(begin() + Pos, std::min(Len, size() - Pos));
   }
 
   size_t find(char C, size_t From = 0) const {
@@ -51,14 +52,6 @@ class StringView {
     return npos;
   }
 
-  StringView substr(size_t From, size_t To) const {
-    if (To >= size())
-      To = size() - 1;
-    if (From >= size())
-      From = size() - 1;
-    return StringView(First + From, First + To);
-  }
-
   StringView dropFront(size_t N = 1) const {
     if (N >= size())
       N = size();

diff  --git a/llvm/include/llvm/Demangle/StringView.h b/llvm/include/llvm/Demangle/StringView.h
index 2e260969ce9a..378e85341637 100644
--- a/llvm/include/llvm/Demangle/StringView.h
+++ b/llvm/include/llvm/Demangle/StringView.h
@@ -36,8 +36,9 @@ class StringView {
   StringView(const char *Str) : First(Str), Last(Str + std::strlen(Str)) {}
   StringView() : First(nullptr), Last(nullptr) {}
 
-  StringView substr(size_t From) const {
-    return StringView(begin() + From, size() - From);
+  StringView substr(size_t Pos, size_t Len = npos) const {
+    assert(Pos <= size());
+    return StringView(begin() + Pos, std::min(Len, size() - Pos));
   }
 
   size_t find(char C, size_t From = 0) const {
@@ -51,14 +52,6 @@ class StringView {
     return npos;
   }
 
-  StringView substr(size_t From, size_t To) const {
-    if (To >= size())
-      To = size() - 1;
-    if (From >= size())
-      From = size() - 1;
-    return StringView(First + From, First + To);
-  }
-
   StringView dropFront(size_t N = 1) const {
     if (N >= size())
       N = size();

diff  --git a/llvm/unittests/Demangle/CMakeLists.txt b/llvm/unittests/Demangle/CMakeLists.txt
index c6291bb2a980..8db2595f39c9 100644
--- a/llvm/unittests/Demangle/CMakeLists.txt
+++ b/llvm/unittests/Demangle/CMakeLists.txt
@@ -7,4 +7,5 @@ add_llvm_unittest(DemangleTests
   DemangleTest.cpp
   ItaniumDemangleTest.cpp
   PartialDemangleTest.cpp
+  StringViewTest.cpp
 )

diff  --git a/llvm/unittests/Demangle/StringViewTest.cpp b/llvm/unittests/Demangle/StringViewTest.cpp
new file mode 100644
index 000000000000..8eaaaf62a4a4
--- /dev/null
+++ b/llvm/unittests/Demangle/StringViewTest.cpp
@@ -0,0 +1,48 @@
+//===- llvm/unittest/StringViewTest.cpp - StringView unit tests -----------===//
+//
+// Part of the LLVM Project, under the Apache License v2.0 with LLVM Exceptions.
+// See https://llvm.org/LICENSE.txt for license information.
+// SPDX-License-Identifier: Apache-2.0 WITH LLVM-exception
+//
+//===----------------------------------------------------------------------===//
+
+#include "llvm/Demangle/StringView.h"
+#include "gtest/gtest.h"
+
+using namespace llvm;
+using llvm::itanium_demangle::StringView;
+
+namespace llvm {
+namespace itanium_demangle {
+
+std::ostream &operator<<(std::ostream &OS, const StringView &S) {
+  return OS.write(S.begin(), S.size());
+}
+
+} // namespace itanium_demangle
+} // namespace llvm
+
+TEST(StringViewTest, EmptyInitializerList) {
+  StringView S = {};
+  EXPECT_TRUE(S.empty());
+
+  S = {};
+  EXPECT_TRUE(S.empty());
+}
+
+TEST(StringViewTest, Substr) {
+  StringView S("abcdef");
+
+  EXPECT_EQ("abcdef", S.substr(0));
+  EXPECT_EQ("f", S.substr(5));
+  EXPECT_EQ("", S.substr(6));
+
+  EXPECT_EQ("", S.substr(0, 0));
+  EXPECT_EQ("a", S.substr(0, 1));
+  EXPECT_EQ("abcde", S.substr(0, 5));
+  EXPECT_EQ("abcdef", S.substr(0, 6));
+  EXPECT_EQ("abcdef", S.substr(0, 7));
+
+  EXPECT_EQ("f", S.substr(5, 100));
+  EXPECT_EQ("", S.substr(6, 100));
+}