Page MenuHomePhabricator

Allow remapping Clang module include paths
ClosedPublic

Authored by aprantl on Mar 18 2020, 1:45 PM.

Diff Detail

Event Timeline

aprantl created this revision.Mar 18 2020, 1:45 PM
aprantl marked an inline comment as done.Mar 18 2020, 2:49 PM
aprantl added inline comments.
clang/lib/CodeGen/CGDebugInfo.cpp
2488

Is this legal?

remapDIPath takes a StringRef to Path. Is the sret value going to be a temporary or are we modifying the contents of the string while reading from it in remapDIPath?

JDevlieghere added inline comments.Mar 18 2020, 3:17 PM
clang/lib/CodeGen/CGDebugInfo.cpp
2488

No, this returns a std::string so you're returning a StringRef to a stack-allocated temporary. The lambda should return a std::string too.

aprantl updated this revision to Diff 251411.Mar 19 2020, 10:19 AM

Don't try to be too clever.

aprantl marked an inline comment as done.Mar 19 2020, 10:20 AM
shafik added inline comments.Mar 19 2020, 1:55 PM
clang/lib/CodeGen/CGDebugInfo.cpp
2487

& -> &TheCU

We should try to explicitly capture what we use, this will avoid potential bugs where we accidentally modify something we did not mean to capture later on when modifying the code.

aprantl updated this revision to Diff 251789.Mar 20 2020, 3:33 PM

Rebased and addressed review comments.

JDevlieghere accepted this revision.Mar 24 2020, 9:43 AM
This revision is now accepted and ready to land.Mar 24 2020, 9:43 AM
This revision was automatically updated to reflect the committed changes.
Herald added a project: Restricted Project. · View Herald TranscriptMar 24 2020, 5:30 PM

I had to revert this because it unexpectedly broke the "expr -- @import Module" test in LLDB.

aprantl reopened this revision.Mar 24 2020, 5:59 PM
This revision is now accepted and ready to land.Mar 24 2020, 5:59 PM

LLDB didn't apply DW_AT_comp_dir to DW_AT_LLVM_include_path. I fixed that now.

This revision was automatically updated to reflect the committed changes.