teemperor updated this revision to Diff 195310.
teemperor removed a reviewer: espindola.
teemperor added a comment.
Herald added a reviewer: espindola.
- Made empty/nullptr check more readable.
- Removed some uses of the new comparison operator for cases where we don't
have a literal to compare against (which makes it hard to estimate if this is
really better).
- Reworded documentation so that the length of the string doesn't really matter
getting a performance benefit, only the fact that whether ConstString we pass
is temporary or not.
CHANGES SINCE LAST ACTION
https://reviews.llvm.org/D60667/new/
https://reviews.llvm.org/D60667
Files:
lldb/include/lldb/Utility/ConstString.h
lldb/source/Plugins/DynamicLoader/MacOSX-DYLD/DynamicLoaderDarwin.cpp
lldb/source/Plugins/DynamicLoader/MacOSX-DYLD/DynamicLoaderMacOS.cpp
lldb/source/Plugins/DynamicLoader/MacOSX-DYLD/DynamicLoaderMacOSXDYLD.cpp
lldb/source/Plugins/ExpressionParser/Clang/ClangExpressionDeclMap.cpp
lldb/source/Plugins/ExpressionParser/Clang/ClangExpressionSourceCode.cpp
lldb/source/Plugins/ExpressionParser/Clang/ClangUserExpression.cpp
lldb/source/Plugins/Language/CPlusPlus/LibCxx.cpp
lldb/source/Plugins/Language/CPlusPlus/LibCxxVector.cpp
lldb/source/Plugins/Language/CPlusPlus/LibStdcpp.cpp
lldb/source/Plugins/Language/CPlusPlus/LibStdcppUniquePointer.cpp
lldb/source/Plugins/Language/ObjC/CF.cpp
lldb/source/Plugins/LanguageRuntime/ObjC/AppleObjCRuntime/AppleObjCRuntime.cpp
lldb/source/Plugins/LanguageRuntime/RenderScript/RenderScriptRuntime/RenderScriptRuntime.cpp
lldb/source/Plugins/ObjectFile/ELF/ObjectFileELF.cpp
lldb/source/Plugins/Platform/Android/PlatformAndroid.cpp
lldb/source/Plugins/SystemRuntime/MacOSX/SystemRuntimeMacOSX.cpp
lldb/unittests/Utility/ConstStringTest.cpp
Index: lldb/unittests/Utility/ConstStringTest.cpp
===================================================================
--- lldb/unittests/Utility/ConstStringTest.cpp
+++ lldb/unittests/Utility/ConstStringTest.cpp
@@ -89,3 +89,51 @@
EXPECT_TRUE(null.IsEmpty());
EXPECT_TRUE(null.IsNull());
}
+
+TEST(ConstStringTest, CompareConstString) {
+ ConstString foo("foo");
+ ConstString foo2("foo");
+ ConstString bar("bar");
+
+ EXPECT_TRUE(foo == foo2);
+ EXPECT_TRUE(foo2 == foo);
+ EXPECT_TRUE(foo == ConstString("foo"));
+
+ EXPECT_FALSE(foo == bar);
+ EXPECT_FALSE(foo2 == bar);
+ EXPECT_FALSE(foo == ConstString("bar"));
+ EXPECT_FALSE(foo == ConstString("different"));
+ EXPECT_FALSE(foo == ConstString(""));
+ EXPECT_FALSE(foo == ConstString());
+
+ ConstString empty("");
+ EXPECT_FALSE(empty == ConstString("bar"));
+ EXPECT_FALSE(empty == ConstString());
+ EXPECT_TRUE(empty == ConstString(""));
+
+ ConstString null;
+ EXPECT_FALSE(null == ConstString("bar"));
+ EXPECT_TRUE(null == ConstString());
+ EXPECT_FALSE(null == ConstString(""));
+}
+
+TEST(ConstStringTest, CompareStringRef) {
+ ConstString foo("foo");
+
+ EXPECT_TRUE(foo == "foo");
+ EXPECT_TRUE(foo != "");
+ EXPECT_FALSE(foo == static_cast<const char *>(nullptr));
+ EXPECT_TRUE(foo != "bar");
+
+ ConstString empty("");
+ EXPECT_FALSE(empty == "foo");
+ EXPECT_FALSE(empty != "");
+ EXPECT_FALSE(empty == static_cast<const char *>(nullptr));
+ EXPECT_TRUE(empty != "bar");
+
+ ConstString null;
+ EXPECT_FALSE(null == "foo");
+ EXPECT_TRUE(null != "");
+ EXPECT_TRUE(null == static_cast<const char *>(nullptr));
+ EXPECT_TRUE(null != "bar");
+}
Index: lldb/source/Plugins/SystemRuntime/MacOSX/SystemRuntimeMacOSX.cpp
===================================================================
--- lldb/source/Plugins/SystemRuntime/MacOSX/SystemRuntimeMacOSX.cpp
+++ lldb/source/Plugins/SystemRuntime/MacOSX/SystemRuntimeMacOSX.cpp
@@ -454,8 +454,7 @@
ThreadSP SystemRuntimeMacOSX::GetExtendedBacktraceThread(ThreadSP real_thread,
ConstString type) {
ThreadSP originating_thread_sp;
- if (BacktraceRecordingHeadersInitialized() &&
- type == ConstString("libdispatch")) {
+ if (BacktraceRecordingHeadersInitialized() && type == "libdispatch") {
Status error;
// real_thread is either an actual, live thread (in which case we need to
@@ -554,7 +553,7 @@
SystemRuntimeMacOSX::GetExtendedBacktraceForQueueItem(QueueItemSP queue_item_sp,
ConstString type) {
ThreadSP extended_thread_sp;
- if (type != ConstString("libdispatch"))
+ if (type != "libdispatch")
return extended_thread_sp;
bool stop_id_is_valid = true;
Index: lldb/source/Plugins/Platform/Android/PlatformAndroid.cpp
===================================================================
--- lldb/source/Plugins/Platform/Android/PlatformAndroid.cpp
+++ lldb/source/Plugins/Platform/Android/PlatformAndroid.cpp
@@ -302,7 +302,7 @@
const FileSpec &dst_file_spec) {
// For oat file we can try to fetch additional debug info from the device
ConstString extension = module_sp->GetFileSpec().GetFileNameExtension();
- if (extension != ConstString(".oat") && extension != ConstString(".odex"))
+ if (extension != ".oat" && extension != ".odex")
return Status(
"Symbol file downloading only supported for oat and odex files");
Index: lldb/source/Plugins/ObjectFile/ELF/ObjectFileELF.cpp
===================================================================
--- lldb/source/Plugins/ObjectFile/ELF/ObjectFileELF.cpp
+++ lldb/source/Plugins/ObjectFile/ELF/ObjectFileELF.cpp
@@ -1668,7 +1668,7 @@
if (!name || !name[0] || !ParseSectionHeaders())
return 0;
for (size_t i = 1; i < m_section_headers.size(); ++i)
- if (m_section_headers[i].section_name == ConstString(name))
+ if (m_section_headers[i].section_name == name)
return i;
return 0;
}
@@ -1996,8 +1996,8 @@
// custom extension and file name makes it highly unlikely that this will
// collide with anything else.
ConstString file_extension = m_file.GetFileNameExtension();
- bool skip_oatdata_oatexec = file_extension == ConstString(".oat") ||
- file_extension == ConstString(".odex");
+ bool skip_oatdata_oatexec =
+ file_extension == ".oat" || file_extension == ".odex";
ArchSpec arch = GetArchitecture();
ModuleSP module_sp(GetModule());
Index: lldb/source/Plugins/LanguageRuntime/RenderScript/RenderScriptRuntime/RenderScriptRuntime.cpp
===================================================================
--- lldb/source/Plugins/LanguageRuntime/RenderScript/RenderScriptRuntime/RenderScriptRuntime.cpp
+++ lldb/source/Plugins/LanguageRuntime/RenderScript/RenderScriptRuntime/RenderScriptRuntime.cpp
@@ -1685,7 +1685,7 @@
continue;
// Only proceed if the module that has loaded corresponds to this script.
- if (file.GetFilename() != ConstString(shared_lib.c_str()))
+ if (file.GetFilename() != shared_lib.c_str())
continue;
// Obtain the script address which we use as a key.
Index: lldb/source/Plugins/LanguageRuntime/ObjC/AppleObjCRuntime/AppleObjCRuntime.cpp
===================================================================
--- lldb/source/Plugins/LanguageRuntime/ObjC/AppleObjCRuntime/AppleObjCRuntime.cpp
+++ lldb/source/Plugins/LanguageRuntime/ObjC/AppleObjCRuntime/AppleObjCRuntime.cpp
@@ -472,7 +472,8 @@
while (descriptor) {
ConstString class_name(descriptor->GetClassName());
- if (class_name == ConstString("NSException")) return cpp_exception;
+ if (class_name == "NSException")
+ return cpp_exception;
descriptor = descriptor->GetSuperclass();
}
Index: lldb/source/Plugins/Language/ObjC/CF.cpp
===================================================================
--- lldb/source/Plugins/Language/ObjC/CF.cpp
+++ lldb/source/Plugins/Language/ObjC/CF.cpp
@@ -139,10 +139,8 @@
bool is_type_ok = false; // check to see if this is a CFBag we know about
if (descriptor->IsCFType()) {
ConstString type_name(valobj.GetTypeName());
- if (type_name == ConstString("__CFMutableBitVector") ||
- type_name == ConstString("__CFBitVector") ||
- type_name == ConstString("CFMutableBitVectorRef") ||
- type_name == ConstString("CFBitVectorRef")) {
+ if (type_name == "__CFMutableBitVector" || type_name == "__CFBitVector" ||
+ type_name == "CFMutableBitVectorRef" || type_name == "CFBitVectorRef") {
if (valobj.IsPointerType())
is_type_ok = true;
}
Index: lldb/source/Plugins/Language/CPlusPlus/LibStdcppUniquePointer.cpp
===================================================================
--- lldb/source/Plugins/Language/CPlusPlus/LibStdcppUniquePointer.cpp
+++ lldb/source/Plugins/Language/CPlusPlus/LibStdcppUniquePointer.cpp
@@ -130,12 +130,11 @@
size_t LibStdcppUniquePtrSyntheticFrontEnd::GetIndexOfChildWithName(
ConstString name) {
- if (name == ConstString("ptr") || name == ConstString("pointer"))
+ if (name == "ptr" || name == "pointer")
return 0;
- if (name == ConstString("del") || name == ConstString("deleter"))
+ if (name == "del" || name == "deleter")
return 1;
- if (name == ConstString("obj") || name == ConstString("object") ||
- name == ConstString("$$dereference$$"))
+ if (name == "obj" || name == "object" || name == "$$dereference$$")
return 2;
return UINT32_MAX;
}
Index: lldb/source/Plugins/Language/CPlusPlus/LibStdcpp.cpp
===================================================================
--- lldb/source/Plugins/Language/CPlusPlus/LibStdcpp.cpp
+++ lldb/source/Plugins/Language/CPlusPlus/LibStdcpp.cpp
@@ -142,9 +142,9 @@
size_t LibstdcppMapIteratorSyntheticFrontEnd::GetIndexOfChildWithName(
ConstString name) {
- if (name == ConstString("first"))
+ if (name == "first")
return 0;
- if (name == ConstString("second"))
+ if (name == "second")
return 1;
return UINT32_MAX;
}
@@ -224,7 +224,7 @@
size_t VectorIteratorSyntheticFrontEnd::GetIndexOfChildWithName(
ConstString name) {
- if (name == ConstString("item"))
+ if (name == "item")
return 0;
return UINT32_MAX;
}
@@ -374,7 +374,7 @@
size_t LibStdcppSharedPtrSyntheticFrontEnd::GetIndexOfChildWithName(
ConstString name) {
- if (name == ConstString("_M_ptr"))
+ if (name == "_M_ptr")
return 0;
return UINT32_MAX;
}
Index: lldb/source/Plugins/Language/CPlusPlus/LibCxxVector.cpp
===================================================================
--- lldb/source/Plugins/Language/CPlusPlus/LibCxxVector.cpp
+++ lldb/source/Plugins/Language/CPlusPlus/LibCxxVector.cpp
@@ -290,7 +290,7 @@
if (!type.IsValid() || type.GetNumTemplateArguments() == 0)
return nullptr;
CompilerType arg_type = type.GetTypeTemplateArgument(0);
- if (arg_type.GetTypeName() == ConstString("bool"))
+ if (arg_type.GetTypeName() == "bool")
return new LibcxxVectorBoolSyntheticFrontEnd(valobj_sp);
return new LibcxxStdVectorSyntheticFrontEnd(valobj_sp);
}
Index: lldb/source/Plugins/Language/CPlusPlus/LibCxx.cpp
===================================================================
--- lldb/source/Plugins/Language/CPlusPlus/LibCxx.cpp
+++ lldb/source/Plugins/Language/CPlusPlus/LibCxx.cpp
@@ -300,9 +300,9 @@
size_t lldb_private::formatters::LibCxxMapIteratorSyntheticFrontEnd::
GetIndexOfChildWithName(ConstString name) {
- if (name == ConstString("first"))
+ if (name == "first")
return 0;
- if (name == ConstString("second"))
+ if (name == "second")
return 1;
return UINT32_MAX;
}
@@ -430,11 +430,11 @@
size_t lldb_private::formatters::LibcxxSharedPtrSyntheticFrontEnd::
GetIndexOfChildWithName(ConstString name) {
- if (name == ConstString("__ptr_"))
+ if (name == "__ptr_")
return 0;
- if (name == ConstString("count"))
+ if (name == "count")
return 1;
- if (name == ConstString("weak_count"))
+ if (name == "weak_count")
return 2;
return UINT32_MAX;
}
Index: lldb/source/Plugins/ExpressionParser/Clang/ClangUserExpression.cpp
===================================================================
--- lldb/source/Plugins/ExpressionParser/Clang/ClangUserExpression.cpp
+++ lldb/source/Plugins/ExpressionParser/Clang/ClangUserExpression.cpp
@@ -489,7 +489,7 @@
// We currently don't support importing any other modules in the expression
// parser.
for (const SourceModule &m : sc.comp_unit->GetImportedModules())
- if (!m.path.empty() && m.path.front() == ConstString("std"))
+ if (!m.path.empty() && m.path.front() == "std")
return {"std"};
return {};
Index: lldb/source/Plugins/ExpressionParser/Clang/ClangExpressionSourceCode.cpp
===================================================================
--- lldb/source/Plugins/ExpressionParser/Clang/ClangExpressionSourceCode.cpp
+++ lldb/source/Plugins/ExpressionParser/Clang/ClangExpressionSourceCode.cpp
@@ -167,8 +167,7 @@
lldb::VariableSP var_sp = var_list_sp->GetVariableAtIndex(i);
ConstString var_name = var_sp->GetName();
- if (!var_name || var_name == ConstString("this") ||
- var_name == ConstString(".block_descriptor"))
+ if (!var_name || var_name == "this" || var_name == ".block_descriptor")
continue;
stream.Printf("using $__lldb_local_vars::%s;\n", var_name.AsCString());
Index: lldb/source/Plugins/ExpressionParser/Clang/ClangExpressionDeclMap.cpp
===================================================================
--- lldb/source/Plugins/ExpressionParser/Clang/ClangExpressionDeclMap.cpp
+++ lldb/source/Plugins/ExpressionParser/Clang/ClangExpressionDeclMap.cpp
@@ -1170,7 +1170,7 @@
return;
}
- if (name == ConstString(g_lldb_local_vars_namespace_cstr)) {
+ if (name == g_lldb_local_vars_namespace_cstr) {
CompilerDeclContext frame_decl_context =
sym_ctx.block != nullptr ? sym_ctx.block->GetDeclContext()
: CompilerDeclContext();
Index: lldb/source/Plugins/DynamicLoader/MacOSX-DYLD/DynamicLoaderMacOSXDYLD.cpp
===================================================================
--- lldb/source/Plugins/DynamicLoader/MacOSX-DYLD/DynamicLoaderMacOSXDYLD.cpp
+++ lldb/source/Plugins/DynamicLoader/MacOSX-DYLD/DynamicLoaderMacOSXDYLD.cpp
@@ -136,7 +136,7 @@
const Symbol *symbol =
frame_sp->GetSymbolContext(eSymbolContextSymbol).symbol;
if (symbol) {
- if (symbol->GetName() == ConstString("_dyld_start"))
+ if (symbol->GetName() == "_dyld_start")
did_exec = true;
}
}
@@ -898,7 +898,7 @@
// starts of file offset zero and that has bytes in the file...
if ((dylib_info.segments[i].fileoff == 0 &&
dylib_info.segments[i].filesize > 0) ||
- (dylib_info.segments[i].name == ConstString("__TEXT"))) {
+ (dylib_info.segments[i].name == "__TEXT")) {
dylib_info.slide = dylib_info.address - dylib_info.segments[i].vmaddr;
// We have found the slide amount, so we can exit this for loop.
break;
Index: lldb/source/Plugins/DynamicLoader/MacOSX-DYLD/DynamicLoaderMacOS.cpp
===================================================================
--- lldb/source/Plugins/DynamicLoader/MacOSX-DYLD/DynamicLoaderMacOS.cpp
+++ lldb/source/Plugins/DynamicLoader/MacOSX-DYLD/DynamicLoaderMacOS.cpp
@@ -111,7 +111,7 @@
const Symbol *symbol =
frame_sp->GetSymbolContext(eSymbolContextSymbol).symbol;
if (symbol) {
- if (symbol->GetName() == ConstString("_dyld_start"))
+ if (symbol->GetName() == "_dyld_start")
did_exec = true;
}
}
Index: lldb/source/Plugins/DynamicLoader/MacOSX-DYLD/DynamicLoaderDarwin.cpp
===================================================================
--- lldb/source/Plugins/DynamicLoader/MacOSX-DYLD/DynamicLoaderDarwin.cpp
+++ lldb/source/Plugins/DynamicLoader/MacOSX-DYLD/DynamicLoaderDarwin.cpp
@@ -474,7 +474,7 @@
// that starts of file offset zero and that has bytes in the file...
if ((image_infos[i].segments[k].fileoff == 0 &&
image_infos[i].segments[k].filesize > 0) ||
- (image_infos[i].segments[k].name == ConstString("__TEXT"))) {
+ (image_infos[i].segments[k].name == "__TEXT")) {
image_infos[i].slide =
image_infos[i].address - image_infos[i].segments[k].vmaddr;
// We have found the slide amount, so we can exit this for loop.
Index: lldb/include/lldb/Utility/ConstString.h
===================================================================
--- lldb/include/lldb/Utility/ConstString.h
+++ lldb/include/lldb/Utility/ConstString.h
@@ -154,6 +154,30 @@
return m_string == rhs.m_string;
}
+ /// Equal to operator against a non-ConstString value.
+ ///
+ /// Returns true if this string is equal to the string in \a rhs. This
+ /// overload is usually slower than comparing against a ConstString value.
+ /// However, if the rhs string not already a ConstString and it is impractical
+ /// to turn it into a non-temporary variable, then this overload is faster.
+ ///
+ /// \param[in] rhs
+ /// Another string object to compare this object to.
+ ///
+ /// \return
+ /// \li \b true if this object is equal to \a rhs.
+ /// \li \b false if this object is not equal to \a rhs.
+ bool operator==(const char *rhs) const {
+ // ConstString differentiates between empty strings and nullptr strings, but
+ // StringRef doesn't. Therefore we have to do this check manually now.
+ if (m_string == nullptr && rhs != nullptr)
+ return false;
+ if (m_string != nullptr && rhs == nullptr)
+ return false;
+
+ return GetStringRef() == rhs;
+ }
+
/// Not equal to operator
///
/// Returns true if this string is not equal to the string in \a rhs. This
@@ -170,6 +194,21 @@
return m_string != rhs.m_string;
}
+ /// Not equal to operator against a non-ConstString value.
+ ///
+ /// Returns true if this string is not equal to the string in \a rhs. This
+ /// overload is usually slower than comparing against a ConstString value.
+ /// However, if the rhs string not already a ConstString and it is impractical
+ /// to turn it into a non-temporary variable, then this overload is faster.
+ ///
+ /// \param[in] rhs
+ /// Another string object to compare this object to.
+ ///
+ /// \return
+ /// \li \b true if this object is not equal to \a rhs.
+ /// \li \b false if this object is equal to \a rhs.
+ bool operator!=(const char *rhs) const { return !(*this == rhs); }
+
bool operator<(ConstString rhs) const;
/// Get the string value as a C string.
_______________________________________________
lldb-commits mailing list
[email protected]
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
