Allow direct comparison of ConstString against StringRef

Summary:
When we want to compare a ConstString against a string literal (or any other non-ConstString),
we currently have to explicitly turn the other string into a ConstString. This makes sense as
comparing ConstStrings against each other is only a fast pointer comparison.

However, currently we (rather incorrectly) use in several places in LLDB temporary ConstStrings when
we just want to compare a given ConstString against a hardcoded value, for example like this:
```
if (extension != ConstString(".oat") && extension != ConstString(".odex"))
```

Obviously this kind of defeats the point of ConstStrings. In the comparison above we would
construct two temporary ConstStrings every time we hit the given code. Constructing a
ConstString is relatively expensive: we need to go to the StringPool, take a read and possibly
an exclusive write-lock and then look up our temporary string in the string map of the pool.
So we do a lot of heavy work for essentially just comparing a <6 characters in two strings.

I initially wanted to just fix these issues by turning the temporary ConstString in static variables/
members, but that made the code much less readable. Instead I propose to add a new overload
for the ConstString comparison operator that takes a StringRef. This comparison operator directly
compares the ConstString content against the given StringRef without turning the StringRef into
a ConstString.

This means that the example above can look like this now:
```
if (extension != ".oat" && extension != ".odex")
```
It also no longer has to unlock/lock two locks and call multiple functions in other TUs for constructing
the temporary ConstString instances. Instead this should end up just being a direct string comparison
of the two given strings on most compilers.

This patch also directly updates all uses of temporary and short ConstStrings in LLDB to use this new
comparison operator. It also adds a some unit tests for the new and old comparison operator.

Reviewers: #lldb, JDevlieghere, espindola, amccarth

Reviewed By: JDevlieghere, amccarth

Subscribers: amccarth, clayborg, JDevlieghere, emaste, arichardson, MaskRay, lldb-commits

Tags: #lldb

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

llvm-svn: 359281

14 files changed, 28 insertions, 32 deletions

diff --git a/lldb/source/Plugins/DynamicLoader/MacOSX-DYLD/DynamicLoaderDarwin.cpp b/lldb/source/Plugins/DynamicLoader/MacOSX-DYLD/DynamicLoaderDarwin.cpp
index 39440fdcb63..7475b1a87aa 100644
--- a/lldb/source/Plugins/DynamicLoader/MacOSX-DYLD/DynamicLoaderDarwin.cpp
+++ b/lldb/source/Plugins/DynamicLoader/MacOSX-DYLD/DynamicLoaderDarwin.cpp
@@ -474,7 +474,7 @@ bool DynamicLoaderDarwin::JSONImageInformationIntoImageInfo(
       // 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.
diff --git a/lldb/source/Plugins/DynamicLoader/MacOSX-DYLD/DynamicLoaderMacOS.cpp b/lldb/source/Plugins/DynamicLoader/MacOSX-DYLD/DynamicLoaderMacOS.cpp
index 53a9371696c..29e4ac0654a 100644
--- a/lldb/source/Plugins/DynamicLoader/MacOSX-DYLD/DynamicLoaderMacOS.cpp
+++ b/lldb/source/Plugins/DynamicLoader/MacOSX-DYLD/DynamicLoaderMacOS.cpp
@@ -111,7 +111,7 @@ bool DynamicLoaderMacOS::ProcessDidExec() {
             const Symbol *symbol =
                 frame_sp->GetSymbolContext(eSymbolContextSymbol).symbol;
             if (symbol) {
-              if (symbol->GetName() == ConstString("_dyld_start"))
+              if (symbol->GetName() == "_dyld_start")
                 did_exec = true;
             }
           }
diff --git a/lldb/source/Plugins/DynamicLoader/MacOSX-DYLD/DynamicLoaderMacOSXDYLD.cpp b/lldb/source/Plugins/DynamicLoader/MacOSX-DYLD/DynamicLoaderMacOSXDYLD.cpp
index f4c50b888d4..9c0125a6db0 100644
--- a/lldb/source/Plugins/DynamicLoader/MacOSX-DYLD/DynamicLoaderMacOSXDYLD.cpp
+++ b/lldb/source/Plugins/DynamicLoader/MacOSX-DYLD/DynamicLoaderMacOSXDYLD.cpp
@@ -136,7 +136,7 @@ bool DynamicLoaderMacOSXDYLD::ProcessDidExec() {
             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 @@ uint32_t DynamicLoaderMacOSXDYLD::ParseLoadCommands(const DataExtractor &data,
     // 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;
diff --git a/lldb/source/Plugins/ExpressionParser/Clang/ClangExpressionSourceCode.cpp b/lldb/source/Plugins/ExpressionParser/Clang/ClangExpressionSourceCode.cpp
index 5be376ea3fa..2fc690e3d5b 100644
--- a/lldb/source/Plugins/ExpressionParser/Clang/ClangExpressionSourceCode.cpp
+++ b/lldb/source/Plugins/ExpressionParser/Clang/ClangExpressionSourceCode.cpp
@@ -167,8 +167,7 @@ static void AddLocalVariableDecls(const lldb::VariableListSP &var_list_sp,
     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());
diff --git a/lldb/source/Plugins/ExpressionParser/Clang/ClangUserExpression.cpp b/lldb/source/Plugins/ExpressionParser/Clang/ClangUserExpression.cpp
index 34cfb8951be..2e64b088539 100644
--- a/lldb/source/Plugins/ExpressionParser/Clang/ClangUserExpression.cpp
+++ b/lldb/source/Plugins/ExpressionParser/Clang/ClangUserExpression.cpp
@@ -488,7 +488,7 @@ ClangUserExpression::GetModulesToImport(ExecutionContext &exe_ctx) {
   // 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 {};
diff --git a/lldb/source/Plugins/Language/CPlusPlus/LibCxx.cpp b/lldb/source/Plugins/Language/CPlusPlus/LibCxx.cpp
index e63452eb95e..9dceca6123f 100644
--- a/lldb/source/Plugins/Language/CPlusPlus/LibCxx.cpp
+++ b/lldb/source/Plugins/Language/CPlusPlus/LibCxx.cpp
@@ -300,9 +300,9 @@ bool lldb_private::formatters::LibCxxMapIteratorSyntheticFrontEnd::
 
 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 @@ bool lldb_private::formatters::LibcxxSharedPtrSyntheticFrontEnd::
 
 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;
 }
diff --git a/lldb/source/Plugins/Language/CPlusPlus/LibCxxVector.cpp b/lldb/source/Plugins/Language/CPlusPlus/LibCxxVector.cpp
index b7ed597d48a..bcd7442bc66 100644
--- a/lldb/source/Plugins/Language/CPlusPlus/LibCxxVector.cpp
+++ b/lldb/source/Plugins/Language/CPlusPlus/LibCxxVector.cpp
@@ -290,7 +290,7 @@ lldb_private::formatters::LibcxxStdVectorSyntheticFrontEndCreator(
   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);
 }
diff --git a/lldb/source/Plugins/Language/CPlusPlus/LibStdcpp.cpp b/lldb/source/Plugins/Language/CPlusPlus/LibStdcpp.cpp
index 0aac93c3c76..0e0f6663c92 100644
--- a/lldb/source/Plugins/Language/CPlusPlus/LibStdcpp.cpp
+++ b/lldb/source/Plugins/Language/CPlusPlus/LibStdcpp.cpp
@@ -142,9 +142,9 @@ bool LibstdcppMapIteratorSyntheticFrontEnd::MightHaveChildren() { return true; }
 
 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 @@ bool VectorIteratorSyntheticFrontEnd::MightHaveChildren() { return true; }
 
 size_t VectorIteratorSyntheticFrontEnd::GetIndexOfChildWithName(
     ConstString name) {
-  if (name == ConstString("item"))
+  if (name == "item")
     return 0;
   return UINT32_MAX;
 }
@@ -374,7 +374,7 @@ bool LibStdcppSharedPtrSyntheticFrontEnd::MightHaveChildren() { return true; }
 
 size_t LibStdcppSharedPtrSyntheticFrontEnd::GetIndexOfChildWithName(
     ConstString name) {
-  if (name == ConstString("_M_ptr"))
+  if (name == "_M_ptr")
     return 0;
   return UINT32_MAX;
 }
diff --git a/lldb/source/Plugins/Language/CPlusPlus/LibStdcppUniquePointer.cpp b/lldb/source/Plugins/Language/CPlusPlus/LibStdcppUniquePointer.cpp
index 451e58f7796..3860f960cb3 100644
--- a/lldb/source/Plugins/Language/CPlusPlus/LibStdcppUniquePointer.cpp
+++ b/lldb/source/Plugins/Language/CPlusPlus/LibStdcppUniquePointer.cpp
@@ -130,12 +130,11 @@ size_t LibStdcppUniquePtrSyntheticFrontEnd::CalculateNumChildren() {
 
 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;
 }
diff --git a/lldb/source/Plugins/Language/ObjC/CF.cpp b/lldb/source/Plugins/Language/ObjC/CF.cpp
index 99a880f3fe6..7db55e15b5c 100644
--- a/lldb/source/Plugins/Language/ObjC/CF.cpp
+++ b/lldb/source/Plugins/Language/ObjC/CF.cpp
@@ -139,10 +139,8 @@ bool lldb_private::formatters::CFBitVectorSummaryProvider(
   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;
     }
diff --git a/lldb/source/Plugins/LanguageRuntime/ObjC/AppleObjCRuntime/AppleObjCRuntime.cpp b/lldb/source/Plugins/LanguageRuntime/ObjC/AppleObjCRuntime/AppleObjCRuntime.cpp
index 419f209068d..8e7014dcd61 100644
--- a/lldb/source/Plugins/LanguageRuntime/ObjC/AppleObjCRuntime/AppleObjCRuntime.cpp
+++ b/lldb/source/Plugins/LanguageRuntime/ObjC/AppleObjCRuntime/AppleObjCRuntime.cpp
@@ -472,7 +472,8 @@ ValueObjectSP AppleObjCRuntime::GetExceptionObjectForThread(
   
   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();
   }
 
diff --git a/lldb/source/Plugins/ObjectFile/ELF/ObjectFileELF.cpp b/lldb/source/Plugins/ObjectFile/ELF/ObjectFileELF.cpp
index ac9642b2968..d41d9722fa2 100644
--- a/lldb/source/Plugins/ObjectFile/ELF/ObjectFileELF.cpp
+++ b/lldb/source/Plugins/ObjectFile/ELF/ObjectFileELF.cpp
@@ -1996,8 +1996,8 @@ unsigned ObjectFileELF::ParseSymbols(Symtab *symtab, user_id_t start_id,
   // 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());
diff --git a/lldb/source/Plugins/Platform/Android/PlatformAndroid.cpp b/lldb/source/Plugins/Platform/Android/PlatformAndroid.cpp
index 1e9c840a241..3b8c9206ca3 100644
--- a/lldb/source/Plugins/Platform/Android/PlatformAndroid.cpp
+++ b/lldb/source/Plugins/Platform/Android/PlatformAndroid.cpp
@@ -302,7 +302,7 @@ Status PlatformAndroid::DownloadSymbolFile(const lldb::ModuleSP &module_sp,
                                            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");
 
diff --git a/lldb/source/Plugins/SystemRuntime/MacOSX/SystemRuntimeMacOSX.cpp b/lldb/source/Plugins/SystemRuntime/MacOSX/SystemRuntimeMacOSX.cpp
index 6b81ec8d3a7..82be85699b2 100644
--- a/lldb/source/Plugins/SystemRuntime/MacOSX/SystemRuntimeMacOSX.cpp
+++ b/lldb/source/Plugins/SystemRuntime/MacOSX/SystemRuntimeMacOSX.cpp
@@ -454,8 +454,7 @@ void SystemRuntimeMacOSX::ReadLibdispatchTSDIndexes() {
 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 @@ ThreadSP
 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;