[PATCH] D51568: [modules] Add `-fdisable-module-directory` flag for relocatable modules
andrewjcg updated this revision to Diff 163621.
andrewjcg added a comment.
fix umbrella writing
Repository:
rC Clang
https://reviews.llvm.org/D51568
Files:
include/clang/Driver/CC1Options.td
include/clang/Lex/HeaderSearchOptions.h
lib/Frontend/CompilerInvocation.cpp
lib/Serialization/ASTWriter.cpp
test/Modules/relocatable-modules.modulemap
Index: test/Modules/relocatable-modules.modulemap
===
--- /dev/null
+++ test/Modules/relocatable-modules.modulemap
@@ -0,0 +1,29 @@
+// Build two otherwise identical modules in two different directories and
+// verify that using `-fdisable-module-directory` makes them identical.
+//
+// RUN: rm -rf %t
+//
+// RUN: mkdir -p %t/p1
+// RUN: cd %t/p1
+// RUN: mkdir -p main other
+// RUN: grep "" %s > main/a.modulemap
+// RUN: grep "" %s > main/a.h
+// RUN: grep "" %s > other/b.h
+// RUN: %clang_cc1 -x c++ -fmodules -emit-module -fdisable-module-directory \
+// RUN: -fmodule-name="a" -Imain -I. -o - main/a.modulemap > a.pcm
+//
+// RUN: mkdir -p %t/p2
+// RUN: cd %t/p2
+// RUN: mkdir -p main other
+// RUN: grep "" %s > main/a.modulemap
+// RUN: grep "" %s > main/a.h
+// RUN: grep "" %s > other/b.h
+// RUN: %clang_cc1 -x c++ -fmodules -emit-module -fdisable-module-directory \
+// RUN: -fmodule-name="a" -Imain -I. -o - main/a.modulemap > a.pcm
+//
+// RUN: diff %t/p1/a.pcm %t/p2/a.pcm
+
+module "a" {//
+} //
+
+#include "b.h" //
Index: lib/Serialization/ASTWriter.cpp
===
--- lib/Serialization/ASTWriter.cpp
+++ lib/Serialization/ASTWriter.cpp
@@ -1339,9 +1339,14 @@
///
/// \return \c true if the path was changed.
static bool cleanPathForOutput(FileManager ,
- SmallVectorImpl ) {
- bool Changed = FileMgr.makeAbsolutePath(Path);
- return Changed | llvm::sys::path::remove_dots(Path);
+ SmallVectorImpl ,
+ bool MakeAbsolute = true) {
+ bool Changed = false;
+ if (MakeAbsolute) {
+Changed |= FileMgr.makeAbsolutePath(Path);
+ }
+ Changed |= llvm::sys::path::remove_dots(Path);
+ return Changed;
}
/// Adjusts the given filename to only write out the portion of the
@@ -1503,7 +1508,11 @@
Stream.EmitRecordWithBlob(AbbrevCode, Record, WritingModule->Name);
}
- if (WritingModule && WritingModule->Directory) {
+ if (WritingModule &&
+ WritingModule->Directory &&
+ !PP.getHeaderSearchInfo()
+ .getHeaderSearchOpts()
+ .DisableModuleDirectory) {
SmallString<128> BaseDir(WritingModule->Directory->getName());
cleanPathForOutput(Context.getSourceManager().getFileManager(), BaseDir);
@@ -2952,12 +2961,26 @@
// Emit the umbrella header, if there is one.
if (auto UmbrellaHeader = Mod->getUmbrellaHeader()) {
RecordData::value_type Record[] = {SUBMODULE_UMBRELLA_HEADER};
- Stream.EmitRecordWithBlob(UmbrellaAbbrev, Record,
-UmbrellaHeader.NameAsWritten);
+ SmallString<128> Buffer;
+ if (PP->getHeaderSearchInfo()
+ .getHeaderSearchOpts()
+ .DisableModuleDirectory) {
+llvm::sys::path::append(Buffer, Mod->Directory->getName());
+ }
+ llvm::sys::path::append(Buffer, UmbrellaHeader.NameAsWritten);
+ llvm::sys::path::remove_dots(Buffer);
+ Stream.EmitRecordWithBlob(UmbrellaAbbrev, Record, Buffer);
} else if (auto UmbrellaDir = Mod->getUmbrellaDir()) {
RecordData::value_type Record[] = {SUBMODULE_UMBRELLA_DIR};
- Stream.EmitRecordWithBlob(UmbrellaDirAbbrev, Record,
-UmbrellaDir.NameAsWritten);
+ SmallString<128> Buffer;
+ if (PP->getHeaderSearchInfo()
+ .getHeaderSearchOpts()
+ .DisableModuleDirectory) {
+llvm::sys::path::append(Buffer, Mod->Directory->getName());
+ }
+ llvm::sys::path::append(Buffer, UmbrellaDir.NameAsWritten);
+ llvm::sys::path::remove_dots(Buffer);
+ Stream.EmitRecordWithBlob(UmbrellaDirAbbrev, Record, Buffer);
}
// Emit the headers.
@@ -4515,7 +4538,11 @@
assert(Context && "should have context when outputting path");
bool Changed =
- cleanPathForOutput(Context->getSourceManager().getFileManager(), Path);
+ cleanPathForOutput(Context->getSourceManager().getFileManager(),
+ Path,
+ !PP->getHeaderSearchInfo()
+ .getHeaderSearchOpts()
+ .DisableModuleDirectory);
// Remove a prefix to make the path relative, if relevant.
const char *PathBegin = Path.data();
Index: lib/Frontend/CompilerInvocation.cpp
===
--- lib/Frontend/CompilerInvocation.cpp
+++ lib/Frontend/CompilerInvocation.cpp
@@ -1746,6 +1746,7 @@
[PATCH] D51568: [modules] Add `-fdisable-module-directory` flag for relocatable modules
andrewjcg added a comment. I'm not sure this is the best approach, but I wasn't sure of a better one (to support module files w/o absolute paths). Another approach I tried, was relativizing the other input files (from outside the module directory) using chains of `../` (e.g. `../../../../../other/modules/module.modulemap`), but this causes issues when symlinks appear in the module directory path. Another potential option could be to add a bit to the serialized module format for each input, to allow some inputs to mark themselves as relative to the CWD, rather than the module directory. Repository: rC Clang 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: [modules] Add `-fdisable-module-directory` flag for relocatable modules
andrewjcg created this revision.
Herald added a subscriber: cfe-commits.
Currently, modules built using module map files embed the path to the directory
that houses their inputs (e.g. headers and module map file) into the output
module file. This path is embedded as an absolute path and the various input
files are either relativized to this path (if they are underneath it) or also
embedded as an absolute path.
This adds a flag which disables use of this absolute module directory path and
instead writes out paths into the module unmodified (e.g. if they were specified
on the command line as relative paths to the CWD, then they remain relative to
the CWD). This allows building modules without any absolute paths, allowing them
to be built and shared between different working directories.
Repository:
rC Clang
https://reviews.llvm.org/D51568
Files:
include/clang/Driver/CC1Options.td
include/clang/Lex/HeaderSearchOptions.h
lib/Frontend/CompilerInvocation.cpp
lib/Serialization/ASTWriter.cpp
test/Modules/relocatable-modules.modulemap
Index: test/Modules/relocatable-modules.modulemap
===
--- /dev/null
+++ test/Modules/relocatable-modules.modulemap
@@ -0,0 +1,29 @@
+// Build two otherwise identical modules in two different directories and
+// verify that using `-fdisable-module-directory` makes them identical.
+//
+// RUN: rm -rf %t
+//
+// RUN: mkdir -p %t/p1
+// RUN: cd %t/p1
+// RUN: mkdir -p main other
+// RUN: grep "" %s > main/a.modulemap
+// RUN: grep "" %s > main/a.h
+// RUN: grep "" %s > other/b.h
+// RUN: %clang_cc1 -x c++ -fmodules -emit-module -fdisable-module-directory \
+// RUN: -fmodule-name="a" -Imain -I. -o - main/a.modulemap > a.pcm
+//
+// RUN: mkdir -p %t/p2
+// RUN: cd %t/p2
+// RUN: mkdir -p main other
+// RUN: grep "" %s > main/a.modulemap
+// RUN: grep "" %s > main/a.h
+// RUN: grep "" %s > other/b.h
+// RUN: %clang_cc1 -x c++ -fmodules -emit-module -fdisable-module-directory \
+// RUN: -fmodule-name="a" -Imain -I. -o - main/a.modulemap > a.pcm
+//
+// RUN: diff %t/p1/a.pcm %t/p2/a.pcm
+
+module "a" {//
+} //
+
+#include "b.h" //
Index: lib/Serialization/ASTWriter.cpp
===
--- lib/Serialization/ASTWriter.cpp
+++ lib/Serialization/ASTWriter.cpp
@@ -1339,9 +1339,14 @@
///
/// \return \c true if the path was changed.
static bool cleanPathForOutput(FileManager ,
- SmallVectorImpl ) {
- bool Changed = FileMgr.makeAbsolutePath(Path);
- return Changed | llvm::sys::path::remove_dots(Path);
+ SmallVectorImpl ,
+ bool MakeAbsolute = true) {
+ bool Changed = false;
+ if (MakeAbsolute) {
+Changed |= FileMgr.makeAbsolutePath(Path);
+ }
+ Changed |= llvm::sys::path::remove_dots(Path);
+ return Changed;
}
/// Adjusts the given filename to only write out the portion of the
@@ -1503,7 +1508,11 @@
Stream.EmitRecordWithBlob(AbbrevCode, Record, WritingModule->Name);
}
- if (WritingModule && WritingModule->Directory) {
+ if (WritingModule &&
+ WritingModule->Directory &&
+ !PP.getHeaderSearchInfo()
+ .getHeaderSearchOpts()
+ .DisableModuleDirectory) {
SmallString<128> BaseDir(WritingModule->Directory->getName());
cleanPathForOutput(Context.getSourceManager().getFileManager(), BaseDir);
@@ -2952,12 +2961,20 @@
// Emit the umbrella header, if there is one.
if (auto UmbrellaHeader = Mod->getUmbrellaHeader()) {
RecordData::value_type Record[] = {SUBMODULE_UMBRELLA_HEADER};
- Stream.EmitRecordWithBlob(UmbrellaAbbrev, Record,
-UmbrellaHeader.NameAsWritten);
+ SmallString<128> Buffer;
+ llvm::sys::path::append(Buffer,
+ Mod->Directory->getName(),
+ UmbrellaHeader.NameAsWritten);
+ llvm::sys::path::remove_dots(Buffer);
+ Stream.EmitRecordWithBlob(UmbrellaAbbrev, Record, Buffer);
} else if (auto UmbrellaDir = Mod->getUmbrellaDir()) {
RecordData::value_type Record[] = {SUBMODULE_UMBRELLA_DIR};
- Stream.EmitRecordWithBlob(UmbrellaDirAbbrev, Record,
-UmbrellaDir.NameAsWritten);
+ SmallString<128> Buffer;
+ llvm::sys::path::append(Buffer,
+ Mod->Directory->getName(),
+ UmbrellaDir.NameAsWritten);
+ llvm::sys::path::remove_dots(Buffer);
+ Stream.EmitRecordWithBlob(UmbrellaDirAbbrev, Record, Buffer);
}
// Emit the headers.
@@ -4515,7 +4532,11 @@
assert(Context && "should have context when outputting path");
bool Changed =
- cleanPathForOutput(Context->getSourceManager().getFileManager(), Path);
+ cleanPathForOutput(Context->getSourceManager().getFileManager(),
+
