Add 'oneOf' utility function for comparing a ConstString against a set of strings

[teemperor]

Summary

We have a few checks in LLDB where we compare a single ConstString against a hardcoded list of strings. This patch introduces a
utility function 'oneOf' in ConstString which is makes this check more readable.

For example, before this patch we had to write this:

if (ft == llvm::sys::fs::file_type::directory_file &&
    (file_spec.GetFileNameExtension() == g_sdk_suffix ||
     file_spec.GetFileNameExtension() == g_kdk_suffix)) {

And after this patch we can now write this:

if (ft == llvm::sys::fs::file_type::directory_file &&
    file_spec.GetFileNameExtension().oneOf(g_sdk_suffix, g_kdk_suffix)) {

[JDevlieghere]

I'm on the fence on this one. I think the == pattern attracts more attention to the fact that we are comparing strings. However, I also see the benefit, like in the example, where even a temporary variable wouldn't help much.

Regarding the name, how would you feel about anyOf instead? Although not relevant here, it has a nice duality with allOf. I think this is what the AST matchers use.

[teemperor]

Yeah, I'm also just like 60% confident that this more readable/expressive.

anyOf sounds good to me. Will update the patch in case this gets community approval.

[labath]

In D61231#1481804, @teemperor wrote:

Yeah, I'm also just like 60% confident that this more readable/expressive.

I'm about 70% against this ( :P ), and the main reason for that opinion is that there is nothing like this in llvm, and if llvm was able to get away without it, then I don't see why shouldn't we be able to do it too. The way these kinds of things are usually done in llvm is via the StringSwitch class, and it seems to me that StringSwitch could be directly used in at least half of the motivating use cases you show here (and in the rest, it could be used after a mild code refactor, which that code needs anyway).

(And yeah, I am aware that going through StringSwitch will mean we lose the fast string comparison that ConstString was meant to provide, but I am sure that this will have no impact on the overall performance here.)

lldb/source/Plugins/Language/CPlusPlus/LibStdcppUniquePointer.cpp

133–139

This is a prime example for a StringSwitch, which is how these kinds of things are usually done in llvm.

return StringSwitch<size_t>(name.GetStringRef()).Cases("ptr, "pointer", 0).Cases("del", "deleter", 1).Cases("obj", "object", "$$dereference$$", 2).Default(UINT32_MAX);

(+ running clang-format over that)

lldb/source/Plugins/Language/ObjC/ObjCLanguage.cpp

1039–1082

Using StringSwitch would make this code around three times shorter.

lldb/include/lldb/Utility/ConstString.h

public:
   ///
   /// \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;
 
+  /// Compares this ConstString against the given set of strings. Returns true
+  /// iff at least one of the given strings is equal to this ConstString.
+  template <typename T, typename... Args>
+  bool oneOf(T first, Args... args) const {
+    return oneOf(first) || oneOf(args...);
+  }
+  template <typename T> bool oneOf(T first) const { return *this == first; }
+
   /// Get the string value as a C string.
   ///
   /// Get the value of the contained string as a NULL terminated C string
   /// value.
   ///
   /// If \a value_if_empty is nullptr, then nullptr will be returned.
   ///
   /// \return

lldb/source/Core/Debugger.cpp

LoadPluginCallback(void *baton, llvm::sys::fs::file_type ft,
   // process the file. We must handle unknown as sometimes the directory
   // enumeration might be enumerating a file system that doesn't have correct
   // file type information.
   if (ft == fs::file_type::regular_file || ft == fs::file_type::symlink_file ||
       ft == fs::file_type::type_unknown) {
     FileSpec plugin_file_spec(path);
     FileSystem::Instance().Resolve(plugin_file_spec);
 
-    if (plugin_file_spec.GetFileNameExtension() != g_dylibext &&
-        plugin_file_spec.GetFileNameExtension() != g_solibext) {
+    if (!plugin_file_spec.GetFileNameExtension().oneOf(g_dylibext,
+                                                       g_solibext)) {
       return FileSystem::eEnumerateDirectoryResultNext;
     }
 
     Status plugin_load_error;
     debugger->LoadPlugin(plugin_file_spec, plugin_load_error);
 
     return FileSystem::eEnumerateDirectoryResultNext;
   } else if (ft == fs::file_type::directory_file ||

lldb/source/Plugins/ExpressionParser/Clang/ClangASTSource.cpp

 }
 
 bool ClangASTSource::IgnoreName(const ConstString name,
                                 bool ignore_all_dollar_names) {
   static const ConstString id_name("id");
   static const ConstString Class_name("Class");
 
   if (m_ast_context->getLangOpts().ObjC)
-    if (name == id_name || name == Class_name)
+    if (name.oneOf(id_name, Class_name))
       return true;
 
   StringRef name_string_ref = name.GetStringRef();
 
   // The ClangASTSource is not responsible for finding $-names.
   return name_string_ref.empty() ||
          (ignore_all_dollar_names && name_string_ref.startswith("$")) ||
          name_string_ref.startswith("_$");

lldb/source/Plugins/ExpressionParser/Clang/ClangExpressionSourceCode.cpp

 }
 
 static void AddLocalVariableDecls(const lldb::VariableListSP &var_list_sp,
                                   StreamString &stream) {
   for (size_t i = 0; i < var_list_sp->GetSize(); i++) {
     lldb::VariableSP var_sp = var_list_sp->GetVariableAtIndex(i);
 
     ConstString var_name = var_sp->GetName();
-    if (!var_name || var_name == "this" || var_name == ".block_descriptor")
+    if (!var_name || var_name.oneOf("this", ".block_descriptor"))
       continue;
 
     stream.Printf("using $__lldb_local_vars::%s;\n", var_name.AsCString());
   }
 }
 
 bool ClangExpressionSourceCode::GetText(std::string &text,
                                    lldb::LanguageType wrapping_language,

lldb/source/Plugins/Language/CPlusPlus/LibStdcppUniquePointer.cpp

 size_t LibStdcppUniquePtrSyntheticFrontEnd::CalculateNumChildren() {
   if (m_del_obj)
     return 2;
   return 1;
 }
 
 size_t LibStdcppUniquePtrSyntheticFrontEnd::GetIndexOfChildWithName(
     ConstString name) {
-  if (name == "ptr" || name == "pointer")
+  if (name.oneOf("ptr", "pointer"))
     return 0;
-  if (name == "del" || name == "deleter")
+  if (name.oneOf("del", "deleter"))
     return 1;
-  if (name == "obj" || name == "object" || name == "$$dereference$$")
+  if (name.oneOf("obj", "object", "$$dereference$$"))
     return 2;
   return UINT32_MAX;
 }
 
 bool LibStdcppUniquePtrSyntheticFrontEnd::GetSummary(
     Stream &stream, const TypeSummaryOptions &options) {
   if (!m_ptr_obj)
     return false;
 
   bool success;

lldb/source/Plugins/Language/ObjC/CF.cpp

bool lldb_private::formatters::CFBitVectorSummaryProvider(
   if (!valobj_addr)
     return false;
 
   uint32_t count = 0;
 
   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 == "__CFMutableBitVector" || type_name == "__CFBitVector" ||
-        type_name == "CFMutableBitVectorRef" || type_name == "CFBitVectorRef") {
+    if (type_name.oneOf("__CFMutableBitVector", "__CFBitVector",
+                        "CFMutableBitVectorRef", "CFBitVectorRef")) {
       if (valobj.IsPointerType())
         is_type_ok = true;
     }
   }
 
   if (!is_type_ok)
     return false;
 

lldb/source/Plugins/Language/ObjC/Cocoa.cpp

bool lldb_private::formatters::NSDateSummaryProvider(
   static const ConstString g___NSDate("__NSDate");
   static const ConstString g___NSTaggedDate("__NSTaggedDate");
   static const ConstString g_NSCalendarDate("NSCalendarDate");
 
   if (class_name.IsEmpty())
     return false;
 
   uint64_t info_bits = 0, value_bits = 0;
-  if ((class_name == g_NSDate) || (class_name == g___NSDate) ||
-      (class_name == g___NSTaggedDate)) {
+  if (class_name.oneOf(g_NSDate, g___NSDate, g___NSTaggedDate)) {
     if (descriptor->GetTaggedPointerInfo(&info_bits, &value_bits)) {
       date_value_bits = ((value_bits << 8) | (info_bits << 4));
       memcpy(&date_value, &date_value_bits, sizeof(date_value_bits));
     } else {
       llvm::Triple triple(
           process_sp->GetTarget().GetArchitecture().GetTriple());
       uint32_t delta =
           (triple.isWatchOS() && triple.isWatchABI()) ? 8 : ptr_size;

lldb/source/Plugins/Language/ObjC/NSDictionary.cpp

bool lldb_private::formatters::NSDictionarySummaryProvider(
   static const ConstString g_DictionaryMImmutable("__NSDictionaryM_Immutable");
   static const ConstString g_Dictionary1("__NSSingleEntryDictionaryI");
   static const ConstString g_Dictionary0("__NSDictionary0");
   static const ConstString g_DictionaryCF("__NSCFDictionary");
 
   if (class_name.IsEmpty())
     return false;
 
-  if (class_name == g_DictionaryI || class_name == g_DictionaryMImmutable) {
+  if (class_name.oneOf(g_DictionaryI, g_DictionaryMImmutable)) {
     Status error;
     value = process_sp->ReadUnsignedIntegerFromMemory(valobj_addr + ptr_size,
                                                       ptr_size, 0, error);
     if (error.Fail())
       return false;
     value &= (is_64bit ? ~0xFC00000000000000UL : ~0xFC000000U);
-  } else if (class_name == g_DictionaryM || class_name == g_DictionaryMLegacy ||
-             class_name == g_DictionaryCF) {
+  } else if (class_name.oneOf(g_DictionaryM, g_DictionaryMLegacy,
+                              g_DictionaryCF)) {
     AppleObjCRuntime *apple_runtime =
     llvm::dyn_cast_or_null<AppleObjCRuntime>(runtime);
     Status error;
     if (apple_runtime && apple_runtime->GetFoundationVersion() >= 1437) {
       value = Foundation1437::__NSDictionaryMSize(*process_sp, valobj_addr,
                                                   error);
     } else {
       value = process_sp->ReadUnsignedIntegerFromMemory(valobj_addr + ptr_size,
                                                         ptr_size, 0, error);
       value &= (is_64bit ? ~0xFC00000000000000UL : ~0xFC000000U);
     }
     if (error.Fail())
       return false;
   } else if (class_name == g_Dictionary1) {
     value = 1;
   } else if (class_name == g_Dictionary0) {
     value = 0;
-  }
-  else {
+  } else {
     auto &map(NSDictionary_Additionals::GetAdditionalSummaries());
     for (auto &candidate : map) {
       if (candidate.first && candidate.first->Match(class_name))
         return candidate.second(valobj, stream, options);
     }
     return false;
   }
 

lldb/source/Plugins/Language/ObjC/ObjCLanguage.cpp

bool ObjCLanguage::GetFormatterPrefixSuffix(ValueObject &valobj,
   static ConstString g_NSStringStar("NSString*");
 
   if (type_hint.IsEmpty())
     return false;
 
   prefix.clear();
   suffix.clear();
 
-  if (type_hint == g_CFBag || type_hint == g_CFBinaryHeap) {
+  if (type_hint.oneOf(g_CFBag, g_CFBinaryHeap)) {
     prefix = "@";
     return true;
   }
 
   if (type_hint == g_NSNumberChar) {
     prefix = "(char)";
     return true;
   }
   if (type_hint == g_NSNumberShort) {
     prefix = "(short)";
     return true;
   }
   if (type_hint == g_NSNumberInt) {
     prefix = "(int)";
     return true;
   }
   if (type_hint == g_NSNumberLong) {
     prefix = "(long)";
     return true;
   }
   if (type_hint == g_NSNumberInt128) {
     prefix = "(int128_t)";
     return true;
   }
   if (type_hint == g_NSNumberFloat) {
     prefix = "(float)";
     return true;
   }
   if (type_hint == g_NSNumberDouble) {
     prefix = "(double)";
     return true;
   }
 
-  if (type_hint == g_NSData || type_hint == g_NSArray) {
+  if (type_hint.oneOf(g_NSData, g_NSArray)) {
     prefix = "@\"";
     suffix = "\"";
     return true;
   }
 
-  if (type_hint == g_NSString || type_hint == g_NSStringStar) {
+  if (type_hint.oneOf(g_NSString, g_NSStringStar)) {
     prefix = "@";
     return true;
   }
 
   return false;
 }
 
 bool ObjCLanguage::IsNilReference(ValueObject &valobj) {
   const uint32_t mask = eTypeIsObjC | eTypeIsPointer;
   bool isObjCpointer =
       (((valobj.GetCompilerType().GetTypeInfo(nullptr)) & mask) == mask);

lldb/source/Plugins/ObjectFile/ELF/ObjectFileELF.cpp

unsigned ObjectFileELF::ParseSymbols(Symtab *symtab, user_id_t start_id,
   // instruction emulation based unwind plans try to emulate all instructions
   // in these symbols. Don't add these symbols to the symbol list as they have
   // no use for the debugger and they are causing a lot of trouble. Filtering
   // can't be restricted to Android because this special object file don't
   // contain the note section specifying the environment to Android but the
   // 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 == ".oat" || file_extension == ".odex";
+  bool skip_oatdata_oatexec = file_extension.oneOf(".oat", ".odex");
 
   ArchSpec arch = GetArchitecture();
   ModuleSP module_sp(GetModule());
   SectionList *module_section_list =
       module_sp ? module_sp->GetSectionList() : nullptr;
 
   // Local cache to avoid doing a FindSectionByName for each symbol. The "const
   // char*" key must came from a ConstString object so they can be compared by

lldb/source/Plugins/ObjectFile/PECOFF/ObjectFilePECOFF.cpp

for (uint32_t idx = 0; idx < nsects; ++idx) {
       static ConstString g_sect_name_dwarf_debug_pubtypes(".debug_pubtypes");
       static ConstString g_sect_name_dwarf_debug_ranges(".debug_ranges");
       static ConstString g_sect_name_dwarf_debug_str(".debug_str");
       static ConstString g_sect_name_dwarf_debug_types(".debug_types");
       static ConstString g_sect_name_eh_frame(".eh_frame");
       static ConstString g_sect_name_go_symtab(".gosymtab");
       SectionType section_type = eSectionTypeOther;
       if (m_sect_headers[idx].flags & llvm::COFF::IMAGE_SCN_CNT_CODE &&
-          ((const_sect_name == g_code_sect_name) ||
-           (const_sect_name == g_CODE_sect_name))) {
+          const_sect_name.oneOf(g_code_sect_name, g_CODE_sect_name)) {
         section_type = eSectionTypeCode;
       } else if (m_sect_headers[idx].flags &
                      llvm::COFF::IMAGE_SCN_CNT_INITIALIZED_DATA &&
-                 ((const_sect_name == g_data_sect_name) ||
-                  (const_sect_name == g_DATA_sect_name))) {
+                 const_sect_name.oneOf(g_data_sect_name, g_DATA_sect_name)) {
         if (m_sect_headers[idx].size == 0 && m_sect_headers[idx].offset == 0)
           section_type = eSectionTypeZeroFill;
         else
           section_type = eSectionTypeData;
       } else if (m_sect_headers[idx].flags &
                      llvm::COFF::IMAGE_SCN_CNT_UNINITIALIZED_DATA &&
-                 ((const_sect_name == g_bss_sect_name) ||
-                  (const_sect_name == g_BSS_sect_name))) {
+                 const_sect_name.oneOf(g_bss_sect_name, g_BSS_sect_name)) {
         if (m_sect_headers[idx].size == 0)
           section_type = eSectionTypeZeroFill;
         else
           section_type = eSectionTypeData;
       } else if (const_sect_name == g_debug_sect_name) {
         section_type = eSectionTypeDebug;
       } else if (const_sect_name == g_stabstr_sect_name) {
         section_type = eSectionTypeDataCString;

lldb/source/Plugins/Platform/MacOSX/PlatformDarwinKernel.cpp

 PlatformDarwinKernel::FindKDKandSDKDirectoriesInDirectory(
     void *baton, llvm::sys::fs::file_type ft, llvm::StringRef path) {
   static ConstString g_sdk_suffix = ConstString(".sdk");
   static ConstString g_kdk_suffix = ConstString(".kdk");
 
   PlatformDarwinKernel *thisp = (PlatformDarwinKernel *)baton;
   FileSpec file_spec(path);
   if (ft == llvm::sys::fs::file_type::directory_file &&
-      (file_spec.GetFileNameExtension() == g_sdk_suffix ||
-       file_spec.GetFileNameExtension() == g_kdk_suffix)) {
+      file_spec.GetFileNameExtension().oneOf(g_sdk_suffix, g_kdk_suffix)) {
     AddRootSubdirsToSearchPaths(thisp, file_spec.GetPath());
   }
   return FileSystem::eEnumerateDirectoryResultNext;
 }
 
 // Recursively search trough m_search_directories looking for kext and kernel
 // binaries, adding files found to the appropriate lists.
 void PlatformDarwinKernel::SearchForKextsAndKernelsRecursively() {

lldb/unittests/Utility/ConstStringTest.cpp

TEST(ConstStringTest, CompareStringRef) {
   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");
 }
+
+TEST(ConstStringTest, OneOf) {
+  ConstString foo("foo");
+
+  EXPECT_TRUE(foo.oneOf("foo"));
+  EXPECT_TRUE(foo.oneOf("foo", "bar"));
+  EXPECT_TRUE(foo.oneOf("bar", "foo"));
+  EXPECT_TRUE(foo.oneOf("bar", "bar", "foo"));
+  EXPECT_TRUE(foo.oneOf("bar", "foo", "bar"));
+  EXPECT_TRUE(foo.oneOf("foo", "foo", "foo"));
+
+  EXPECT_FALSE(foo.oneOf(""));
+  EXPECT_FALSE(foo.oneOf("bar"));
+  EXPECT_FALSE(foo.oneOf("bar", ""));
+  EXPECT_FALSE(foo.oneOf("bar", "bar"));
+
+  ConstString empty("");
+  EXPECT_TRUE(empty.oneOf(""));
+  EXPECT_TRUE(empty.oneOf("foo", ""));
+  EXPECT_TRUE(empty.oneOf("foo", "", "foo"));
+  EXPECT_TRUE(empty.oneOf("foo", "", "foo"));
+  EXPECT_TRUE(empty.oneOf("foo", "foo", ""));
+}