rsmith added a comment.
In D51568#1314004 <https://reviews.llvm.org/D51568#1314004>, @andrewjcg wrote:
> > I don't think we need to change the serialization format for this: a
> > serialized path beginning with / is already treated as absolute and any
> > other path is already treated as relative, so we don't need a flag to carry
> > that information.
>
> But I think we need this since we now have two types of relative paths -- a
> CWD-relative path and a module-home-relative path -- and we use this flag to
> discern them for the AST reader. Previously, because `cleanPathForOutput`
> would always absolutify input paths, we didn't need this flag -- any relative
> path was relative the module home and all other paths were absolute.
Ah, of course, that's exactly what I was missing :) I guess I live in a
too-`-fmodule-map-file-home-is-cwd`-centric world where the two are always the
same (even when modules are relocated).
Following the 0-1-infinity principle, I think it might make sense to have a
'catalog' of prefixes for paths (relative to the module, relative to the
compiler's CWD, relative to the compiler's resource directory, relative to the
sysroot) that we try stripping off, with different markers to say which one we
removed. (Right now, if you relocate the compiler, you invalidate any .pcm file
that references files in its resource directory, for instance.) But this seems
like a step in a good direction.
Just checking that we're on the same page here... suppose I do this:
- compile module described in `foo/module.modulemap` (with no
`-fmodule-map-file-home-is-cwd`, so module-relative paths have the `foo/`
prefix stripped off them)
- use `-Ibar/` to find some textual headers
Then, if I relocate the `foo/` module to `elsewhere/foo` and pass the
corresponding `pcm` file to a compilation using that module, I will still
expect to find the `bar/` files referenced by the `pcm` file relative to the
compiler's working directory, not in `elsewhere/bar`. Is that what you're
expecting, or would you expect the file paths to be emitted as module-relative
`../bar/...` paths? (Note that the latter can break if `foo` is a symlink.)
================
Comment at: lib/Serialization/ASTReader.cpp:2094-2096
+ bool IsRelativeModuleDirectory = static_cast<bool>(Record[6]);
R.Filename = Blob;
+ if (IsRelativeModuleDirectory) {
----------------
This'd be more readable (throughout the patch) spelled as
`IsRelativeToModuleDirectory`. (I was wondering "What is a relative module
directory?")
Repository:
rC Clang
CHANGES SINCE LAST ACTION
https://reviews.llvm.org/D51568/new/
https://reviews.llvm.org/D51568
_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
