This is an archive of the discontinued LLVM Phabricator instance.

[LLD][COFF] Clean paths in PDB even when /pdbsourcepath isn't used
ClosedPublic

Authored by aganea on Aug 31 2021, 3:28 PM.

Details

Summary

Some of our build system scripts & third party solutions have a hard time without this.
Before this patch we had "dotted" paths such as:

StringTable:
        15:  - 'F:\blah\.\file.cpp'

Diff Detail

Event Timeline

aganea requested review of this revision.Aug 31 2021, 3:28 PM
aganea created this revision.
Herald added a project: Restricted Project. · View Herald TranscriptAug 31 2021, 3:28 PM
rnk accepted this revision.Aug 31 2021, 3:40 PM

lgtm

lld/COFF/PDB.cpp
283

Seems consistent

This revision is now accepted and ready to land.Aug 31 2021, 3:40 PM

I'm mildly surprised the make_absolute call doesn't remove unnecessary dotted paths. I guess that's related to the weird schism between sys::fs and sys::path.

Having recently tried out std::filesystem, I was pleasantly surprised that it was not nearly as terrible as I expected. :-) I know it doesn't meet the cross platform needs of LLVM, but maybe we should consider making LLVM's path handling more like it at some point...

Having recently tried out std::filesystem, I was pleasantly surprised that it was not nearly as terrible as I expected. :-) I know it doesn't meet the cross platform needs of LLVM, but maybe we should consider making LLVM's path handling more like it at some point...

I think(?) the LLVM path handling is kinda modelled to be like std::filesystem, although there are a number of more or less subtle differences. Adjusting it to be a closer match probably would be good, although it would require fixing up all cases of those subtle changes.

Regarding using std::filesystem in LLVM, I'm not offhand entirely sure which cross platform needs it doesn't support. On Windows it can handle paths with both kinds of slashes (and you can export it to either form), but it doesn't support handling paths with backslashes as separators on non-Windows platforms - which I guess can be needed in some cases.

Also regarding using std::filesystem in LLVM; libc++ does support std::filesystem on Windows these days, but it's problematic to use it in MSVC mode as it requires compiler-rt builtins for handling __int128, see https://reviews.llvm.org/D91139#2429595. So until that's resolved, it'd be hard to use it in LLVM itself as it would break self-hosting.