rsmith 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.

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. (For an implicitly built module, we probably need to track the absolute 
path so we can detect whether we found the right PCM file.)



================
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));
----------------
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?


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
  • [PATCH] D51568: [... Brian Gesiak via Phabricator via cfe-commits
    • [PATCH] D515... Manman Ren via Phabricator via cfe-commits
    • [PATCH] D515... Richard Smith - zygoloid via Phabricator via cfe-commits
    • [PATCH] D515... Bruno Cardoso Lopes via Phabricator via cfe-commits
    • [PATCH] D515... Andrew Gallagher via Phabricator via cfe-commits
    • [PATCH] D515... Richard Smith - zygoloid via Phabricator via cfe-commits

Reply via email to