Skip to content

[lldb] Streamline ConstString -> std::string conversion (NFC) #79649

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

Merged
merged 1 commit into from
Jan 26, 2024

Conversation

JDevlieghere
Copy link
Member

Make it easier to go from a ConstString to a std::string without having to go through a C-String or a llvm::StringRef. I made the conversion operator explicit as this is a relatively expensive operations (compared to a StringRef or string_view).

@llvmbot
Copy link
Member

llvmbot commented Jan 26, 2024

@llvm/pr-subscribers-lldb

Author: Jonas Devlieghere (JDevlieghere)

Changes

Make it easier to go from a ConstString to a std::string without having to go through a C-String or a llvm::StringRef. I made the conversion operator explicit as this is a relatively expensive operations (compared to a StringRef or string_view).


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

2 Files Affected:

  • (modified) lldb/include/lldb/Utility/ConstString.h (+13-2)
  • (modified) lldb/unittests/Utility/ConstStringTest.cpp (+14)
diff --git a/lldb/include/lldb/Utility/ConstString.h b/lldb/include/lldb/Utility/ConstString.h
index cbea4cbf916a430..0ee3f0184b0d08c 100644
--- a/lldb/include/lldb/Utility/ConstString.h
+++ b/lldb/include/lldb/Utility/ConstString.h
@@ -167,8 +167,14 @@ class ConstString {
 
   // Implicitly convert \class ConstString instances to \class StringRef.
   operator llvm::StringRef() const { return GetStringRef(); }
-  // Implicitly convert \class ConstString instances to \calss std::string_view.
-  operator std::string_view() const { return std::string_view(m_string, GetLength()); }
+
+  // Implicitly convert \class ConstString instances to \class std::string_view.
+  operator std::string_view() const {
+    return std::string_view(m_string, GetLength());
+  }
+
+  // Explicitly convert \class ConstString instances to \class std::string.
+  explicit operator std::string() const { return GetString(); }
 
   /// Get the string value as a C string.
   ///
@@ -192,6 +198,11 @@ class ConstString {
     return llvm::StringRef(m_string, GetLength());
   }
 
+  /// Get the string value as a std::string
+  std::string GetString() const {
+    return std::string(m_string, GetLength());
+  }
+
   /// Get the string value as a C string.
   ///
   /// Get the value of the contained string as a NULL terminated C string
diff --git a/lldb/unittests/Utility/ConstStringTest.cpp b/lldb/unittests/Utility/ConstStringTest.cpp
index 9affa927570a5c7..716f2d8d6c80428 100644
--- a/lldb/unittests/Utility/ConstStringTest.cpp
+++ b/lldb/unittests/Utility/ConstStringTest.cpp
@@ -137,3 +137,17 @@ TEST(ConstStringTest, CompareStringRef) {
   EXPECT_TRUE(null == static_cast<const char *>(nullptr));
   EXPECT_TRUE(null != "bar");
 }
+
+TEST(ConstStringTest, StringConversions) {
+  ConstString foo("foo");
+
+  // Member functions.
+  EXPECT_EQ(llvm::StringRef("foo"), foo.GetStringRef());
+  EXPECT_EQ(std::string("foo"), foo.GetString());
+  EXPECT_STREQ("foo", foo.AsCString());
+
+  // Conversion operators.
+  EXPECT_EQ(llvm::StringRef("foo"), llvm::StringRef(foo));
+  EXPECT_EQ(std::string("foo"), std::string_view(foo));
+  EXPECT_EQ(std::string("foo"), std::string(foo));
+}

Copy link

github-actions bot commented Jan 26, 2024

✅ With the latest revision this PR passed the C/C++ code formatter.

Make it easier to go from a ConstString to a std::string without having
to go through a C-String or a llvm::StringRef. I made the conversion
operator explicit as this is a relatively expensive operations (compared
to a StringRef or string_view).
@@ -192,6 +198,9 @@ class ConstString {
return llvm::StringRef(m_string, GetLength());
}

/// Get the string value as a std::string
std::string GetString() const { return std::string(m_string, GetLength()); }
Copy link
Contributor

Choose a reason for hiding this comment

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

why is this necessary, if we have the explicit operator?

Copy link
Member Author

Choose a reason for hiding this comment

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

It's not necessary, it's more for consistency with how we already do things (similar to how StringRef has the explicit operator and the .str() method on the llvm side).

@JDevlieghere JDevlieghere merged commit 33860b2 into llvm:main Jan 26, 2024
@JDevlieghere JDevlieghere deleted the make-adrian-happy branch January 26, 2024 23:04
adrian-prantl pushed a commit to adrian-prantl/llvm-project that referenced this pull request Feb 1, 2024
…9649)

Make it easier to go from a ConstString to a std::string without having
to go through a C-String or a llvm::StringRef. I made the conversion
operator explicit as this is a relatively expensive operations (compared
to a StringRef or string_view).

(cherry picked from commit 33860b2)
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants