andrewjcg marked 2 inline comments as done.
andrewjcg added a comment.
> 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.
I attempted another take on this diff which just made all paths relative the
CWD (and did away with module home relative paths), but this didn't work since
the reader substitutes in a new module home.
> I'm also not convinced we need to put this behind a flag. It would seem
> reasonable to me to simply always emit the MODULE_DIRECTORY as relative (if
> we found the module via a relative path), at least for an explicitly-built
> module.
Yeah, makes sense. Will fix this.
================
Comment at: lib/Serialization/ASTWriter.cpp:4524-4546
+bool ASTWriter::PreparePathForOutput(SmallVectorImpl<char> &Path,
+ bool &IsRelativeModuleDirectory) {
assert(Context && "should have context when outputting path");
bool Changed =
- cleanPathForOutput(Context->getSourceManager().getFileManager(), Path);
+ cleanPathForOutput(Context->getSourceManager().getFileManager(), Path,
+ llvm::sys::path::is_absolute(BaseDirectory));
----------------
rsmith wrote:
> The intent of this function is to use an absolute form of both the file path
> and `BaseDirectory` so that we can strip off `BaseDirectory` even if the file
> path ends up absolute somehow. Rather than changing `BaseDirectory` above
> and then changing the code here to compensate, could you only change the code
> that writes out the `MODULE_DIRECTORY` record to write a relative path?
So we also several non-module-home relative paths get processed here, so just I
think we'd have to throw away the output of `cleanPathForOutput` if the
`Changed == false`, to preserve the input relative path when
non-module-home-relative?
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
