[PATCH] D103930: [clang][HeaderSearch] Fix implicit module when using header maps
andrewjcg added a comment. @ivanmurashko: Sorry for the delay getting back to you here. Feel free to commandeer, as I don't have plans to get to this soon. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D103930/new/ https://reviews.llvm.org/D103930 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D103930: [clang][HeaderSearch] Fix implicit module when using header maps
andrewjcg added a comment. Thanks! Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D103930/new/ https://reviews.llvm.org/D103930 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D103930: [clang][HeaderSearch] Fix implicit module when using header maps
andrewjcg updated this revision to Diff 352898.
andrewjcg added a comment.
capitalize param
Repository:
rG LLVM Github Monorepo
CHANGES SINCE LAST ACTION
https://reviews.llvm.org/D103930/new/
https://reviews.llvm.org/D103930
Files:
clang/lib/Lex/HeaderSearch.cpp
clang/test/Modules/Inputs/implicit-module-header-maps/a.h
clang/test/Modules/Inputs/implicit-module-header-maps/a.hmap.json
clang/test/Modules/Inputs/implicit-module-header-maps/a.module.modulemap
clang/test/Modules/Inputs/implicit-module-header-maps/b.hmap.json
clang/test/Modules/implicit-module-header-maps.cpp
Index: clang/test/Modules/implicit-module-header-maps.cpp
===
--- /dev/null
+++ clang/test/Modules/implicit-module-header-maps.cpp
@@ -0,0 +1,32 @@
+// RUN: rm -rf %t
+// RUN: mkdir %t
+// RUN: cd %t
+//
+// RUN: %hmaptool write %S/Inputs/implicit-module-header-maps/a.hmap.json %t/hmap
+//
+// RUN: mkdir -p %t/After
+// RUN: cp %S/Inputs/implicit-module-header-maps/a.h %t/After/Mapping.h
+// RUN: cp %S/Inputs/implicit-module-header-maps/a.module.modulemap %t/module.modulemap
+//
+// RUN: %clang -Rmodule-build -fmodules -fimplicit-modules -fimplicit-module-maps -fmodule-map-file=%t/module.modulemap -fsyntax-only %s -I %t/hmap
+//
+// RUN: rm -rf %t
+// RUN: mkdir %t
+// RUN: cd %t
+//
+// RUN: sed -e "s|OUTPUTS_DIR|%t|g" %S/Inputs/implicit-module-header-maps/b.hmap.json > %t/hmap.json
+// RUN: %hmaptool write %t/hmap.json %t/hmap
+//
+// RUN: mkdir -p %t/After
+// RUN: cp %S/Inputs/implicit-module-header-maps/a.h %t/After/Mapping.h
+// RUN: cp %S/Inputs/implicit-module-header-maps/a.module.modulemap %t/module.modulemap
+//
+// RUN: %clang -Rmodule-build -fmodules -fimplicit-modules -fimplicit-module-maps -fmodule-map-file=%t/module.modulemap -fsyntax-only %s -I %t/hmap
+
+#define FOO
+// This include will fail if:
+// 1) modules are't used, as the `FOO` define will propagate into the included
+//header and trip a `#error`, or
+// 2) header maps aren't usesd, as the header name doesn't exist and relies on
+//the header map to remap it to the real header.
+#include "Before/Mapping.h"
Index: clang/test/Modules/Inputs/implicit-module-header-maps/b.hmap.json
===
--- /dev/null
+++ clang/test/Modules/Inputs/implicit-module-header-maps/b.hmap.json
@@ -0,0 +1,6 @@
+{
+ "mappings" :
+{
+ "Before/Mapping.h" : "OUTPUTS_DIR/After/Mapping.h"
+}
+}
Index: clang/test/Modules/Inputs/implicit-module-header-maps/a.module.modulemap
===
--- /dev/null
+++ clang/test/Modules/Inputs/implicit-module-header-maps/a.module.modulemap
@@ -0,0 +1,3 @@
+module a {
+ header "After/Mapping.h"
+}
Index: clang/test/Modules/Inputs/implicit-module-header-maps/a.hmap.json
===
--- /dev/null
+++ clang/test/Modules/Inputs/implicit-module-header-maps/a.hmap.json
@@ -0,0 +1,7 @@
+{
+ "mappings" :
+{
+ "Before/Mapping.h" : "After/Mapping.h",
+ "After/Mapping.h" : "After/Mapping.h"
+}
+}
Index: clang/test/Modules/Inputs/implicit-module-header-maps/a.h
===
--- /dev/null
+++ clang/test/Modules/Inputs/implicit-module-header-maps/a.h
@@ -0,0 +1,3 @@
+#ifdef FOO
+#error foo
+#endif
Index: clang/lib/Lex/HeaderSearch.cpp
===
--- clang/lib/Lex/HeaderSearch.cpp
+++ clang/lib/Lex/HeaderSearch.cpp
@@ -417,7 +417,8 @@
IsInHeaderMap = true;
- auto FixupSearchPath = [&]() {
+ auto FixupSearchPathAndFindUsableModule =
+ [&](auto File) -> Optional {
if (SearchPath) {
StringRef SearchPathRef(getName());
SearchPath->clear();
@@ -427,6 +428,12 @@
RelativePath->clear();
RelativePath->append(Filename.begin(), Filename.end());
}
+if (!HS.findUsableModuleForHeader(
+(), File.getFileEntry().getDir(),
+RequestingModule, SuggestedModule, isSystemHeaderDirectory())) {
+ return None;
+}
+return File;
};
// Check if the headermap maps the filename to a framework include
@@ -437,12 +444,10 @@
Filename = StringRef(MappedName.begin(), MappedName.size());
Optional Result = HM->LookupFile(Filename, HS.getFileMgr());
if (Result) {
- FixupSearchPath();
- return *Result;
+ return FixupSearchPathAndFindUsableModule(*Result);
}
} else if (auto Res = HS.getFileMgr().getOptionalFileRef(Dest)) {
-FixupSearchPath();
-return *Res;
+return FixupSearchPathAndFindUsableModule(*Res);
}
return None;
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D103930: [clang][HeaderSearch] Fix implicit module when using header maps
andrewjcg updated this revision to Diff 352824.
andrewjcg added a comment.
fix comment
Repository:
rG LLVM Github Monorepo
CHANGES SINCE LAST ACTION
https://reviews.llvm.org/D103930/new/
https://reviews.llvm.org/D103930
Files:
clang/lib/Lex/HeaderSearch.cpp
clang/test/Modules/Inputs/implicit-module-header-maps/a.h
clang/test/Modules/Inputs/implicit-module-header-maps/a.hmap.json
clang/test/Modules/Inputs/implicit-module-header-maps/a.module.modulemap
clang/test/Modules/Inputs/implicit-module-header-maps/b.hmap.json
clang/test/Modules/implicit-module-header-maps.cpp
Index: clang/test/Modules/implicit-module-header-maps.cpp
===
--- /dev/null
+++ clang/test/Modules/implicit-module-header-maps.cpp
@@ -0,0 +1,32 @@
+// RUN: rm -rf %t
+// RUN: mkdir %t
+// RUN: cd %t
+//
+// RUN: %hmaptool write %S/Inputs/implicit-module-header-maps/a.hmap.json %t/hmap
+//
+// RUN: mkdir -p %t/After
+// RUN: cp %S/Inputs/implicit-module-header-maps/a.h %t/After/Mapping.h
+// RUN: cp %S/Inputs/implicit-module-header-maps/a.module.modulemap %t/module.modulemap
+//
+// RUN: %clang -Rmodule-build -fmodules -fimplicit-modules -fimplicit-module-maps -fmodule-map-file=%t/module.modulemap -fsyntax-only %s -I %t/hmap
+//
+// RUN: rm -rf %t
+// RUN: mkdir %t
+// RUN: cd %t
+//
+// RUN: sed -e "s|OUTPUTS_DIR|%t|g" %S/Inputs/implicit-module-header-maps/b.hmap.json > %t/hmap.json
+// RUN: %hmaptool write %t/hmap.json %t/hmap
+//
+// RUN: mkdir -p %t/After
+// RUN: cp %S/Inputs/implicit-module-header-maps/a.h %t/After/Mapping.h
+// RUN: cp %S/Inputs/implicit-module-header-maps/a.module.modulemap %t/module.modulemap
+//
+// RUN: %clang -Rmodule-build -fmodules -fimplicit-modules -fimplicit-module-maps -fmodule-map-file=%t/module.modulemap -fsyntax-only %s -I %t/hmap
+
+#define FOO
+// This include will fail if:
+// 1) modules are't used, as the `FOO` define will propagate into the included
+//header and trip a `#error`, or
+// 2) header maps aren't usesd, as the header name doesn't exist and relies on
+//the header map to remap it to the real header.
+#include "Before/Mapping.h"
Index: clang/test/Modules/Inputs/implicit-module-header-maps/b.hmap.json
===
--- /dev/null
+++ clang/test/Modules/Inputs/implicit-module-header-maps/b.hmap.json
@@ -0,0 +1,6 @@
+{
+ "mappings" :
+{
+ "Before/Mapping.h" : "OUTPUTS_DIR/After/Mapping.h"
+}
+}
Index: clang/test/Modules/Inputs/implicit-module-header-maps/a.module.modulemap
===
--- /dev/null
+++ clang/test/Modules/Inputs/implicit-module-header-maps/a.module.modulemap
@@ -0,0 +1,3 @@
+module a {
+ header "After/Mapping.h"
+}
Index: clang/test/Modules/Inputs/implicit-module-header-maps/a.hmap.json
===
--- /dev/null
+++ clang/test/Modules/Inputs/implicit-module-header-maps/a.hmap.json
@@ -0,0 +1,7 @@
+{
+ "mappings" :
+{
+ "Before/Mapping.h" : "After/Mapping.h",
+ "After/Mapping.h" : "After/Mapping.h"
+}
+}
Index: clang/test/Modules/Inputs/implicit-module-header-maps/a.h
===
--- /dev/null
+++ clang/test/Modules/Inputs/implicit-module-header-maps/a.h
@@ -0,0 +1,3 @@
+#ifdef FOO
+#error foo
+#endif
Index: clang/lib/Lex/HeaderSearch.cpp
===
--- clang/lib/Lex/HeaderSearch.cpp
+++ clang/lib/Lex/HeaderSearch.cpp
@@ -417,7 +417,8 @@
IsInHeaderMap = true;
- auto FixupSearchPath = [&]() {
+ auto FixupSearchPathAndFindUsableModule =
+ [&](auto file) -> Optional {
if (SearchPath) {
StringRef SearchPathRef(getName());
SearchPath->clear();
@@ -427,6 +428,12 @@
RelativePath->clear();
RelativePath->append(Filename.begin(), Filename.end());
}
+if (!HS.findUsableModuleForHeader(
+(), file.getFileEntry().getDir(),
+RequestingModule, SuggestedModule, isSystemHeaderDirectory())) {
+ return None;
+}
+return file;
};
// Check if the headermap maps the filename to a framework include
@@ -437,12 +444,10 @@
Filename = StringRef(MappedName.begin(), MappedName.size());
Optional Result = HM->LookupFile(Filename, HS.getFileMgr());
if (Result) {
- FixupSearchPath();
- return *Result;
+ return FixupSearchPathAndFindUsableModule(*Result);
}
} else if (auto Res = HS.getFileMgr().getOptionalFileRef(Dest)) {
-FixupSearchPath();
-return *Res;
+return FixupSearchPathAndFindUsableModule(*Res);
}
return None;
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D103930: [clang][HeaderSearch] Fix implicit module when using header maps
andrewjcg added inline comments. Comment at: clang/test/Modules/implicit-module-header-maps.cpp:27 +#define FOO +// This include will fail if modules weren't used. The include name itself +// doesn't exist and relies on the header map to remap it to the real header. andrewjcg wrote: > dexonsmith wrote: > > Do you really mean if modules aren't used, or do you mean if header maps > > aren't used? > > > > (I think we want to find the same headers on disk whether or not modules > > are on... if this patch changes that, then I guess I'm not totally > > understanding why...) > Ahh, meant if header maps aren't used. Will fix. Ah no wait. The include should fail if either header maps or modules isn't used (header maps required for the remapping and modules required to prevent the `FOO` define from propagating into the included header and triggering the `#error`). Will update the comment. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D103930/new/ https://reviews.llvm.org/D103930 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D103930: [clang][HeaderSearch] Fix implicit module when using header maps
andrewjcg added inline comments. Comment at: clang/test/Modules/implicit-module-header-maps.cpp:27 +#define FOO +// This include will fail if modules weren't used. The include name itself +// doesn't exist and relies on the header map to remap it to the real header. dexonsmith wrote: > Do you really mean if modules aren't used, or do you mean if header maps > aren't used? > > (I think we want to find the same headers on disk whether or not modules are > on... if this patch changes that, then I guess I'm not totally understanding > why...) Ahh, meant if header maps aren't used. Will fix. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D103930/new/ https://reviews.llvm.org/D103930 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D103930: [clang][HeaderSearch] Fix implicit module when using header maps
andrewjcg updated this revision to Diff 352740.
andrewjcg added a comment.
fix sed for windows test
Repository:
rG LLVM Github Monorepo
CHANGES SINCE LAST ACTION
https://reviews.llvm.org/D103930/new/
https://reviews.llvm.org/D103930
Files:
clang/lib/Lex/HeaderSearch.cpp
clang/test/Modules/Inputs/implicit-module-header-maps/a.h
clang/test/Modules/Inputs/implicit-module-header-maps/a.hmap.json
clang/test/Modules/Inputs/implicit-module-header-maps/a.module.modulemap
clang/test/Modules/Inputs/implicit-module-header-maps/b.hmap.json
clang/test/Modules/implicit-module-header-maps.cpp
Index: clang/test/Modules/implicit-module-header-maps.cpp
===
--- /dev/null
+++ clang/test/Modules/implicit-module-header-maps.cpp
@@ -0,0 +1,29 @@
+// RUN: rm -rf %t
+// RUN: mkdir %t
+// RUN: cd %t
+//
+// RUN: %hmaptool write %S/Inputs/implicit-module-header-maps/a.hmap.json %t/hmap
+//
+// RUN: mkdir -p %t/After
+// RUN: cp %S/Inputs/implicit-module-header-maps/a.h %t/After/Mapping.h
+// RUN: cp %S/Inputs/implicit-module-header-maps/a.module.modulemap %t/module.modulemap
+//
+// RUN: %clang -Rmodule-build -fmodules -fimplicit-modules -fimplicit-module-maps -fmodule-map-file=%t/module.modulemap -fsyntax-only %s -I %t/hmap
+//
+// RUN: rm -rf %t
+// RUN: mkdir %t
+// RUN: cd %t
+//
+// RUN: sed -e "s|OUTPUTS_DIR|%t|g" %S/Inputs/implicit-module-header-maps/b.hmap.json > %t/hmap.json
+// RUN: %hmaptool write %t/hmap.json %t/hmap
+//
+// RUN: mkdir -p %t/After
+// RUN: cp %S/Inputs/implicit-module-header-maps/a.h %t/After/Mapping.h
+// RUN: cp %S/Inputs/implicit-module-header-maps/a.module.modulemap %t/module.modulemap
+//
+// RUN: %clang -Rmodule-build -fmodules -fimplicit-modules -fimplicit-module-maps -fmodule-map-file=%t/module.modulemap -fsyntax-only %s -I %t/hmap
+
+#define FOO
+// This include will fail if modules weren't used. The include name itself
+// doesn't exist and relies on the header map to remap it to the real header.
+#include "Before/Mapping.h"
Index: clang/test/Modules/Inputs/implicit-module-header-maps/b.hmap.json
===
--- /dev/null
+++ clang/test/Modules/Inputs/implicit-module-header-maps/b.hmap.json
@@ -0,0 +1,6 @@
+{
+ "mappings" :
+{
+ "Before/Mapping.h" : "OUTPUTS_DIR/After/Mapping.h"
+}
+}
Index: clang/test/Modules/Inputs/implicit-module-header-maps/a.module.modulemap
===
--- /dev/null
+++ clang/test/Modules/Inputs/implicit-module-header-maps/a.module.modulemap
@@ -0,0 +1,3 @@
+module a {
+ header "After/Mapping.h"
+}
Index: clang/test/Modules/Inputs/implicit-module-header-maps/a.hmap.json
===
--- /dev/null
+++ clang/test/Modules/Inputs/implicit-module-header-maps/a.hmap.json
@@ -0,0 +1,7 @@
+{
+ "mappings" :
+{
+ "Before/Mapping.h" : "After/Mapping.h",
+ "After/Mapping.h" : "After/Mapping.h"
+}
+}
Index: clang/test/Modules/Inputs/implicit-module-header-maps/a.h
===
--- /dev/null
+++ clang/test/Modules/Inputs/implicit-module-header-maps/a.h
@@ -0,0 +1,3 @@
+#ifdef FOO
+#error foo
+#endif
Index: clang/lib/Lex/HeaderSearch.cpp
===
--- clang/lib/Lex/HeaderSearch.cpp
+++ clang/lib/Lex/HeaderSearch.cpp
@@ -417,7 +417,8 @@
IsInHeaderMap = true;
- auto FixupSearchPath = [&]() {
+ auto FixupSearchPathAndFindUsableModule =
+ [&](auto file) -> Optional {
if (SearchPath) {
StringRef SearchPathRef(getName());
SearchPath->clear();
@@ -427,6 +428,12 @@
RelativePath->clear();
RelativePath->append(Filename.begin(), Filename.end());
}
+if (!HS.findUsableModuleForHeader(
+(), file.getFileEntry().getDir(),
+RequestingModule, SuggestedModule, isSystemHeaderDirectory())) {
+ return None;
+}
+return file;
};
// Check if the headermap maps the filename to a framework include
@@ -437,12 +444,10 @@
Filename = StringRef(MappedName.begin(), MappedName.size());
Optional Result = HM->LookupFile(Filename, HS.getFileMgr());
if (Result) {
- FixupSearchPath();
- return *Result;
+ return FixupSearchPathAndFindUsableModule(*Result);
}
} else if (auto Res = HS.getFileMgr().getOptionalFileRef(Dest)) {
-FixupSearchPath();
-return *Res;
+return FixupSearchPathAndFindUsableModule(*Res);
}
return None;
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D103930: [clang][HeaderSearch] Fix implicit module when using header maps
andrewjcg updated this revision to Diff 352299.
andrewjcg added a comment.
feedback
Repository:
rG LLVM Github Monorepo
CHANGES SINCE LAST ACTION
https://reviews.llvm.org/D103930/new/
https://reviews.llvm.org/D103930
Files:
clang/lib/Lex/HeaderSearch.cpp
clang/test/Modules/Inputs/implicit-module-header-maps/a.h
clang/test/Modules/Inputs/implicit-module-header-maps/a.hmap.json
clang/test/Modules/Inputs/implicit-module-header-maps/a.module.modulemap
clang/test/Modules/Inputs/implicit-module-header-maps/b.hmap.json
clang/test/Modules/implicit-module-header-maps.cpp
Index: clang/test/Modules/implicit-module-header-maps.cpp
===
--- /dev/null
+++ clang/test/Modules/implicit-module-header-maps.cpp
@@ -0,0 +1,29 @@
+// RUN: rm -rf %t
+// RUN: mkdir %t
+// RUN: cd %t
+//
+// RUN: %hmaptool write %S/Inputs/implicit-module-header-maps/a.hmap.json %t/hmap
+//
+// RUN: mkdir -p %t/After
+// RUN: cp %S/Inputs/implicit-module-header-maps/a.h %t/After/Mapping.h
+// RUN: cp %S/Inputs/implicit-module-header-maps/a.module.modulemap %t/module.modulemap
+//
+// RUN: %clang -Rmodule-build -fmodules -fimplicit-modules -fimplicit-module-maps -fmodule-map-file=%t/module.modulemap -fsyntax-only %s -I %t/hmap
+//
+// RUN: rm -rf %t
+// RUN: mkdir %t
+// RUN: cd %t
+//
+// RUN: sed -e "s:OUTPUTS_DIR:%t:g" %S/Inputs/implicit-module-header-maps/b.hmap.json > %t/hmap.json
+// RUN: %hmaptool write %t/hmap.json %t/hmap
+//
+// RUN: mkdir -p %t/After
+// RUN: cp %S/Inputs/implicit-module-header-maps/a.h %t/After/Mapping.h
+// RUN: cp %S/Inputs/implicit-module-header-maps/a.module.modulemap %t/module.modulemap
+//
+// RUN: %clang -Rmodule-build -fmodules -fimplicit-modules -fimplicit-module-maps -fmodule-map-file=%t/module.modulemap -fsyntax-only %s -I %t/hmap
+
+#define FOO
+// This include will fail if modules weren't used. The include name itself
+// doesn't exist and relies on the header map to remap it to the real header.
+#include "Before/Mapping.h"
Index: clang/test/Modules/Inputs/implicit-module-header-maps/b.hmap.json
===
--- /dev/null
+++ clang/test/Modules/Inputs/implicit-module-header-maps/b.hmap.json
@@ -0,0 +1,6 @@
+{
+ "mappings" :
+{
+ "Before/Mapping.h" : "OUTPUTS_DIR/After/Mapping.h"
+}
+}
Index: clang/test/Modules/Inputs/implicit-module-header-maps/a.module.modulemap
===
--- /dev/null
+++ clang/test/Modules/Inputs/implicit-module-header-maps/a.module.modulemap
@@ -0,0 +1,3 @@
+module a {
+ header "After/Mapping.h"
+}
Index: clang/test/Modules/Inputs/implicit-module-header-maps/a.hmap.json
===
--- /dev/null
+++ clang/test/Modules/Inputs/implicit-module-header-maps/a.hmap.json
@@ -0,0 +1,7 @@
+{
+ "mappings" :
+{
+ "Before/Mapping.h" : "After/Mapping.h",
+ "After/Mapping.h" : "After/Mapping.h"
+}
+}
Index: clang/test/Modules/Inputs/implicit-module-header-maps/a.h
===
--- /dev/null
+++ clang/test/Modules/Inputs/implicit-module-header-maps/a.h
@@ -0,0 +1,3 @@
+#ifdef FOO
+#error foo
+#endif
Index: clang/lib/Lex/HeaderSearch.cpp
===
--- clang/lib/Lex/HeaderSearch.cpp
+++ clang/lib/Lex/HeaderSearch.cpp
@@ -417,7 +417,8 @@
IsInHeaderMap = true;
- auto FixupSearchPath = [&]() {
+ auto FixupSearchPathAndFindUsableModule =
+ [&](auto file) -> Optional {
if (SearchPath) {
StringRef SearchPathRef(getName());
SearchPath->clear();
@@ -427,6 +428,12 @@
RelativePath->clear();
RelativePath->append(Filename.begin(), Filename.end());
}
+if (!HS.findUsableModuleForHeader(
+(), file.getFileEntry().getDir(),
+RequestingModule, SuggestedModule, isSystemHeaderDirectory())) {
+ return None;
+}
+return file;
};
// Check if the headermap maps the filename to a framework include
@@ -437,12 +444,10 @@
Filename = StringRef(MappedName.begin(), MappedName.size());
Optional Result = HM->LookupFile(Filename, HS.getFileMgr());
if (Result) {
- FixupSearchPath();
- return *Result;
+ return FixupSearchPathAndFindUsableModule(*Result);
}
} else if (auto Res = HS.getFileMgr().getOptionalFileRef(Dest)) {
-FixupSearchPath();
-return *Res;
+return FixupSearchPathAndFindUsableModule(*Res);
}
return None;
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D103930: [clang][HeaderSearch] Fix implicit module when using header maps
andrewjcg added a comment. Hmm, I can't repro the module test failures locally. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D103930/new/ https://reviews.llvm.org/D103930 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D103930: [clang][HeaderSearch] Fix implicit module when using header maps
andrewjcg updated this revision to Diff 351195.
andrewjcg added a comment.
rebase
Repository:
rG LLVM Github Monorepo
CHANGES SINCE LAST ACTION
https://reviews.llvm.org/D103930/new/
https://reviews.llvm.org/D103930
Files:
clang/lib/Lex/HeaderSearch.cpp
clang/test/Modules/Inputs/implicit-module-header-maps/a.h
clang/test/Modules/Inputs/implicit-module-header-maps/a.hmap.json
clang/test/Modules/Inputs/implicit-module-header-maps/a.module.modulemap
clang/test/Modules/Inputs/implicit-module-header-maps/b.hmap.json
clang/test/Modules/implicit-module-header-maps.cpp
Index: clang/test/Modules/implicit-module-header-maps.cpp
===
--- /dev/null
+++ clang/test/Modules/implicit-module-header-maps.cpp
@@ -0,0 +1,27 @@
+// RUN: rm -rf %T
+// RUN: mkdir %T
+// RUN: cd %T
+//
+// RUN: %hmaptool write %S/Inputs/implicit-module-header-maps/a.hmap.json
%T/hmap
+//
+// RUN: mkdir -p %T/After
+// RUN: cp %S/Inputs/implicit-module-header-maps/a.h %T/After/Mapping.h
+// RUN: cp %S/Inputs/implicit-module-header-maps/a.module.modulemap
%T/module.modulemap
+//
+// RUN: %clang -Rmodule-build -fmodules -fimplicit-modules
-fimplicit-module-maps -fmodule-map-file=%T/module.modulemap -fsyntax-only %s
-I %T/hmap
+//
+// RUN: rm -rf %T
+// RUN: mkdir %T
+// RUN: cd %T
+//
+// RUN: sed -e "s:OUTPUTS_DIR:%T:g"
%S/Inputs/implicit-module-header-maps/b.hmap.json > %T/hmap.json
+// RUN: %hmaptool write %T/hmap.json %T/hmap
+//
+// RUN: mkdir -p %T/After
+// RUN: cp %S/Inputs/implicit-module-header-maps/a.h %T/After/Mapping.h
+// RUN: cp %S/Inputs/implicit-module-header-maps/a.module.modulemap
%T/module.modulemap
+//
+// RUN: %clang -Rmodule-build -fmodules -fimplicit-modules
-fimplicit-module-maps -fmodule-map-file=%T/module.modulemap -fsyntax-only %s
-I %T/hmap
+
+#define FOO
+#include "Before/Mapping.h"
Index: clang/test/Modules/Inputs/implicit-module-header-maps/b.hmap.json
===
--- /dev/null
+++ clang/test/Modules/Inputs/implicit-module-header-maps/b.hmap.json
@@ -0,0 +1,6 @@
+{
+ "mappings" :
+{
+ "Before/Mapping.h" : "OUTPUTS_DIR/After/Mapping.h"
+}
+}
Index: clang/test/Modules/Inputs/implicit-module-header-maps/a.module.modulemap
===
--- /dev/null
+++ clang/test/Modules/Inputs/implicit-module-header-maps/a.module.modulemap
@@ -0,0 +1,3 @@
+module a {
+ header "After/Mapping.h"
+}
Index: clang/test/Modules/Inputs/implicit-module-header-maps/a.hmap.json
===
--- /dev/null
+++ clang/test/Modules/Inputs/implicit-module-header-maps/a.hmap.json
@@ -0,0 +1,7 @@
+{
+ "mappings" :
+{
+ "Before/Mapping.h" : "After/Mapping.h",
+ "After/Mapping.h" : "After/Mapping.h"
+}
+}
Index: clang/test/Modules/Inputs/implicit-module-header-maps/a.h
===
--- /dev/null
+++ clang/test/Modules/Inputs/implicit-module-header-maps/a.h
@@ -0,0 +1,3 @@
+#ifdef FOO
+#error foo
+#endif
Index: clang/lib/Lex/HeaderSearch.cpp
===
--- clang/lib/Lex/HeaderSearch.cpp
+++ clang/lib/Lex/HeaderSearch.cpp
@@ -438,10 +438,20 @@
Optional Result = HM->LookupFile(Filename, HS.getFileMgr());
if (Result) {
FixupSearchPath();
+ if (!HS.findUsableModuleForHeader(
+ >getFileEntry(), Result->getFileEntry().getDir(),
+ RequestingModule, SuggestedModule, isSystemHeaderDirectory())) {
+return None;
+ }
return *Result;
}
} else if (auto Res = HS.getFileMgr().getOptionalFileRef(Dest)) {
FixupSearchPath();
+if (!HS.findUsableModuleForHeader(
+>getFileEntry(), Res->getFileEntry().getDir(),
+RequestingModule, SuggestedModule, isSystemHeaderDirectory())) {
+ return None;
+}
return *Res;
}
Index: clang/test/Modules/implicit-module-header-maps.cpp
===
--- /dev/null
+++ clang/test/Modules/implicit-module-header-maps.cpp
@@ -0,0 +1,27 @@
+// RUN: rm -rf %T
+// RUN: mkdir %T
+// RUN: cd %T
+//
+// RUN: %hmaptool write %S/Inputs/implicit-module-header-maps/a.hmap.json %T/hmap
+//
+// RUN: mkdir -p %T/After
+// RUN: cp %S/Inputs/implicit-module-header-maps/a.h %T/After/Mapping.h
+// RUN: cp %S/Inputs/implicit-module-header-maps/a.module.modulemap %T/module.modulemap
+//
+// RUN: %clang -Rmodule-build -fmodules -fimplicit-modules -fimplicit-module-maps -fmodule-map-file=%T/module.modulemap -fsyntax-only %s -I %T/hmap
+//
+// RUN: rm -rf %T
+// RUN: mkdir %T
+// RUN: cd %T
+//
+// RUN: sed -e "s:OUTPUTS_DIR:%T:g" %S/Inputs/implicit-module-header-maps/b.hmap.json > %T/hmap.json
+// RUN: %hmaptool write %T/hmap.json %T/hmap
+//
+// RUN: mkdir -p %T/After
+// RUN: cp
[PATCH] D103930: [clang][HeaderSearch] Fix implicit module when using header maps
andrewjcg updated this revision to Diff 350767.
andrewjcg added a comment.
lint
Repository:
rG LLVM Github Monorepo
CHANGES SINCE LAST ACTION
https://reviews.llvm.org/D103930/new/
https://reviews.llvm.org/D103930
Files:
clang/lib/Lex/HeaderSearch.cpp
clang/test/Modules/Inputs/implicit-module-header-maps/a.h
clang/test/Modules/Inputs/implicit-module-header-maps/a.hmap.json
clang/test/Modules/Inputs/implicit-module-header-maps/a.module.modulemap
clang/test/Modules/Inputs/implicit-module-header-maps/b.hmap.json
clang/test/Modules/implicit-module-header-maps.cpp
Index: clang/test/Modules/implicit-module-header-maps.cpp
===
--- /dev/null
+++ clang/test/Modules/implicit-module-header-maps.cpp
@@ -0,0 +1,27 @@
+// RUN: rm -rf %T
+// RUN: mkdir %T
+// RUN: cd %T
+//
+// RUN: %hmaptool write %S/Inputs/implicit-module-header-maps/a.hmap.json
%T/hmap
+//
+// RUN: mkdir -p %T/After
+// RUN: cp %S/Inputs/implicit-module-header-maps/a.h %T/After/Mapping.h
+// RUN: cp %S/Inputs/implicit-module-header-maps/a.module.modulemap
%T/module.modulemap
+//
+// RUN: %clang -Rmodule-build -fmodules -fimplicit-modules
-fimplicit-module-maps -fmodule-map-file=%T/module.modulemap -fsyntax-only %s
-I %T/hmap
+//
+// RUN: rm -rf %T
+// RUN: mkdir %T
+// RUN: cd %T
+//
+// RUN: sed -e "s:OUTPUTS_DIR:%T:g"
%S/Inputs/implicit-module-header-maps/b.hmap.json > %T/hmap.json
+// RUN: %hmaptool write %T/hmap.json %T/hmap
+//
+// RUN: mkdir -p %T/After
+// RUN: cp %S/Inputs/implicit-module-header-maps/a.h %T/After/Mapping.h
+// RUN: cp %S/Inputs/implicit-module-header-maps/a.module.modulemap
%T/module.modulemap
+//
+// RUN: %clang -Rmodule-build -fmodules -fimplicit-modules
-fimplicit-module-maps -fmodule-map-file=%T/module.modulemap -fsyntax-only %s
-I %T/hmap
+
+#define FOO
+#include "Before/Mapping.h"
Index: clang/test/Modules/Inputs/implicit-module-header-maps/b.hmap.json
===
--- /dev/null
+++ clang/test/Modules/Inputs/implicit-module-header-maps/b.hmap.json
@@ -0,0 +1,6 @@
+{
+ "mappings" :
+{
+ "Before/Mapping.h" : "OUTPUTS_DIR/After/Mapping.h"
+}
+}
Index: clang/test/Modules/Inputs/implicit-module-header-maps/a.module.modulemap
===
--- /dev/null
+++ clang/test/Modules/Inputs/implicit-module-header-maps/a.module.modulemap
@@ -0,0 +1,3 @@
+module a {
+ header "After/Mapping.h"
+}
Index: clang/test/Modules/Inputs/implicit-module-header-maps/a.hmap.json
===
--- /dev/null
+++ clang/test/Modules/Inputs/implicit-module-header-maps/a.hmap.json
@@ -0,0 +1,7 @@
+{
+ "mappings" :
+{
+ "Before/Mapping.h" : "After/Mapping.h",
+ "After/Mapping.h" : "After/Mapping.h"
+}
+}
Index: clang/test/Modules/Inputs/implicit-module-header-maps/a.h
===
--- /dev/null
+++ clang/test/Modules/Inputs/implicit-module-header-maps/a.h
@@ -0,0 +1,3 @@
+#ifdef FOO
+#error foo
+#endif
Index: clang/lib/Lex/HeaderSearch.cpp
===
--- clang/lib/Lex/HeaderSearch.cpp
+++ clang/lib/Lex/HeaderSearch.cpp
@@ -438,10 +438,20 @@
Optional Result = HM->LookupFile(Filename, HS.getFileMgr());
if (Result) {
FixupSearchPath();
+ if (!HS.findUsableModuleForHeader(
+ >getFileEntry(), Result->getFileEntry().getDir(),
+ RequestingModule, SuggestedModule, isSystemHeaderDirectory())) {
+return None;
+ }
return *Result;
}
} else if (auto Res = HS.getFileMgr().getOptionalFileRef(Dest)) {
FixupSearchPath();
+if (!HS.findUsableModuleForHeader(
+>getFileEntry(), Res->getFileEntry().getDir(),
+RequestingModule, SuggestedModule, isSystemHeaderDirectory())) {
+ return None;
+}
return *Res;
}
Index: clang/test/Modules/implicit-module-header-maps.cpp
===
--- /dev/null
+++ clang/test/Modules/implicit-module-header-maps.cpp
@@ -0,0 +1,27 @@
+// RUN: rm -rf %T
+// RUN: mkdir %T
+// RUN: cd %T
+//
+// RUN: %hmaptool write %S/Inputs/implicit-module-header-maps/a.hmap.json %T/hmap
+//
+// RUN: mkdir -p %T/After
+// RUN: cp %S/Inputs/implicit-module-header-maps/a.h %T/After/Mapping.h
+// RUN: cp %S/Inputs/implicit-module-header-maps/a.module.modulemap %T/module.modulemap
+//
+// RUN: %clang -Rmodule-build -fmodules -fimplicit-modules -fimplicit-module-maps -fmodule-map-file=%T/module.modulemap -fsyntax-only %s -I %T/hmap
+//
+// RUN: rm -rf %T
+// RUN: mkdir %T
+// RUN: cd %T
+//
+// RUN: sed -e "s:OUTPUTS_DIR:%T:g" %S/Inputs/implicit-module-header-maps/b.hmap.json > %T/hmap.json
+// RUN: %hmaptool write %T/hmap.json %T/hmap
+//
+// RUN: mkdir -p %T/After
+// RUN: cp
[PATCH] D103930: [clang][HeaderSearch] Fix implicit module when using header maps
andrewjcg added inline comments. Comment at: clang/test/Modules/implicit-module-header-maps.cpp:27 +#define FOO +#include "Before/Mapping.h" This include will fail if modules weren't used. The include name itself doesn't exist and relies on the header map to remap it to the real header. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D103930/new/ https://reviews.llvm.org/D103930 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D103930: [clang][HeaderSearch] Fix implicit module when using header maps
andrewjcg added a comment. We were hitting this in our build environment when mixing header maps with clang module maps, where the use of the former would prevent properly associated an included header with it's module via the module map. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D103930/new/ https://reviews.llvm.org/D103930 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D103930: [clang][HeaderSearch] Fix implicit module when using header maps
andrewjcg created this revision.
Herald added a subscriber: wenlei.
andrewjcg updated this revision to Diff 350757.
andrewjcg added a comment.
andrewjcg edited the summary of this revision.
andrewjcg added reviewers: bruno, rsmith.
andrewjcg published this revision for review.
Herald added a project: clang.
Herald added a subscriber: cfe-commits.
add tests
Previously, if a header was found via in a header map, and not just remapped.
we wouldn't also find the module it maps to when using implicit modules (for
module maps that were explicitly loaded).
This diff just updates these code paths to also locate the owning module via
`findUsableModuleForHeader`.
Repository:
rG LLVM Github Monorepo
https://reviews.llvm.org/D103930
Files:
clang/lib/Lex/HeaderSearch.cpp
clang/test/Modules/Inputs/implicit-module-header-maps/a.h
clang/test/Modules/Inputs/implicit-module-header-maps/a.hmap.json
clang/test/Modules/Inputs/implicit-module-header-maps/a.module.modulemap
clang/test/Modules/Inputs/implicit-module-header-maps/b.hmap.json
clang/test/Modules/implicit-module-header-maps.cpp
Index: clang/test/Modules/implicit-module-header-maps.cpp
===
--- /dev/null
+++ clang/test/Modules/implicit-module-header-maps.cpp
@@ -0,0 +1,27 @@
+// RUN: rm -rf %T
+// RUN: mkdir %T
+// RUN: cd %T
+//
+// RUN: %hmaptool write %S/Inputs/implicit-module-header-maps/a.hmap.json
%T/hmap
+//
+// RUN: mkdir -p %T/After
+// RUN: cp %S/Inputs/implicit-module-header-maps/a.h %T/After/Mapping.h
+// RUN: cp %S/Inputs/implicit-module-header-maps/a.module.modulemap
%T/module.modulemap
+//
+// RUN: %clang -Rmodule-build -fmodules -fimplicit-modules
-fimplicit-module-maps -fmodule-map-file=%T/module.modulemap -fsyntax-only %s
-I %T/hmap
+//
+// RUN: rm -rf %T
+// RUN: mkdir %T
+// RUN: cd %T
+//
+// RUN: sed -e "s:OUTPUTS_DIR:%T:g"
%S/Inputs/implicit-module-header-maps/b.hmap.json > %T/hmap.json
+// RUN: %hmaptool write %T/hmap.json %T/hmap
+//
+// RUN: mkdir -p %T/After
+// RUN: cp %S/Inputs/implicit-module-header-maps/a.h %T/After/Mapping.h
+// RUN: cp %S/Inputs/implicit-module-header-maps/a.module.modulemap
%T/module.modulemap
+//
+// RUN: %clang -Rmodule-build -fmodules -fimplicit-modules
-fimplicit-module-maps -fmodule-map-file=%T/module.modulemap -fsyntax-only %s
-I %T/hmap
+
+#define FOO
+#include "Before/Mapping.h"
Index: clang/test/Modules/Inputs/implicit-module-header-maps/b.hmap.json
===
--- /dev/null
+++ clang/test/Modules/Inputs/implicit-module-header-maps/b.hmap.json
@@ -0,0 +1,6 @@
+{
+ "mappings" :
+{
+ "Before/Mapping.h" : "OUTPUTS_DIR/After/Mapping.h"
+}
+}
Index: clang/test/Modules/Inputs/implicit-module-header-maps/a.module.modulemap
===
--- /dev/null
+++ clang/test/Modules/Inputs/implicit-module-header-maps/a.module.modulemap
@@ -0,0 +1,3 @@
+module a {
+ header "After/Mapping.h"
+}
Index: clang/test/Modules/Inputs/implicit-module-header-maps/a.hmap.json
===
--- /dev/null
+++ clang/test/Modules/Inputs/implicit-module-header-maps/a.hmap.json
@@ -0,0 +1,7 @@
+{
+ "mappings" :
+{
+ "Before/Mapping.h" : "After/Mapping.h",
+ "After/Mapping.h" : "After/Mapping.h"
+}
+}
Index: clang/test/Modules/Inputs/implicit-module-header-maps/a.h
===
--- /dev/null
+++ clang/test/Modules/Inputs/implicit-module-header-maps/a.h
@@ -0,0 +1,3 @@
+#ifdef FOO
+#error foo
+#endif
Index: clang/lib/Lex/HeaderSearch.cpp
===
--- clang/lib/Lex/HeaderSearch.cpp
+++ clang/lib/Lex/HeaderSearch.cpp
@@ -438,10 +438,22 @@
Optional Result = HM->LookupFile(Filename, HS.getFileMgr());
if (Result) {
FixupSearchPath();
+ if (!HS.findUsableModuleForHeader(>getFileEntry(),
+Result->getFileEntry().getDir(),
+RequestingModule, SuggestedModule,
+isSystemHeaderDirectory())) {
+return None;
+ }
return *Result;
}
} else if (auto Res = HS.getFileMgr().getOptionalFileRef(Dest)) {
FixupSearchPath();
+if (!HS.findUsableModuleForHeader(>getFileEntry(),
+ Res->getFileEntry().getDir(),
+ RequestingModule, SuggestedModule,
+ isSystemHeaderDirectory())) {
+ return None;
+}
return *Res;
}
Index: clang/test/Modules/implicit-module-header-maps.cpp
===
--- /dev/null
+++ clang/test/Modules/implicit-module-header-maps.cpp
@@ -0,0 +1,27 @@
+// RUN: rm -rf %T
+// RUN: mkdir %T
+// RUN: cd %T
+//
[PATCH] D83154: clang: Add -fcoverage-prefix-map
andrewjcg added inline comments. Comment at: clang/lib/CodeGen/CoverageMappingGen.cpp:1334 + llvm::SmallString<256> Path(Filename); + llvm::sys::fs::make_absolute(Path); + llvm::sys::path::remove_dots(Path, /*remove_dot_dot=*/true); keith wrote: > rnk wrote: > > keith wrote: > > > rnk wrote: > > > > Please only make the path absolute if nothing in the prefix map > > > > matches. Otherwise, the user must embed the CWD into the prefix map, > > > > which is needlessly difficult for the build system. I believe it is > > > > also consistent with the way that the debug info prefix map works. It > > > > appears to operate on the possibly relative source paths received on > > > > the command line (-I...). > > > Are you suggesting that I try to remap them relatively, and if that > > > fails, absolutize it and attempt the remapping again? Or don't absolutize > > > them at all anymore? > > I'd prefer to do the remapping once without any absolutization, and if that > > fails, preserve the behavior of absolutizing. > > > > It would be my preference to not absolutize paths at all, since that is > > what is done for debug info. However, @vsk implemented things this way, and > > I do understand that this is convenient default behavior for most users: > > the paths in the coverage are valid anywhere on their system. So, changing > > this behavior is out of scope for this patch. > I'm not sure how this changed would work for our use case. With bazel the > usage of this works something like: > > 1. Relative paths are passed to the compiler `clang ... foo.c` > 2. We would normally do `-fprofile-prefix-map=$PWD=.` to remap them > > I think if we made this change, we would either have to: > > 1. Make the paths we pass absolute, which we couldn't do for reproducibility > 2. Add some known prefix to the paths, like `.`, so we could > `-fprofile-prefix-map=.=.` just to avoid this absolutizing codepath > > So I think without actually removing the absolutizing behavior at the same > time, this wouldn't work as we'd hope. > > Let me know if I'm mis-understanding your suggestion! > > If we did what I said above, I think it would work since relative path > remapping would fail, but then prefix mapping the absolute path would work. FWIW, there's a similar issue with doing the absolutizing for the Buck build tool, which by default sets `-fdebug-compilation-dir=.` for all compilations, then expects to use `-fdebug-prefix-map` (or `-ffile-prefix-map`) to fixup relative paths of sandboxed header symlink trees to point to their *real* paths (e.g. something like `clang -c -fdebug-compilation-dir=. -Iheader-tree-sandbox -ffile-prefix-map=header-tree-sandbox=original_dir`) (e.g. https://github.com/facebook/buck/blob/master/src/com/facebook/buck/cxx/toolchain/PrefixMapDebugPathSanitizer.java#L134). So, in this case, *not* absolutizing would help here, but it kind of feels that the real issue is that there's not an equivalent `-fprofile-compilation-dir` to opt-out of the absolutizing of profile paths (which is orthogonal to this change)... Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D83154/new/ https://reviews.llvm.org/D83154 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D86853: [modules] Fix crash in call to `FunctionDecl::setPure()`
andrewjcg updated this revision to Diff 289506.
andrewjcg marked 2 inline comments as done.
andrewjcg added a comment.
feedback
Repository:
rG LLVM Github Monorepo
CHANGES SINCE LAST ACTION
https://reviews.llvm.org/D86853/new/
https://reviews.llvm.org/D86853
Files:
clang/lib/Serialization/ASTReaderDecl.cpp
clang/test/Modules/Inputs/set-pure-crash/a.h
clang/test/Modules/Inputs/set-pure-crash/b.h
clang/test/Modules/Inputs/set-pure-crash/c.h
clang/test/Modules/Inputs/set-pure-crash/module.modulemap
clang/test/Modules/set-pure-crash.cpp
Index: clang/test/Modules/set-pure-crash.cpp
===
--- /dev/null
+++ clang/test/Modules/set-pure-crash.cpp
@@ -0,0 +1,9 @@
+// RUN: rm -rf %t
+// RUN: %clang_cc1 -fsyntax-only -fmodules -fimplicit-module-maps -fmodules-cache-path=%t -x c++ -I %S/Inputs/set-pure-crash -verify %s -o %t
+
+// expected-no-diagnostics
+
+#include "b.h"
+#include "c.h"
+
+auto t = simple();
Index: clang/test/Modules/Inputs/set-pure-crash/module.modulemap
===
--- /dev/null
+++ clang/test/Modules/Inputs/set-pure-crash/module.modulemap
@@ -0,0 +1,11 @@
+module a {
+ header "a.h"
+}
+
+module b {
+ header "b.h"
+}
+
+module c {
+ header "c.h"
+}
Index: clang/test/Modules/Inputs/set-pure-crash/c.h
===
--- /dev/null
+++ clang/test/Modules/Inputs/set-pure-crash/c.h
@@ -0,0 +1,5 @@
+#pragma once
+
+template
+struct simple {
+};
Index: clang/test/Modules/Inputs/set-pure-crash/b.h
===
--- /dev/null
+++ clang/test/Modules/Inputs/set-pure-crash/b.h
@@ -0,0 +1,14 @@
+#pragma once
+
+#include "a.h"
+#include "c.h"
+
+template >
+void foo(Fun) {}
+
+class Child : public Base {
+public:
+ void func() {
+foo([]() {});
+ }
+};
Index: clang/test/Modules/Inputs/set-pure-crash/a.h
===
--- /dev/null
+++ clang/test/Modules/Inputs/set-pure-crash/a.h
@@ -0,0 +1,11 @@
+#pragma once
+
+struct Tag {};
+
+template
+class Base {
+public:
+ virtual void func() = 0;
+};
+
+Base bar();
Index: clang/lib/Serialization/ASTReaderDecl.cpp
===
--- clang/lib/Serialization/ASTReaderDecl.cpp
+++ clang/lib/Serialization/ASTReaderDecl.cpp
@@ -864,7 +864,10 @@
FD->setInlineSpecified(Record.readInt());
FD->setImplicitlyInline(Record.readInt());
FD->setVirtualAsWritten(Record.readInt());
- FD->setPure(Record.readInt());
+ // We defer calling `FunctionDecl::setPure()` here as for methods of
+ // `CXXTemplateSpecializationDecl`s, we may not have connected up the
+ // definition (which is required for `setPure`).
+ const bool Pure = Record.readInt();
FD->setHasInheritedPrototype(Record.readInt());
FD->setHasWrittenPrototype(Record.readInt());
FD->setDeletedAsWritten(Record.readInt());
@@ -1012,6 +1015,10 @@
}
}
+ // Defer calling `setPure` until merging above has guaranteed we've set
+ // `DefinitionData` (as this will need to access it).
+ FD->setPure(Pure);
+
// Read in the parameters.
unsigned NumParams = Record.readInt();
SmallVector Params;
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D86802: [Modules] Don't parse/load explicit module maps if modules are disabled
andrewjcg abandoned this revision. andrewjcg added a comment. Ahh, I see, make sense. The motivating issue was due to an apparent bug where realpaths in umbrella dir support for module map files get leaked into dep files for includes starting with `..` (e.g. `#include "../foo.h"`) in non-modular builds. I'll try get a repo together for a bug report for that issue instead. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D86802/new/ https://reviews.llvm.org/D86802 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D86802: [Modules] Don't parse/load explicit module maps if modules are disabled
andrewjcg updated this revision to Diff 289236.
andrewjcg added a comment.
simplify test
Repository:
rG LLVM Github Monorepo
CHANGES SINCE LAST ACTION
https://reviews.llvm.org/D86802/new/
https://reviews.llvm.org/D86802
Files:
clang/lib/Frontend/FrontendAction.cpp
clang/test/Modules/explicit-module-maps.cpp
Index: clang/test/Modules/explicit-module-maps.cpp
===
--- /dev/null
+++ clang/test/Modules/explicit-module-maps.cpp
@@ -0,0 +1,8 @@
+// Test that an explicit module map (which fails to parse/load in this case)
+// isn't used if modules aren't enabled.
+
+// RUN: rm -rf %t
+// RUN: mkdir %t
+// RUN: echo 'DOES NOT PARSE!' > %t/invalid.modulemap
+// RUN: %clang_cc1 -fmodule-map-file=%t/invalid.modulemap -verify %s
+// expected-no-diagnostics
Index: clang/lib/Frontend/FrontendAction.cpp
===
--- clang/lib/Frontend/FrontendAction.cpp
+++ clang/lib/Frontend/FrontendAction.cpp
@@ -807,12 +807,14 @@
goto failure;
// If we were asked to load any module map files, do so now.
- for (const auto : CI.getFrontendOpts().ModuleMapFiles) {
-if (auto File = CI.getFileManager().getFile(Filename))
- CI.getPreprocessor().getHeaderSearchInfo().loadModuleMapFile(
- *File, /*IsSystem*/false);
-else
- CI.getDiagnostics().Report(diag::err_module_map_not_found) << Filename;
+ if (CI.getLangOpts().Modules) {
+for (const auto : CI.getFrontendOpts().ModuleMapFiles) {
+ if (auto File = CI.getFileManager().getFile(Filename))
+CI.getPreprocessor().getHeaderSearchInfo().loadModuleMapFile(
+*File, /*IsSystem*/ false);
+ else
+CI.getDiagnostics().Report(diag::err_module_map_not_found) << Filename;
+}
}
// Add a module declaration scope so that modules from -fmodule-map-file
Index: clang/test/Modules/explicit-module-maps.cpp
===
--- /dev/null
+++ clang/test/Modules/explicit-module-maps.cpp
@@ -0,0 +1,8 @@
+// Test that an explicit module map (which fails to parse/load in this case)
+// isn't used if modules aren't enabled.
+
+// RUN: rm -rf %t
+// RUN: mkdir %t
+// RUN: echo 'DOES NOT PARSE!' > %t/invalid.modulemap
+// RUN: %clang_cc1 -fmodule-map-file=%t/invalid.modulemap -verify %s
+// expected-no-diagnostics
Index: clang/lib/Frontend/FrontendAction.cpp
===
--- clang/lib/Frontend/FrontendAction.cpp
+++ clang/lib/Frontend/FrontendAction.cpp
@@ -807,12 +807,14 @@
goto failure;
// If we were asked to load any module map files, do so now.
- for (const auto : CI.getFrontendOpts().ModuleMapFiles) {
-if (auto File = CI.getFileManager().getFile(Filename))
- CI.getPreprocessor().getHeaderSearchInfo().loadModuleMapFile(
- *File, /*IsSystem*/false);
-else
- CI.getDiagnostics().Report(diag::err_module_map_not_found) << Filename;
+ if (CI.getLangOpts().Modules) {
+for (const auto : CI.getFrontendOpts().ModuleMapFiles) {
+ if (auto File = CI.getFileManager().getFile(Filename))
+CI.getPreprocessor().getHeaderSearchInfo().loadModuleMapFile(
+*File, /*IsSystem*/ false);
+ else
+CI.getDiagnostics().Report(diag::err_module_map_not_found) << Filename;
+}
}
// Add a module declaration scope so that modules from -fmodule-map-file
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D86853: [modules] Repro for pure virtual base class method crash
andrewjcg created this revision.
Herald added subscribers: cfe-commits, danielkiss.
Herald added a project: clang.
andrewjcg requested review of this revision.
Repository:
rG LLVM Github Monorepo
https://reviews.llvm.org/D86853
Files:
clang/lib/Serialization/ASTReaderDecl.cpp
clang/test/Modules/Inputs/set-pure-crash/a.h
clang/test/Modules/Inputs/set-pure-crash/b.h
clang/test/Modules/Inputs/set-pure-crash/c.h
clang/test/Modules/Inputs/set-pure-crash/module.modulemap
clang/test/Modules/set-pure-crash.cpp
Index: clang/test/Modules/set-pure-crash.cpp
===
--- /dev/null
+++ clang/test/Modules/set-pure-crash.cpp
@@ -0,0 +1,9 @@
+// RUN: rm -rf %t
+// RUN: %clang_cc1 -O0 -emit-llvm -fmodules -fimplicit-module-maps -fmodules-cache-path=%t -x c++ -I %S/Inputs/set-pure-crash -verify %s -o %t
+
+// expected-no-diagnostics
+
+#include "b.h"
+#include "c.h"
+
+auto t = simple();
Index: clang/test/Modules/Inputs/set-pure-crash/module.modulemap
===
--- /dev/null
+++ clang/test/Modules/Inputs/set-pure-crash/module.modulemap
@@ -0,0 +1,11 @@
+module a {
+ header "a.h"
+}
+
+module b {
+ header "b.h"
+}
+
+module c {
+ header "c.h"
+}
Index: clang/test/Modules/Inputs/set-pure-crash/c.h
===
--- /dev/null
+++ clang/test/Modules/Inputs/set-pure-crash/c.h
@@ -0,0 +1,5 @@
+#pragma once
+
+template
+struct simple {
+};
Index: clang/test/Modules/Inputs/set-pure-crash/b.h
===
--- /dev/null
+++ clang/test/Modules/Inputs/set-pure-crash/b.h
@@ -0,0 +1,14 @@
+#pragma once
+
+#include "a.h"
+#include "c.h"
+
+template >
+void foo(Fun) {}
+
+class Child : public Base {
+public:
+ void func() {
+foo([]() {});
+ }
+};
Index: clang/test/Modules/Inputs/set-pure-crash/a.h
===
--- /dev/null
+++ clang/test/Modules/Inputs/set-pure-crash/a.h
@@ -0,0 +1,11 @@
+#pragma once
+
+struct Tag {};
+
+template
+class Base {
+public:
+ virtual void func() = 0;
+};
+
+Base bar();
Index: clang/lib/Serialization/ASTReaderDecl.cpp
===
--- clang/lib/Serialization/ASTReaderDecl.cpp
+++ clang/lib/Serialization/ASTReaderDecl.cpp
@@ -864,7 +864,10 @@
FD->setInlineSpecified(Record.readInt());
FD->setImplicitlyInline(Record.readInt());
FD->setVirtualAsWritten(Record.readInt());
- FD->setPure(Record.readInt());
+ // We defer calling `FunctionDecl::setPure()` here as for methods of
+ // `CXXTemplateSpecializationDecl`s, we may not have connected up the
+ // definition (which is required for `setPure`).
+ const bool pure = Record.readInt();
FD->setHasInheritedPrototype(Record.readInt());
FD->setHasWrittenPrototype(Record.readInt());
FD->setDeletedAsWritten(Record.readInt());
@@ -1012,6 +1015,10 @@
}
}
+ // Defer calling `setPure` until merging above has guaranteed we've set
+ // `DefinitionData` (as this will need to access it).
+ FD->setPure(pure);
+
// Read in the parameters.
unsigned NumParams = Record.readInt();
SmallVector Params;
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D86802: [Modules] Don't parse/load explicit module maps if modules are disabled
andrewjcg updated this revision to Diff 288695.
andrewjcg added a comment.
add test
Repository:
rG LLVM Github Monorepo
CHANGES SINCE LAST ACTION
https://reviews.llvm.org/D86802/new/
https://reviews.llvm.org/D86802
Files:
clang/lib/Frontend/FrontendAction.cpp
clang/test/Modules/Inputs/explicit-module-maps/invalid.modulemap
clang/test/Modules/explicit-module-maps.cpp
Index: clang/test/Modules/explicit-module-maps.cpp
===
--- /dev/null
+++ clang/test/Modules/explicit-module-maps.cpp
@@ -0,0 +1,5 @@
+// Test that an explicit module map (which fails to parse/load in this case)
+// isn't used if modules aren't enabled.
+
+// RUN: %clang_cc1
-fmodule-map-file=%S/Inputs/explicit-module-maps/invalid.modulemap -verify %s
+// expected-no-diagnostics
Index: clang/test/Modules/Inputs/explicit-module-maps/invalid.modulemap
===
--- /dev/null
+++ clang/test/Modules/Inputs/explicit-module-maps/invalid.modulemap
@@ -0,0 +1 @@
+DOES NOT PARSE!
Index: clang/lib/Frontend/FrontendAction.cpp
===
--- clang/lib/Frontend/FrontendAction.cpp
+++ clang/lib/Frontend/FrontendAction.cpp
@@ -807,12 +807,14 @@
goto failure;
// If we were asked to load any module map files, do so now.
- for (const auto : CI.getFrontendOpts().ModuleMapFiles) {
-if (auto File = CI.getFileManager().getFile(Filename))
- CI.getPreprocessor().getHeaderSearchInfo().loadModuleMapFile(
- *File, /*IsSystem*/false);
-else
- CI.getDiagnostics().Report(diag::err_module_map_not_found) << Filename;
+ if (CI.getLangOpts().Modules) {
+for (const auto : CI.getFrontendOpts().ModuleMapFiles) {
+ if (auto File = CI.getFileManager().getFile(Filename))
+CI.getPreprocessor().getHeaderSearchInfo().loadModuleMapFile(
+*File, /*IsSystem*/ false);
+ else
+CI.getDiagnostics().Report(diag::err_module_map_not_found) << Filename;
+}
}
// Add a module declaration scope so that modules from -fmodule-map-file
Index: clang/test/Modules/explicit-module-maps.cpp
===
--- /dev/null
+++ clang/test/Modules/explicit-module-maps.cpp
@@ -0,0 +1,5 @@
+// Test that an explicit module map (which fails to parse/load in this case)
+// isn't used if modules aren't enabled.
+
+// RUN: %clang_cc1 -fmodule-map-file=%S/Inputs/explicit-module-maps/invalid.modulemap -verify %s
+// expected-no-diagnostics
Index: clang/test/Modules/Inputs/explicit-module-maps/invalid.modulemap
===
--- /dev/null
+++ clang/test/Modules/Inputs/explicit-module-maps/invalid.modulemap
@@ -0,0 +1 @@
+DOES NOT PARSE!
Index: clang/lib/Frontend/FrontendAction.cpp
===
--- clang/lib/Frontend/FrontendAction.cpp
+++ clang/lib/Frontend/FrontendAction.cpp
@@ -807,12 +807,14 @@
goto failure;
// If we were asked to load any module map files, do so now.
- for (const auto : CI.getFrontendOpts().ModuleMapFiles) {
-if (auto File = CI.getFileManager().getFile(Filename))
- CI.getPreprocessor().getHeaderSearchInfo().loadModuleMapFile(
- *File, /*IsSystem*/false);
-else
- CI.getDiagnostics().Report(diag::err_module_map_not_found) << Filename;
+ if (CI.getLangOpts().Modules) {
+for (const auto : CI.getFrontendOpts().ModuleMapFiles) {
+ if (auto File = CI.getFileManager().getFile(Filename))
+CI.getPreprocessor().getHeaderSearchInfo().loadModuleMapFile(
+*File, /*IsSystem*/ false);
+ else
+CI.getDiagnostics().Report(diag::err_module_map_not_found) << Filename;
+}
}
// Add a module declaration scope so that modules from -fmodule-map-file
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D86802: [Modules] Don't parse/load explicit module maps if modules are disabled
andrewjcg added a comment. > How about a malformed module map is not loaded and gives no errors? Heh yeah, was thinking the same :) Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D86802/new/ https://reviews.llvm.org/D86802 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D86802: [Modules] Don't parse/load explicit module maps if modules are disabled
andrewjcg added a comment. > Can you add a simple testcase to prove the point of the change? Yup, will do! Comment at: clang/lib/Frontend/FrontendAction.cpp:814 +CI.getPreprocessor().getHeaderSearchInfo().loadModuleMapFile( +*File, /*IsSystem*/ false); + else bruno wrote: > Is this clang-formatted? Ahh, yeah. Should I avoid this sort of thing (and e.g. instead do this in explicit codemod/refactoring diffs)? Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D86802/new/ https://reviews.llvm.org/D86802 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D86802: [Modules] Don't parse/load explicit module maps if modules are disabled
andrewjcg created this revision.
Herald added a project: clang.
Herald added a subscriber: cfe-commits.
andrewjcg requested review of this revision.
In some build environments/systems, flags for explicit module map files
may be propagated up to dependents which may not choose to enable use of
modules (e.g. if a particular compilation doesn't currently work in
modular builds).
In this case, the explicit module files are still passed, even though
they're not used (and can trigger additional I/O at header search path,
like realpath'ing include roots if an explicit module map contains a
umbrella dir).
This diff avoids parsing/loading these module map files if modules
haven't been enabled.
Repository:
rG LLVM Github Monorepo
https://reviews.llvm.org/D86802
Files:
clang/lib/Frontend/FrontendAction.cpp
Index: clang/lib/Frontend/FrontendAction.cpp
===
--- clang/lib/Frontend/FrontendAction.cpp
+++ clang/lib/Frontend/FrontendAction.cpp
@@ -807,12 +807,14 @@
goto failure;
// If we were asked to load any module map files, do so now.
- for (const auto : CI.getFrontendOpts().ModuleMapFiles) {
-if (auto File = CI.getFileManager().getFile(Filename))
- CI.getPreprocessor().getHeaderSearchInfo().loadModuleMapFile(
- *File, /*IsSystem*/false);
-else
- CI.getDiagnostics().Report(diag::err_module_map_not_found) << Filename;
+ if (CI.getLangOpts().Modules) {
+for (const auto : CI.getFrontendOpts().ModuleMapFiles) {
+ if (auto File = CI.getFileManager().getFile(Filename))
+CI.getPreprocessor().getHeaderSearchInfo().loadModuleMapFile(
+*File, /*IsSystem*/ false);
+ else
+CI.getDiagnostics().Report(diag::err_module_map_not_found) << Filename;
+}
}
// Add a module declaration scope so that modules from -fmodule-map-file
Index: clang/lib/Frontend/FrontendAction.cpp
===
--- clang/lib/Frontend/FrontendAction.cpp
+++ clang/lib/Frontend/FrontendAction.cpp
@@ -807,12 +807,14 @@
goto failure;
// If we were asked to load any module map files, do so now.
- for (const auto : CI.getFrontendOpts().ModuleMapFiles) {
-if (auto File = CI.getFileManager().getFile(Filename))
- CI.getPreprocessor().getHeaderSearchInfo().loadModuleMapFile(
- *File, /*IsSystem*/false);
-else
- CI.getDiagnostics().Report(diag::err_module_map_not_found) << Filename;
+ if (CI.getLangOpts().Modules) {
+for (const auto : CI.getFrontendOpts().ModuleMapFiles) {
+ if (auto File = CI.getFileManager().getFile(Filename))
+CI.getPreprocessor().getHeaderSearchInfo().loadModuleMapFile(
+*File, /*IsSystem*/ false);
+ else
+CI.getDiagnostics().Report(diag::err_module_map_not_found) << Filename;
+}
}
// Add a module declaration scope so that modules from -fmodule-map-file
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D85084: [modules] Repro for pure virtual base class method crash
andrewjcg created this revision.
Herald added a project: clang.
Herald added a subscriber: cfe-commits.
andrewjcg requested review of this revision.
Repository:
rG LLVM Github Monorepo
https://reviews.llvm.org/D85084
Files:
clang/test/Modules/Inputs/set-pure-crash/a.h
clang/test/Modules/Inputs/set-pure-crash/b.h
clang/test/Modules/Inputs/set-pure-crash/c.h
clang/test/Modules/Inputs/set-pure-crash/module.modulemap
clang/test/Modules/set-pure-crash.cpp
Index: clang/test/Modules/set-pure-crash.cpp
===
--- /dev/null
+++ clang/test/Modules/set-pure-crash.cpp
@@ -0,0 +1,9 @@
+// RUN: rm -rf %t
+// RUN: %clang_cc1 -O0 -emit-llvm -fmodules -fimplicit-module-maps
-fmodules-cache-path=%t -x c++ -I %S/Inputs/set-pure-crash -verify %s -o %t
+
+// expected-no-diagnostics
+
+#include "b.h"
+#include "c.h"
+
+auto t = func();
Index: clang/test/Modules/Inputs/set-pure-crash/module.modulemap
===
--- /dev/null
+++ clang/test/Modules/Inputs/set-pure-crash/module.modulemap
@@ -0,0 +1,11 @@
+module a {
+ header "a.h"
+}
+
+module b {
+ header "b.h"
+}
+
+module c {
+ header "c.h"
+}
Index: clang/test/Modules/Inputs/set-pure-crash/c.h
===
--- /dev/null
+++ clang/test/Modules/Inputs/set-pure-crash/c.h
@@ -0,0 +1,5 @@
+#pragma once
+
+template
+struct func {
+};
Index: clang/test/Modules/Inputs/set-pure-crash/b.h
===
--- /dev/null
+++ clang/test/Modules/Inputs/set-pure-crash/b.h
@@ -0,0 +1,14 @@
+#pragma once
+
+#include "a.h"
+#include "c.h"
+
+template >
+void foo(Fun) {}
+
+class Child : public Base {
+public:
+ void func() {
+foo([]() {});
+ }
+};
Index: clang/test/Modules/Inputs/set-pure-crash/a.h
===
--- /dev/null
+++ clang/test/Modules/Inputs/set-pure-crash/a.h
@@ -0,0 +1,11 @@
+#pragma once
+
+struct Tag {};
+
+template
+class Base {
+public:
+ virtual void func() = 0;
+};
+
+Base bar();
Index: clang/test/Modules/set-pure-crash.cpp
===
--- /dev/null
+++ clang/test/Modules/set-pure-crash.cpp
@@ -0,0 +1,9 @@
+// RUN: rm -rf %t
+// RUN: %clang_cc1 -O0 -emit-llvm -fmodules -fimplicit-module-maps -fmodules-cache-path=%t -x c++ -I %S/Inputs/set-pure-crash -verify %s -o %t
+
+// expected-no-diagnostics
+
+#include "b.h"
+#include "c.h"
+
+auto t = func();
Index: clang/test/Modules/Inputs/set-pure-crash/module.modulemap
===
--- /dev/null
+++ clang/test/Modules/Inputs/set-pure-crash/module.modulemap
@@ -0,0 +1,11 @@
+module a {
+ header "a.h"
+}
+
+module b {
+ header "b.h"
+}
+
+module c {
+ header "c.h"
+}
Index: clang/test/Modules/Inputs/set-pure-crash/c.h
===
--- /dev/null
+++ clang/test/Modules/Inputs/set-pure-crash/c.h
@@ -0,0 +1,5 @@
+#pragma once
+
+template
+struct func {
+};
Index: clang/test/Modules/Inputs/set-pure-crash/b.h
===
--- /dev/null
+++ clang/test/Modules/Inputs/set-pure-crash/b.h
@@ -0,0 +1,14 @@
+#pragma once
+
+#include "a.h"
+#include "c.h"
+
+template >
+void foo(Fun) {}
+
+class Child : public Base {
+public:
+ void func() {
+foo([]() {});
+ }
+};
Index: clang/test/Modules/Inputs/set-pure-crash/a.h
===
--- /dev/null
+++ clang/test/Modules/Inputs/set-pure-crash/a.h
@@ -0,0 +1,11 @@
+#pragma once
+
+struct Tag {};
+
+template
+class Base {
+public:
+ virtual void func() = 0;
+};
+
+Base bar();
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D70219: Make `-fmodule-file==` apply to .pcm file compilations
andrewjcg updated this revision to Diff 232190.
andrewjcg added a comment.
rebase onto monorepo and clang-format
Repository:
rG LLVM Github Monorepo
CHANGES SINCE LAST ACTION
https://reviews.llvm.org/D70219/new/
https://reviews.llvm.org/D70219
Files:
clang/include/clang/Frontend/ASTUnit.h
clang/lib/Frontend/ASTUnit.cpp
clang/lib/Frontend/FrontendAction.cpp
clang/test/Modules/Inputs/explicit-build-pcm/a.h
clang/test/Modules/Inputs/explicit-build-pcm/b.h
clang/test/Modules/Inputs/explicit-build-pcm/c.h
clang/test/Modules/Inputs/explicit-build-pcm/d.h
clang/test/Modules/Inputs/explicit-build-pcm/module.modulemap
clang/test/Modules/explicit-build-pcm.cpp
Index: clang/test/Modules/explicit-build-pcm.cpp
===
--- /dev/null
+++ clang/test/Modules/explicit-build-pcm.cpp
@@ -0,0 +1,26 @@
+// RUN: rm -rf %t
+// expected-no-diagnostics
+
+// RUN: %clang_cc1 -x c++ -std=c++11 -fmodules -fimplicit-module-maps -fmodules-cache-path=%t -Rmodule-build -fno-modules-error-recovery \
+// RUN:-fmodule-name=a -emit-module %S/Inputs/explicit-build/module.modulemap -o %t/a.pcm
+
+// RUN: %clang_cc1 -x c++ -std=c++11 -fmodules -fimplicit-module-maps -fmodules-cache-path=%t -Rmodule-build -fno-modules-error-recovery \
+// RUN:-fmodule-file=a=%t/a.pcm \
+// RUN:-fmodule-name=b -emit-module %S/Inputs/explicit-build/module.modulemap -o %t/b.pcm
+
+// RUN: %clang_cc1 -x c++ -std=c++11 -fmodules -fimplicit-module-maps -fmodules-cache-path=%t -Rmodule-build -fno-modules-error-recovery \
+// RUN:-fmodule-file=a=%t/a.pcm \
+// RUN:-fmodule-file=b=%t/b.pcm \
+// RUN:-fmodule-name=c -emit-module %S/Inputs/explicit-build/module.modulemap -o %t/c.pcm
+
+// RUN: mv %t/a.pcm %t/a.moved.pcm
+// RUN: mv %t/b.pcm %t/b.moved.pcm
+// RUN: mv %t/c.pcm %t/c.moved.pcm
+//
+// RUN: %clang_cc1 -x pcm -std=c++11 -fmodules -fimplicit-module-maps -fmodules-cache-path=%t -Rmodule-build -fno-modules-error-recovery \
+// RUN:-fmodule-file=a=%t/a.moved.pcm \
+// RUN:-fmodule-file=b=%t/b.moved.pcm \
+// RUN:%t/c.moved.pcm -o %t/c.o
+// expected-no-diagnostics
+
+#include
Index: clang/test/Modules/Inputs/explicit-build-pcm/module.modulemap
===
--- /dev/null
+++ clang/test/Modules/Inputs/explicit-build-pcm/module.modulemap
@@ -0,0 +1,4 @@
+module a { header "a.h" }
+module b { header "b.h" export * }
+module c { header "c.h" export * }
+module d { header "d.h" }
Index: clang/test/Modules/Inputs/explicit-build-pcm/c.h
===
--- /dev/null
+++ clang/test/Modules/Inputs/explicit-build-pcm/c.h
@@ -0,0 +1,7 @@
+#include "b.h"
+
+#if !__building_module(c)
+#error "should only get here when building module c"
+#endif
+
+const int c = 3;
Index: clang/test/Modules/Inputs/explicit-build-pcm/b.h
===
--- /dev/null
+++ clang/test/Modules/Inputs/explicit-build-pcm/b.h
@@ -0,0 +1,7 @@
+#include "a.h"
+
+#if !__building_module(b)
+#error "should only get here when building module b"
+#endif
+
+const int b = 2;
Index: clang/test/Modules/Inputs/explicit-build-pcm/a.h
===
--- /dev/null
+++ clang/test/Modules/Inputs/explicit-build-pcm/a.h
@@ -0,0 +1,5 @@
+#if !__building_module(a) && !BUILDING_A_PCH
+#error "should only get here when building module a"
+#endif
+
+const int a = 1;
Index: clang/lib/Frontend/FrontendAction.cpp
===
--- clang/lib/Frontend/FrontendAction.cpp
+++ clang/lib/Frontend/FrontendAction.cpp
@@ -565,7 +565,9 @@
std::unique_ptr AST = ASTUnit::LoadFromASTFile(
InputFile, CI.getPCHContainerReader(), ASTUnit::LoadPreprocessorOnly,
-ASTDiags, CI.getFileSystemOpts(), CI.getCodeGenOpts().DebugTypeExtRefs);
+ASTDiags, CI.getFileSystemOpts(), CI.getCodeGenOpts().DebugTypeExtRefs,
+false, None, CaptureDiagsKind::None, false, false,
+CI.getHeaderSearchOpts().PrebuiltModuleFiles);
if (!AST)
goto failure;
@@ -631,7 +633,9 @@
std::unique_ptr AST = ASTUnit::LoadFromASTFile(
InputFile, CI.getPCHContainerReader(), ASTUnit::LoadEverything, Diags,
-CI.getFileSystemOpts(), CI.getCodeGenOpts().DebugTypeExtRefs);
+CI.getFileSystemOpts(), CI.getCodeGenOpts().DebugTypeExtRefs, false,
+None, CaptureDiagsKind::None, false, false,
+CI.getHeaderSearchOpts().PrebuiltModuleFiles);
if (!AST)
goto failure;
Index: clang/lib/Frontend/ASTUnit.cpp
===
--- clang/lib/Frontend/ASTUnit.cpp
+++ clang/lib/Frontend/ASTUnit.cpp
@@ -759,7 +759,8 @@
const FileSystemOptions , bool UseDebugInfo,
bool
[PATCH] D70219: Make `-fmodule-file==` apply to .pcm file compilations
andrewjcg created this revision.
Herald added a project: clang.
Herald added a subscriber: cfe-commits.
When precompiling a header module, `-fmodule-file=name>=` flags
can be used to provide an updated path to a module, which allows modules
to be moved from the location they were compiled against.
This diff makes these flags also apply to when compiling a `.pcm` file
into a `.o`.
Repository:
rC Clang
https://reviews.llvm.org/D70219
Files:
include/clang/Frontend/ASTUnit.h
lib/Frontend/ASTUnit.cpp
lib/Frontend/FrontendAction.cpp
test/Modules/Inputs/explicit-build-pcm/a.h
test/Modules/Inputs/explicit-build-pcm/b.h
test/Modules/Inputs/explicit-build-pcm/c.h
test/Modules/Inputs/explicit-build-pcm/d.h
test/Modules/Inputs/explicit-build-pcm/module.modulemap
test/Modules/explicit-build-pcm.cpp
Index: test/Modules/explicit-build-pcm.cpp
===
--- /dev/null
+++ test/Modules/explicit-build-pcm.cpp
@@ -0,0 +1,26 @@
+// RUN: rm -rf %t
+// expected-no-diagnostics
+
+// RUN: %clang_cc1 -x c++ -std=c++11 -fmodules -fimplicit-module-maps -fmodules-cache-path=%t -Rmodule-build -fno-modules-error-recovery \
+// RUN:-fmodule-name=a -emit-module %S/Inputs/explicit-build/module.modulemap -o %t/a.pcm
+
+// RUN: %clang_cc1 -x c++ -std=c++11 -fmodules -fimplicit-module-maps -fmodules-cache-path=%t -Rmodule-build -fno-modules-error-recovery \
+// RUN:-fmodule-file=a=%t/a.pcm \
+// RUN:-fmodule-name=b -emit-module %S/Inputs/explicit-build/module.modulemap -o %t/b.pcm
+
+// RUN: %clang_cc1 -x c++ -std=c++11 -fmodules -fimplicit-module-maps -fmodules-cache-path=%t -Rmodule-build -fno-modules-error-recovery \
+// RUN:-fmodule-file=a=%t/a.pcm \
+// RUN:-fmodule-file=b=%t/b.pcm \
+// RUN:-fmodule-name=c -emit-module %S/Inputs/explicit-build/module.modulemap -o %t/c.pcm
+
+// RUN: mv %t/a.pcm %t/a.moved.pcm
+// RUN: mv %t/b.pcm %t/b.moved.pcm
+// RUN: mv %t/c.pcm %t/c.moved.pcm
+//
+// RUN: %clang_cc1 -x pcm -std=c++11 -fmodules -fimplicit-module-maps -fmodules-cache-path=%t -Rmodule-build -fno-modules-error-recovery \
+// RUN:-fmodule-file=a=%t/a.moved.pcm \
+// RUN:-fmodule-file=b=%t/b.moved.pcm \
+// RUN:%t/c.moved.pcm -o %t/c.o
+// expected-no-diagnostics
+
+#include
Index: test/Modules/Inputs/explicit-build-pcm/module.modulemap
===
--- /dev/null
+++ test/Modules/Inputs/explicit-build-pcm/module.modulemap
@@ -0,0 +1,4 @@
+module a { header "a.h" }
+module b { header "b.h" export * }
+module c { header "c.h" export * }
+module d { header "d.h" }
Index: test/Modules/Inputs/explicit-build-pcm/c.h
===
--- /dev/null
+++ test/Modules/Inputs/explicit-build-pcm/c.h
@@ -0,0 +1,7 @@
+#include "b.h"
+
+#if !__building_module(c)
+#error "should only get here when building module c"
+#endif
+
+const int c = 3;
Index: test/Modules/Inputs/explicit-build-pcm/b.h
===
--- /dev/null
+++ test/Modules/Inputs/explicit-build-pcm/b.h
@@ -0,0 +1,7 @@
+#include "a.h"
+
+#if !__building_module(b)
+#error "should only get here when building module b"
+#endif
+
+const int b = 2;
Index: test/Modules/Inputs/explicit-build-pcm/a.h
===
--- /dev/null
+++ test/Modules/Inputs/explicit-build-pcm/a.h
@@ -0,0 +1,5 @@
+#if !__building_module(a) && !BUILDING_A_PCH
+#error "should only get here when building module a"
+#endif
+
+const int a = 1;
Index: lib/Frontend/FrontendAction.cpp
===
--- lib/Frontend/FrontendAction.cpp
+++ lib/Frontend/FrontendAction.cpp
@@ -564,7 +564,8 @@
std::unique_ptr AST = ASTUnit::LoadFromASTFile(
InputFile, CI.getPCHContainerReader(), ASTUnit::LoadPreprocessorOnly,
-ASTDiags, CI.getFileSystemOpts(), CI.getCodeGenOpts().DebugTypeExtRefs);
+ASTDiags, CI.getFileSystemOpts(), CI.getCodeGenOpts().DebugTypeExtRefs,
+false, None, CaptureDiagsKind::None, false, false, CI.getHeaderSearchOpts().PrebuiltModuleFiles);
if (!AST)
goto failure;
@@ -630,7 +631,8 @@
std::unique_ptr AST = ASTUnit::LoadFromASTFile(
InputFile, CI.getPCHContainerReader(), ASTUnit::LoadEverything, Diags,
-CI.getFileSystemOpts(), CI.getCodeGenOpts().DebugTypeExtRefs);
+CI.getFileSystemOpts(), CI.getCodeGenOpts().DebugTypeExtRefs,
+false, None, CaptureDiagsKind::None, false, false, CI.getHeaderSearchOpts().PrebuiltModuleFiles);
if (!AST)
goto failure;
Index: lib/Frontend/ASTUnit.cpp
===
--- lib/Frontend/ASTUnit.cpp
+++ lib/Frontend/ASTUnit.cpp
@@ -759,7 +759,8 @@
const FileSystemOptions , bool
[PATCH] D58920: [Modules][PR39287] Consolidate multiple std's
andrewjcg added a comment. Sorry for the delay. Just catching up on the code this covers, so apologies if the questions don't make sense. Comment at: lib/Sema/SemaDeclCXX.cpp:9214-9215 getStdNamespace()->setImplicit(true); +if (getLangOpts().Modules) + getStdNamespace()->setHasExternalVisibleStorage(true); } I think you mentioned in another forum that one side effect of this change is that it's causing multiple candidates for names in `std`. Is this the implicit align constructros/destructors? Is this because we're marking the implicit namespace as being externally visible? Comment at: lib/Serialization/ASTReader.cpp:9291-9295 // Load pending declaration chains. for (unsigned I = 0; I != PendingDeclChains.size(); ++I) loadPendingDeclChain(PendingDeclChains[I].first, PendingDeclChains[I].second); PendingDeclChains.clear(); How does modules normally "connect up" definitions of the same namspace occurring in the imported module? Is it done here (e.g. so that a name lookup will search all namespace definitions)? Is an alternate solution to this diff to make sure this handles the `std` implicit special case? Apologies for the naivety here -- still getting up to speed on this code. Repository: rC Clang CHANGES SINCE LAST ACTION https://reviews.llvm.org/D58920/new/ https://reviews.llvm.org/D58920 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D52956: Support `-fno-visibility-inlines-hidden`
andrewjcg added a comment. Herald added a project: clang. Sorry for the delay here, but this should be ready to go. As this is my first accepted diff to LLVM, should I follow https://llvm.org/docs/DeveloperPolicy.html#obtaining-commit-access to get commit access, or is there some other process for first-time committers? Repository: rC Clang CHANGES SINCE LAST ACTION https://reviews.llvm.org/D52956/new/ https://reviews.llvm.org/D52956 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D51568: [modules] Add `-fno-absolute-module-directory` flag for relocatable modules
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 ,
+ bool ) {
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
[PATCH] D52956: Support `-fno-visibility-inlines-hidden`
andrewjcg created this revision.
Herald added subscribers: cfe-commits, eraman.
Undoes `-fvisibility-inlines-hidden`.
Test Plan: added test
Repository:
rC Clang
https://reviews.llvm.org/D52956
Files:
include/clang/Driver/Options.td
lib/Driver/ToolChains/Clang.cpp
lib/Frontend/CompilerInvocation.cpp
test/Driver/visibility-inlines-hidden.cpp
Index: test/Driver/visibility-inlines-hidden.cpp
===
--- /dev/null
+++ test/Driver/visibility-inlines-hidden.cpp
@@ -0,0 +1,19 @@
+// RUN: %clang -### -S -fvisibility-inlines-hidden %s 2> %t.log
+// RUN: FileCheck -check-prefix=CHECK-1 %s < %t.log
+// CHECK-1: "-fvisibility-inlines-hidden"
+// CHECK-1-NOT: "-fno-visibility-inlines-hidden"
+
+// RUN: %clang -### -S -fno-visibility-inlines-hidden %s 2> %t.log
+// RUN: FileCheck -check-prefix=CHECK-2 %s < %t.log
+// CHECK-2: "-fno-visibility-inlines-hidden"
+// CHECK-2-NOT: "-fvisibility-inlines-hidden"
+
+// RUN: %clang -### -S -fvisibility-inlines-hidden
-fno-visibility-inlines-hidden %s 2> %t.log
+// RUN: FileCheck -check-prefix=CHECK-3 %s < %t.log
+// CHECK-3: "-fno-visibility-inlines-hidden"
+// CHECK-3-NOT: "-fvisibility-inlines-hidden"
+
+// RUN: %clang -### -S -fno-visibility-inlines-hidden
-fvisibility-inlines-hidden %s 2> %t.log
+// RUN: FileCheck -check-prefix=CHECK-4 %s < %t.log
+// CHECK-4: "-fvisibility-inlines-hidden"
+// CHECK-4-NOT: "-fno-visibility-inlines-hidden"
Index: lib/Frontend/CompilerInvocation.cpp
===
--- lib/Frontend/CompilerInvocation.cpp
+++ lib/Frontend/CompilerInvocation.cpp
@@ -2310,8 +2310,11 @@
Opts.setTypeVisibilityMode(Opts.getValueVisibilityMode());
}
- if (Args.hasArg(OPT_fvisibility_inlines_hidden))
+ if (Args.hasFlag(OPT_fvisibility_inlines_hidden,
+ OPT_fno_visibility_inlines_hidden,
+ false)) {
Opts.InlineVisibilityHidden = 1;
+ }
if (Args.hasArg(OPT_ftrapv)) {
Opts.setSignedOverflowBehavior(LangOptions::SOB_Trapping);
Index: lib/Driver/ToolChains/Clang.cpp
===
--- lib/Driver/ToolChains/Clang.cpp
+++ lib/Driver/ToolChains/Clang.cpp
@@ -4167,7 +4167,8 @@
}
}
- Args.AddLastArg(CmdArgs, options::OPT_fvisibility_inlines_hidden);
+ Args.AddLastArg(CmdArgs, options::OPT_fvisibility_inlines_hidden,
+ options::OPT_fno_visibility_inlines_hidden);
Args.AddLastArg(CmdArgs, options::OPT_ftlsmodel_EQ);
Index: include/clang/Driver/Options.td
===
--- include/clang/Driver/Options.td
+++ include/clang/Driver/Options.td
@@ -1714,6 +1714,9 @@
def fvisibility_inlines_hidden : Flag<["-"], "fvisibility-inlines-hidden">,
Group,
HelpText<"Give inline C++ member functions hidden visibility by default">,
Flags<[CC1Option]>;
+def fno_visibility_inlines_hidden : Flag<["-"],
"fno-visibility-inlines-hidden">, Group,
+ HelpText<"Do not give inline C++ member functions hidden visibility by
default">,
+ Flags<[CC1Option]>;
def fvisibility_ms_compat : Flag<["-"], "fvisibility-ms-compat">,
Group,
HelpText<"Give global types 'default' visibility and global functions and "
"variables 'hidden' visibility by default">;
Index: test/Driver/visibility-inlines-hidden.cpp
===
--- /dev/null
+++ test/Driver/visibility-inlines-hidden.cpp
@@ -0,0 +1,19 @@
+// RUN: %clang -### -S -fvisibility-inlines-hidden %s 2> %t.log
+// RUN: FileCheck -check-prefix=CHECK-1 %s < %t.log
+// CHECK-1: "-fvisibility-inlines-hidden"
+// CHECK-1-NOT: "-fno-visibility-inlines-hidden"
+
+// RUN: %clang -### -S -fno-visibility-inlines-hidden %s 2> %t.log
+// RUN: FileCheck -check-prefix=CHECK-2 %s < %t.log
+// CHECK-2: "-fno-visibility-inlines-hidden"
+// CHECK-2-NOT: "-fvisibility-inlines-hidden"
+
+// RUN: %clang -### -S -fvisibility-inlines-hidden -fno-visibility-inlines-hidden %s 2> %t.log
+// RUN: FileCheck -check-prefix=CHECK-3 %s < %t.log
+// CHECK-3: "-fno-visibility-inlines-hidden"
+// CHECK-3-NOT: "-fvisibility-inlines-hidden"
+
+// RUN: %clang -### -S -fno-visibility-inlines-hidden -fvisibility-inlines-hidden %s 2> %t.log
+// RUN: FileCheck -check-prefix=CHECK-4 %s < %t.log
+// CHECK-4: "-fvisibility-inlines-hidden"
+// CHECK-4-NOT: "-fno-visibility-inlines-hidden"
Index: lib/Frontend/CompilerInvocation.cpp
===
--- lib/Frontend/CompilerInvocation.cpp
+++ lib/Frontend/CompilerInvocation.cpp
@@ -2310,8 +2310,11 @@
Opts.setTypeVisibilityMode(Opts.getValueVisibilityMode());
}
- if (Args.hasArg(OPT_fvisibility_inlines_hidden))
+ if (Args.hasFlag(OPT_fvisibility_inlines_hidden,
+ OPT_fno_visibility_inlines_hidden,
+ false)) {
Opts.InlineVisibilityHidden = 1;
+ }
[PATCH] D51568: [modules] Add `-fno-absolute-module-directory` flag for relocatable modules
andrewjcg updated this revision to Diff 166709.
andrewjcg added a comment.
Dropping the module directory entirely and fully resolving paths on
serialization
broke some things during deserialization, specifically when the deserializer
wanted
to update paths to use an alternate module directory.
This switches to a different strategy of only relativizing the paths that are
actually under the module home dir, and adding a bit to the serialized paths to
indiciate this. This bit is read on deserializiation to determine whether the
path is resolved against the module directory or not.
Repository:
rC Clang
https://reviews.llvm.org/D51568
Files:
include/clang/Driver/CC1Options.td
include/clang/Lex/HeaderSearchOptions.h
include/clang/Serialization/ASTReader.h
include/clang/Serialization/ASTWriter.h
lib/Frontend/CompilerInvocation.cpp
lib/Serialization/ASTReader.cpp
lib/Serialization/ASTReaderInternals.h
lib/Serialization/ASTWriter.cpp
lib/Serialization/GlobalModuleIndex.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 `-fno-absolute-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 -fno-absolute-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 -fno-absolute-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/GlobalModuleIndex.cpp
===
--- lib/Serialization/GlobalModuleIndex.cpp
+++ lib/Serialization/GlobalModuleIndex.cpp
@@ -628,6 +628,7 @@
SmallString<128> ImportedFile(Record.begin() + Idx,
Record.begin() + Idx + Length);
Idx += Length;
+Idx++; // Relative
// Find the imported module file.
const FileEntry *DependsOnFile
Index: lib/Serialization/ASTWriter.cpp
===
--- lib/Serialization/ASTWriter.cpp
+++ lib/Serialization/ASTWriter.cpp
@@ -1327,9 +1327,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
@@ -1493,7 +1498,10 @@
if (WritingModule && WritingModule->Directory) {
SmallString<128> BaseDir(WritingModule->Directory->getName());
-cleanPathForOutput(Context.getSourceManager().getFileManager(), BaseDir);
+cleanPathForOutput(Context.getSourceManager().getFileManager(), BaseDir,
+ !PP.getHeaderSearchInfo()
+.getHeaderSearchOpts()
+.NoAbsoluteModuleDirectory);
// If the home of the module is the current working directory, then we
// want to pick up the cwd of the build process loading the module, not
@@ -1708,6 +1716,7 @@
auto FileAbbrev = std::make_shared();
FileAbbrev->Add(BitCodeAbbrevOp(ORIGINAL_FILE));
FileAbbrev->Add(BitCodeAbbrevOp(BitCodeAbbrevOp::VBR, 6)); // File ID
+FileAbbrev->Add(BitCodeAbbrevOp(BitCodeAbbrevOp::Fixed, 1)); // Relative
FileAbbrev->Add(BitCodeAbbrevOp(BitCodeAbbrevOp::Blob)); // File name
unsigned FileAbbrevCode = Stream.EmitAbbrev(std::move(FileAbbrev));
@@ -1772,6 +1781,7 @@
IFAbbrev->Add(BitCodeAbbrevOp(BitCodeAbbrevOp::Fixed, 1)); // Overridden
IFAbbrev->Add(BitCodeAbbrevOp(BitCodeAbbrevOp::Fixed, 1)); // Transient
IFAbbrev->Add(BitCodeAbbrevOp(BitCodeAbbrevOp::Fixed, 1)); // Module map
+ IFAbbrev->Add(BitCodeAbbrevOp(BitCodeAbbrevOp::Fixed, 1)); // Relative
IFAbbrev->Add(BitCodeAbbrevOp(BitCodeAbbrevOp::Blob)); // File name
unsigned IFAbbrevCode =
[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(),
+
