[lldb] Change return type of FileSpec::GetFileNameExtension

[bulbazord]

Summary

These don't really need to be in ConstStrings. It's nice that comparing
ConstStrings is fast (just a pointer comparison) but the cost of
creating the ConstString usually already includes the cost of doing a
StringRef comparison anyway, so this is just extra work and extra memory
consumption for basically no benefit.

[JDevlieghere]

LGTM

lldb/include/lldb/Utility/FileSpec.h

public:
 
   /// Extract the extension of the file.
   ///
   /// Returns a ConstString that represents the extension of the filename for
   /// this FileSpec object. If this object does not represent a file, or the
   /// filename has no extension, ConstString(nullptr) is returned. The dot
   /// ('.') character is the first character in the returned string.
   ///
-  /// \return Returns the extension of the file as a ConstString object.
-  ConstString GetFileNameExtension() const;
+  /// \return Returns the extension of the file as a StringRef.
+  llvm::StringRef GetFileNameExtension() const;
 
   /// Return the filename without the extension part
   ///
   /// Returns a ConstString that represents the filename of this object
   /// without the extension part (e.g. for a file named "foo.bar", "foo" is
   /// returned)
   ///
   /// \return Returns the filename without extension as a ConstString object.

lldb/source/Commands/CommandObjectTarget.cpp

if (command.GetArgumentCount() != 1) {
       result.AppendErrorWithFormat("'%s' takes exactly one pcm path argument.",
                                    m_cmd_name.c_str());
       return false;
     }
 
     const char *pcm_path = command.GetArgumentAtIndex(0);
     FileSpec pcm_file{pcm_path};
 
-    if (pcm_file.GetFileNameExtension().GetStringRef() != ".pcm") {
+    if (pcm_file.GetFileNameExtension() != ".pcm") {
       result.AppendError("file must have a .pcm extension");
       return false;
     }
 
     if (!FileSystem::Instance().Exists(pcm_file)) {
       result.AppendError("pcm file does not exist");
       return false;
     }

lldb/source/Core/Debugger.cpp

bool Debugger::LoadPlugin(const FileSpec &spec, Status &error) {
   return false;
 }
 
 static FileSystem::EnumerateDirectoryResult
 LoadPluginCallback(void *baton, llvm::sys::fs::file_type ft,
                    llvm::StringRef path) {
   Status error;
 
-  static ConstString g_dylibext(".dylib");
-  static ConstString g_solibext(".so");
+  static constexpr llvm::StringLiteral g_dylibext(".dylib");
+  static constexpr llvm::StringLiteral g_solibext(".so");
 
   if (!baton)
     return FileSystem::eEnumerateDirectoryResultQuit;
 
   Debugger *debugger = (Debugger *)baton;
 
   namespace fs = llvm::sys::fs;
   // If we have a regular file, a symbolic link or unknown file type, try and

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

unsigned ObjectFileELF::ParseSymbols(Symtab *symtab, user_id_t start_id,
   // symbol name to the user and very slow unwinding speed because the
   // 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();
+  llvm::StringRef file_extension = m_file.GetFileNameExtension();
   bool skip_oatdata_oatexec =
       file_extension == ".oat" || file_extension == ".odex";
 
   ArchSpec arch = GetArchitecture();
   ModuleSP module_sp(GetModule());
   SectionList *module_section_list =
       module_sp ? module_sp->GetSectionList() : nullptr;
 

lldb/source/Plugins/Platform/Android/PlatformAndroid.cpp

uint32_t PlatformAndroid::GetSdkVersion() {
   // FIXME: improve error handling
   llvm::to_integer(version_string, m_sdk_version);
   return m_sdk_version;
 }
 
 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();
+  llvm::StringRef extension = module_sp->GetFileSpec().GetFileNameExtension();
   if (extension != ".oat" && extension != ".odex")
     return Status(
         "Symbol file downloading only supported for oat and odex files");
 
   // If we have no information about the platform file we can't execute oatdump
   if (!module_sp->GetPlatformFileSpec())
     return Status("No platform file specified");
 

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

FileSystem::Instance().EnumerateDirectory(
       dir.c_str(), find_directories, find_files, find_other,
       FindKDKandSDKDirectoriesInDirectory, this);
 }
 
 // Helper function to find *.sdk and *.kdk directories in a given directory.
 FileSystem::EnumerateDirectoryResult
 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");
+  static constexpr llvm::StringLiteral g_sdk_suffix = ".sdk";
+  static constexpr llvm::StringLiteral g_kdk_suffix = ".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)) {
     AddRootSubdirsToSearchPaths(thisp, file_spec.GetPath());
   }
PlatformDarwinKernel::GetKernelsAndKextsInDirectoryNoRecursion(
     void *baton, llvm::sys::fs::file_type ft, llvm::StringRef path) {
   return GetKernelsAndKextsInDirectoryHelper(baton, ft, path, false);
 }
 
 FileSystem::EnumerateDirectoryResult
 PlatformDarwinKernel::GetKernelsAndKextsInDirectoryHelper(
     void *baton, llvm::sys::fs::file_type ft, llvm::StringRef path,
     bool recurse) {
-  static ConstString g_kext_suffix = ConstString(".kext");
-  static ConstString g_dsym_suffix = ConstString(".dSYM");
+  static constexpr llvm::StringLiteral g_kext_suffix = ".kext";
+  static constexpr llvm::StringLiteral g_dsym_suffix = ".dSYM";
 
   FileSpec file_spec(path);
-  ConstString file_spec_extension = file_spec.GetFileNameExtension();
+  llvm::StringRef file_spec_extension = file_spec.GetFileNameExtension();
 
   Log *log = GetLog(LLDBLog::Platform);
 
   LLDB_LOGV(log, "PlatformDarwinKernel examining '{0}'", file_spec);
 
   PlatformDarwinKernel *thisp = (PlatformDarwinKernel *)baton;
 
   llvm::StringRef filename = file_spec.GetFilename().GetStringRef();

 
 // Given a FileSpec of /dir/dir/mach.development.t7004.dSYM
 // Return true if only the dSYM exists, no binary next to it.
 //    /dir/dir/mach.development.t7004.dSYM
 //    but no
 //    /dir/dir/mach.development.t7004
 bool PlatformDarwinKernel::KerneldSYMHasNoSiblingBinary(
     const FileSpec &kernel_dsym) {
-  static ConstString g_dsym_suffix = ConstString(".dSYM");
+  static constexpr llvm::StringLiteral g_dsym_suffix = ".dSYM";
   std::string possible_path = kernel_dsym.GetPath();
   if (kernel_dsym.GetFileNameExtension() != g_dsym_suffix)
     return false;
 
   FileSpec binary_filespec = kernel_dsym;
   // Chop off the '.dSYM' extension on the filename
   binary_filespec.SetFilename(binary_filespec.GetFileNameStrippingExtension());
 

 // TODO: This method returns a vector of FileSpec's because a
 // dSYM bundle may contain multiple DWARF binaries, but it
 // only implements returning the base name binary for now;
 // it should iterate over every binary in the DWARF subdir
 // and return them all.
 std::vector<FileSpec>
 PlatformDarwinKernel::GetDWARFBinaryInDSYMBundle(FileSpec dsym_bundle) {
   std::vector<FileSpec> results;
-  static ConstString g_dsym_suffix = ConstString(".dSYM");
+  static constexpr llvm::StringLiteral g_dsym_suffix = ".dSYM";
   if (dsym_bundle.GetFileNameExtension() != g_dsym_suffix) {
     return results;
   }
   // Drop the '.dSYM' from the filename
   std::string filename =
       dsym_bundle.GetFileNameStrippingExtension().GetCString();
   std::string dirname = dsym_bundle.GetDirectory().GetCString();
 

lldb/source/Plugins/ScriptInterpreter/Lua/Lua.cpp

 
 llvm::Error Lua::LoadModule(llvm::StringRef filename) {
   FileSpec file(filename);
   if (!FileSystem::Instance().Exists(file)) {
     return llvm::make_error<llvm::StringError>("invalid path",
                                                llvm::inconvertibleErrorCode());
   }
 
-  ConstString module_extension = file.GetFileNameExtension();
+  llvm::StringRef module_extension = file.GetFileNameExtension();
   if (module_extension != ".lua") {
     return llvm::make_error<llvm::StringError>("invalid extension",
                                                llvm::inconvertibleErrorCode());
   }
 
   int error = luaL_loadfile(m_lua_state, filename.data()) ||
               lua_pcall(m_lua_state, 0, 1, 0);
   if (error != LUA_OK) {

lldb/source/Plugins/ScriptInterpreter/Python/ScriptInterpreterPython.cpp

 void ScriptInterpreterPython::SharedLibraryDirectoryHelper(
     FileSpec &this_file) {
   // When we're loaded from python, this_file will point to the file inside the
   // python package directory. Replace it with the one in the lib directory.
 #ifdef _WIN32
   // On windows, we need to manually back out of the python tree, and go into
   // the bin directory. This is pretty much the inverse of what ComputePythonDir
   // does.
-  if (this_file.GetFileNameExtension() == ConstString(".pyd")) {
+  if (this_file.GetFileNameExtension() == ".pyd") {
     this_file.RemoveLastPathComponent(); // _lldb.pyd or _lldb_d.pyd
     this_file.RemoveLastPathComponent(); // lldb
     llvm::StringRef libdir = LLDB_PYTHON_RELATIVE_LIBDIR;
     for (auto it = llvm::sys::path::begin(libdir),
               end = llvm::sys::path::end(libdir);
          it != end; ++it)
       this_file.RemoveLastPathComponent();
     this_file.AppendPathComponent("bin");

lldb/source/Utility/FileSpec.cpp

if (m_directory && m_filename && m_directory.GetStringRef().back() != '/' &&
       m_filename.GetStringRef().back() != '/')
     path.insert(path.end(), '/');
   path.append(m_filename.GetStringRef().begin(),
               m_filename.GetStringRef().end());
   if (denormalize && !path.empty())
     Denormalize(path, m_style);
 }
 
-ConstString FileSpec::GetFileNameExtension() const {
-  return ConstString(
-      llvm::sys::path::extension(m_filename.GetStringRef(), m_style));
+llvm::StringRef FileSpec::GetFileNameExtension() const {
+  return llvm::sys::path::extension(m_filename.GetStringRef(), m_style);
 }
 
 ConstString FileSpec::GetFileNameStrippingExtension() const {
   return ConstString(llvm::sys::path::stem(m_filename.GetStringRef(), m_style));
 }
 
 // Return the size in bytes that this object takes in memory. This returns the
 // size in bytes of this object, not any shared string values it may refer to.

 /// Returns true if the filespec represents an implementation source
 /// file (files with a ".c", ".cpp", ".m", ".mm" (many more)
 /// extension).
 ///
 /// \return
 ///     \b true if the filespec represents an implementation source
 ///     file, \b false otherwise.
 bool FileSpec::IsSourceImplementationFile() const {
-  ConstString extension(GetFileNameExtension());
-  if (!extension)
+  llvm::StringRef extension = GetFileNameExtension();
+  if (extension.empty())
     return false;
 
   static RegularExpression g_source_file_regex(llvm::StringRef(
       "^.([cC]|[mM]|[mM][mM]|[cC][pP][pP]|[cC]\\+\\+|[cC][xX][xX]|[cC][cC]|["
       "cC][pP]|[sS]|[aA][sS][mM]|[fF]|[fF]77|[fF]90|[fF]95|[fF]03|[fF][oO]["
       "rR]|[fF][tT][nN]|[fF][pP][pP]|[aA][dD][aA]|[aA][dD][bB]|[aA][dD][sS])"
       "$"));
-  return g_source_file_regex.Execute(extension.GetStringRef());
+  return g_source_file_regex.Execute(extension);
 }
 
 bool FileSpec::IsRelative() const {
   return !IsAbsolute();
 }
 
 bool FileSpec::IsAbsolute() const {
   // Check if we have cached if this path is absolute to avoid recalculating.

lldb/unittests/Utility/FileSpecTest.cpp

 
 TEST(FileSpecTest, TestFileNameExtensions) {
   FileSpec dylib = PosixSpec("/tmp/foo.dylib");
   FileSpec exe = PosixSpec("/tmp/foo");
   FileSpec dSYM = PosixSpec("/tmp/foo.dSYM/");
   FileSpec just_dot = PosixSpec("/tmp/bar.");
 
   EXPECT_TRUE(dylib.GetFileNameExtension() == ".dylib");
-  EXPECT_TRUE(exe.GetFileNameExtension() == ConstString(nullptr));
+  EXPECT_TRUE(exe.GetFileNameExtension() == llvm::StringRef());
   EXPECT_TRUE(dSYM.GetFileNameExtension() == ".dSYM");
   EXPECT_TRUE(just_dot.GetFileNameExtension() == ".");
 
   FileSpec dll = WindowsSpec("C:\\tmp\\foo.dll");
   FileSpec win_noext = WindowsSpec("C:\\tmp\\foo");
 
   EXPECT_TRUE(dll.GetFileNameExtension() == ".dll");
-  EXPECT_TRUE(win_noext.GetFileNameExtension() == ConstString(nullptr));
+  EXPECT_TRUE(win_noext.GetFileNameExtension() == llvm::StringRef());
 }
 
 TEST(FileSpecTest, TestFileNameStrippingExtension) {
   FileSpec dylib = PosixSpec("/tmp/foo.dylib");
   FileSpec exe = PosixSpec("/tmp/foo");
   FileSpec just_dot = PosixSpec("/tmp/bar.");
 
   EXPECT_TRUE(dylib.GetFileNameStrippingExtension() == "foo");