[Lldb-commits] [PATCH] D149096: [lldb] Speed up looking for dSYM next to executable

[Med Ismail Bennani via Phabricator via lldb-commits]

mib added inline comments.


================
Comment at: lldb/source/Symbol/LocateSymbolFile.cpp:89-91
+  dsym_file.append(".dSYM");
+  llvm::sys::path::append(dsym_file, path_style, "Contents", "Resources",
+                          "DWARF", filename);
----------------
Why do you need the first call to `append` ?


================
Comment at: lldb/source/Symbol/LocateSymbolFile.cpp:102
 
-    // See if we have "../CF.framework" - so we'll look for
-    // CF.framework.dSYM/Contents/Resources/DWARF/CF
-    // We need to drop the last suffix after '.' to match
-    // 'CF' in the DWARF subdir.
-    std::string binary_name(filename.AsCString());
-    auto last_dot = binary_name.find_last_of('.');
-    if (last_dot != std::string::npos) {
-      binary_name.erase(last_dot);
-      dsym_fspec = dsym_directory;
-      dsym_fspec.AppendPathComponent(binary_name);
-      if (FileSystem::Instance().Exists(dsym_fspec) &&
-          FileAtPathContainsArchAndUUID(dsym_fspec,
-                                        mod_spec.GetArchitecturePtr(),
-                                        mod_spec.GetUUIDPtr())) {
-        return true;
+  // See if we have ".../CF.framework" - so we'll look for
+  // CF.framework.dSYM/Contents/Resources/DWARF/CF
----------------
Is this extra dot on purpose ?


================
Comment at: lldb/source/Symbol/LocateSymbolFile.cpp:105-108
+  if (filename.endswith(".framework")) {
+    const auto last_dot = dsym_file.find_last_of('.');
+    if (last_dot != llvm::StringRef::npos) {
+      dsym_file.truncate(last_dot - 1);
----------------
This part is a bit confusing ... may be a comment would help understand what we're trying to achieve here.


================
Comment at: lldb/source/Symbol/LocateSymbolFile.cpp:175-178
+    llvm::StringRef filename =
+        llvm::sys::path::filename(parent_path, path_style);
+    if (filename.find('.') == llvm::StringRef::npos)
+      continue;
----------------
Same here ... this could use a comment to explain what we're doing.


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D149096/new/

https://reviews.llvm.org/D149096