[demangler] Use standard semantics for StringView::substr

[tmiasko]

Summary

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.

[ldionne]

This LGTM at a glance, but I'd like Erik to give his thumbs up.

[erik.pilkington]

LGTM as well, thanks!

[tmiasko]

Could you commit this for me? Thank you.

[tmiasko]

Ping. I don't have commit access.

[Mordante]

Can you provide your name and email address so we can commit the patch on your behalf?

[tmiasko]

Tomasz Miąsko <[e-mail]>

libcxxabi/src/demangle/StringView.h

public:
   StringView(const char (&Str)[N]) : First(Str), Last(Str + N - 1) {}
   StringView(const char *First_, const char *Last_)
       : First(First_), Last(Last_) {}
   StringView(const char *First_, size_t Len)
       : First(First_), Last(First_ + Len) {}
   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 {
     size_t FindBegin = std::min(From, size());
     // Avoid calling memchr with nullptr.
     if (FindBegin < size()) {
       // Just forward to memchr, which is faster than a hand-rolled loop.
       if (const void *P = ::memchr(First + FindBegin, C, size() - FindBegin))
         return size_t(static_cast<const char *>(P) - First);
     }
     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();
     return StringView(First + N, Last);
   }
 
   StringView dropBack(size_t N = 1) const {
     if (N >= size())

llvm/include/llvm/Demangle/StringView.h

public:
   StringView(const char (&Str)[N]) : First(Str), Last(Str + N - 1) {}
   StringView(const char *First_, const char *Last_)
       : First(First_), Last(Last_) {}
   StringView(const char *First_, size_t Len)
       : First(First_), Last(First_ + Len) {}
   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 {
     size_t FindBegin = std::min(From, size());
     // Avoid calling memchr with nullptr.
     if (FindBegin < size()) {
       // Just forward to memchr, which is faster than a hand-rolled loop.
       if (const void *P = ::memchr(First + FindBegin, C, size() - FindBegin))
         return size_t(static_cast<const char *>(P) - First);
     }
     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();
     return StringView(First + N, Last);
   }
 
   StringView dropBack(size_t N = 1) const {
     if (N >= size())

llvm/unittests/Demangle/CMakeLists.txt

 set(LLVM_LINK_COMPONENTS
   Demangle
   Support
 )
 
 add_llvm_unittest(DemangleTests
   DemangleTest.cpp
   ItaniumDemangleTest.cpp
   PartialDemangleTest.cpp
+  StringViewTest.cpp
 )

llvm/unittests/Demangle/StringViewTest.cpp

+//===- 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));
+}