bulbazord 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);
----------------
mib wrote:
> Why do you need the first call to `append` ?
The code suggestion you've given doesn't work for 2 reasons:
- `llvm::sys::path::append` only takes 4 Twine arguments, so this won't compile.
- `llvm::sys::path::append` appends things as if they were paths. So for
`/bin/ls` you'll end up with `/bin/ls/.dSYM/Contents/Resources/DWARF` instead
of `/bin/ls.dSYM/Contents/Resources/DWARF`.
================
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
----------------
mib wrote:
> Is this extra dot on purpose ?
I wanted to distinguish between `$parent_dir/CF.framework` because what is the
parent_dir in this case? I used 3 dots to indicate `$some_path/CF.framework`.
================
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);
----------------
mib wrote:
> This part is a bit confusing ... may be a comment would help understand what
> we're trying to achieve here.
I'll add more to the comment.
================
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;
----------------
mib wrote:
> Same here ... this could use a comment to explain what we're doing.
Will add.
Repository:
rG LLVM Github Monorepo
CHANGES SINCE LAST ACTION
https://reviews.llvm.org/D149096/new/
https://reviews.llvm.org/D149096
_______________________________________________
lldb-commits mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
