labath marked 4 inline comments as done.
labath added inline comments.
================
Comment at: source/Plugins/SymbolFile/DWARF/DWARFUnit.cpp:775-787
+void DWARFUnit::ComputePathStyle() {
+ m_path_style = FileSpec::Style::native;
+ const DWARFDebugInfoEntry *die = GetUnitDIEPtrOnly();
+ if (!die)
+ return;
+ llvm::StringRef comp_dir =
+ die->GetAttributeValueAsString(m_dwarf, this, DW_AT_comp_dir, NULL);
----------------
clayborg wrote:
> I would suggest adding a "FileSpec DWARFUnit::GetFileSpec()" and moving code
> from SymbolFileDWARF::ParseCompileUnit into this function:
> ```
> FileSpec cu_file_spec(cu_die.GetName());
> if (cu_file_spec) {
> // If we have a full path to the compile unit, we don't need to
> // resolve the file. This can be expensive e.g. when the source
> // files are
> // NFS mounted.
> if (cu_file_spec.IsRelative()) {
> const char *cu_comp_dir{
> cu_die.GetAttributeValueAsString(DW_AT_comp_dir,
> nullptr)};
>
> cu_file_spec.PrependPathComponent(resolveCompDir(cu_comp_dir));
> }
> ```
>
> We might even cache the FileSpec object inside DWARFUnit so it doesn't have
> to be recomputed?
>
> Then use this resolved path. DW_AT_comp_dir isn't always there, sometimes the
> DW_AT_name is a full path only. So we should centralize this in DWARFUnit. We
> might want to make a function like:
>
> ```
> void DWARFUnit::ResolveCompileUnitDirectoryRelativeFile(FileSpec &spec);
> ```
>
> And then have "FileSpec DWARFUnit::GetFileSpec()" call it. I was looking at
> the other uses of DW_AT_comp_dir in the source, and the line tables tend to
> need to resolve relative paths. A few other locations. Might be nice to get
> the DWARFUnit and resolve the path using that? We can add "FileSpec
> DWARFDie::GetPath()" accessor, could even add a "void
> DWARFDie::ResolveRelativePath(FileSpec &)"
I think this version mostly implements what you had in mind. I added a
`DWARFUnit::GetCompilationDirectory()` which returns the DW_AT_comp_dir
attribute (I would expect that `DWARFUnit::GetFileSpec()` would return
DW_AT_name, which is not useful for relative path resolution, except as a hint
for the path style).
I've also added `FileSpec::MakeAbsolute` to shorten the `if(!spec.relative())
spec.prepend(dir))` pattern we had in some places. I didn't add any special
FileSpec accessor to DWARFDie, as it didn't seem particularly useful (at this
point). The only place where that's used now is for the computation of the name
of the compile unit, and that's pretty concise now anyway.
================
Comment at: source/Plugins/SymbolFile/DWARF/DWARFUnit.h:255
+ void ComputePathStyle();
+
----------------
zturner wrote:
> aprantl wrote:
> > I would probably call this DetectPathStyle() since it's more a heuristic?
> Maybe even `Guess` since `Compute` implies absolute certainty.
With all the other changes, this is now called
`ComputeCompDirAndGuessPathStyle`. Note that this is just a private helper
function and the public accessor is still called `GetPathStyle()`
CHANGES SINCE LAST ACTION
https://reviews.llvm.org/D56543/new/
https://reviews.llvm.org/D56543
_______________________________________________
lldb-commits mailing list
lldb-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
