This revision was landed with ongoing or failed builds.
This revision was automatically updated to reflect the committed changes.
Closed by commit rGe06a91c5996b: [clang][modules] Avoid re-exporting PCH
imports on every later module import (authored by benlangmuir).
Repository:
rG LLVM Github Monorepo
CHANGES SINCE LAST ACTION
https://reviews.llvm.org/D148176/new/
https://reviews.llvm.org/D148176
Files:
clang/include/clang/Serialization/ASTReader.h
clang/lib/Serialization/ASTReader.cpp
clang/test/ClangScanDeps/modules-pch-imports.c
clang/test/Modules/submodule-visibility-pch.c
Index: clang/test/Modules/submodule-visibility-pch.c
===================================================================
--- /dev/null
+++ clang/test/Modules/submodule-visibility-pch.c
@@ -0,0 +1,56 @@
+// Verify that the use of a PCH does not accidentally make modules from the PCH
+// visible to submodules when using -fmodules-local-submodule-visibility
+// and -fmodule-name to trigger a textual include.
+
+// RUN: rm -rf %t
+// RUN: split-file %s %t
+
+// First check that it works with a header
+
+// RUN: %clang_cc1 -fmodules -fmodules-cache-path=%t/cache \
+// RUN: -fmodules-local-submodule-visibility -fimplicit-module-maps \
+// RUN: -fmodule-name=Mod \
+// RUN: %t/tu.c -include %t/prefix.h -I %t -verify
+
+// Now with a PCH
+
+// RUN: %clang_cc1 -fmodules -fmodules-cache-path=%t/cache \
+// RUN: -fmodules-local-submodule-visibility -fimplicit-module-maps \
+// RUN: -x c-header %t/prefix.h -emit-pch -o %t/prefix.pch -I %t
+
+// RUN: %clang_cc1 -fmodules -fmodules-cache-path=%t/cache \
+// RUN: -fmodules-local-submodule-visibility -fimplicit-module-maps \
+// RUN: -fmodule-name=Mod \
+// RUN: %t/tu.c -include-pch %t/prefix.pch -I %t -verify
+
+//--- module.modulemap
+module ModViaPCH { header "ModViaPCH.h" }
+module ModViaInclude { header "ModViaInclude.h" }
+module Mod { header "Mod.h" }
+module SomeOtherMod { header "SomeOtherMod.h" }
+
+//--- ModViaPCH.h
+#define ModViaPCH 1
+
+//--- ModViaInclude.h
+#define ModViaInclude 2
+
+//--- SomeOtherMod.h
+// empty
+
+//--- Mod.h
+#include "SomeOtherMod.h"
+#ifdef ModViaPCH
+#error "Visibility violation ModViaPCH"
+#endif
+#ifdef ModViaInclude
+#error "Visibility violation ModViaInclude"
+#endif
+
+//--- prefix.h
+#include "ModViaPCH.h"
+
+//--- tu.c
+#include "ModViaInclude.h"
+#include "Mod.h"
+// expected-no-diagnostics
Index: clang/test/ClangScanDeps/modules-pch-imports.c
===================================================================
--- /dev/null
+++ clang/test/ClangScanDeps/modules-pch-imports.c
@@ -0,0 +1,108 @@
+// Check that a module from -fmodule-name= does not accidentally pick up extra
+// dependencies that come from a PCH.
+
+// RUN: rm -rf %t
+// RUN: split-file %s %t
+// RUN: sed "s|DIR|%/t|g" %t/cdb.json.template > %t/cdb.json
+// RUN: sed "s|DIR|%/t|g" %t/cdb_pch.json.template > %t/cdb_pch.json
+
+// Scan PCH
+// RUN: clang-scan-deps -compilation-database %t/cdb_pch.json \
+// RUN: -format experimental-full -mode preprocess-dependency-directives \
+// RUN: > %t/deps_pch.json
+
+// Build PCH
+// RUN: %deps-to-rsp %t/deps_pch.json --module-name A > %t/A.rsp
+// RUN: %deps-to-rsp %t/deps_pch.json --module-name B > %t/B.rsp
+// RUN: %deps-to-rsp %t/deps_pch.json --tu-index 0 > %t/pch.rsp
+// RUN: %clang @%t/A.rsp
+// RUN: %clang @%t/B.rsp
+// RUN: %clang @%t/pch.rsp
+
+// Scan TU with PCH
+// RUN: clang-scan-deps -compilation-database %t/cdb.json \
+// RUN: -format experimental-full -mode preprocess-dependency-directives \
+// RUN: > %t/deps.json
+
+// RUN: cat %t/deps.json | sed 's:\\\\\?:/:g' | FileCheck %s -DPREFIX=%/t
+
+// Verify that the only modular import in C is E and not the unrelated modules
+// A or B that come from the PCH.
+
+// CHECK: {
+// CHECK-NEXT: "modules": [
+// CHECK-NEXT: {
+// CHECK: "clang-module-deps": [
+// CHECK-NEXT: {
+// CHECK: "module-name": "E"
+// CHECK: }
+// CHECK-NEXT: ]
+// CHECK: "clang-modulemap-file": "[[PREFIX]]/module.modulemap"
+// CHECK: "command-line": [
+// CHECK-NOT: "-fmodule-file=
+// CHECK: "-fmodule-file={{(E=)?}}[[PREFIX]]/{{.*}}/E-{{.*}}.pcm"
+// CHECK-NOT: "-fmodule-file=
+// CHECK: ]
+// CHECK: "name": "C"
+// CHECK: }
+
+
+//--- cdb_pch.json.template
+[{
+ "file": "DIR/prefix.h",
+ "directory": "DIR",
+ "command": "clang -x c-header DIR/prefix.h -o DIR/prefix.h.pch -fmodules -fimplicit-modules -fimplicit-module-maps -fmodules-cache-path=DIR/module-cache"
+}]
+
+//--- cdb.json.template
+[{
+ "file": "DIR/tu.c",
+ "directory": "DIR",
+ "command": "clang -fsyntax-only DIR/tu.c -include DIR/prefix.h -fmodule-name=C -fmodules -fimplicit-modules -fimplicit-module-maps -fmodules-cache-path=DIR/module-cache"
+}]
+
+//--- module.modulemap
+module A { header "A.h" export * }
+module B { header "B.h" export * }
+module C { header "C.h" export * }
+module D { header "D.h" export * }
+module E { header "E.h" export * }
+
+//--- A.h
+#pragma once
+struct A { int x; };
+
+//--- B.h
+#pragma once
+#include "A.h"
+struct B { struct A a; };
+
+//--- C.h
+#pragma once
+#include "E.h"
+struct C { struct E e; };
+
+//--- D.h
+#pragma once
+#include "C.h"
+struct D { struct C c; };
+
+//--- E.h
+#pragma once
+struct E { int y; };
+
+//--- prefix.h
+#include "B.h"
+
+//--- tu.c
+// C.h is first included textually due to -fmodule-name=C.
+#include "C.h"
+// importing D pulls in a modular import of C; it's this build of C that we
+// are verifying above
+#include "D.h"
+
+void tu(void) {
+ struct A a;
+ struct B b;
+ struct C c;
+}
Index: clang/lib/Serialization/ASTReader.cpp
===================================================================
--- clang/lib/Serialization/ASTReader.cpp
+++ clang/lib/Serialization/ASTReader.cpp
@@ -3728,7 +3728,7 @@
unsigned GlobalID = getGlobalSubmoduleID(F, Record[I++]);
SourceLocation Loc = ReadSourceLocation(F, Record, I);
if (GlobalID) {
- ImportedModules.push_back(ImportedSubmodule(GlobalID, Loc));
+ PendingImportedModules.push_back(ImportedSubmodule(GlobalID, Loc));
if (DeserializationListener)
DeserializationListener->ModuleImportRead(GlobalID, Loc);
}
@@ -4445,8 +4445,8 @@
UnresolvedModuleRefs.clear();
if (Imported)
- Imported->append(ImportedModules.begin(),
- ImportedModules.end());
+ Imported->append(PendingImportedModules.begin(),
+ PendingImportedModules.end());
// FIXME: How do we load the 'use'd modules? They may not be submodules.
// Might be unnecessary as use declarations are only used to build the
@@ -5050,7 +5050,7 @@
// Re-export any modules that were imported by a non-module AST file.
// FIXME: This does not make macro-only imports visible again.
- for (auto &Import : ImportedModules) {
+ for (auto &Import : PendingImportedModules) {
if (Module *Imported = getSubmodule(Import.ID)) {
makeModuleVisible(Imported, Module::AllVisible,
/*ImportLoc=*/Import.ImportLoc);
@@ -5060,6 +5060,10 @@
// nullptr here, we do the same later, in UpdateSema().
}
}
+
+ // Hand off these modules to Sema.
+ PendingImportedModulesSema.append(PendingImportedModules);
+ PendingImportedModules.clear();
}
void ASTReader::finalizeForWriting() {
@@ -8105,13 +8109,14 @@
}
// For non-modular AST files, restore visiblity of modules.
- for (auto &Import : ImportedModules) {
+ for (auto &Import : PendingImportedModulesSema) {
if (Import.ImportLoc.isInvalid())
continue;
if (Module *Imported = getSubmodule(Import.ID)) {
SemaObj->makeModuleVisible(Imported, Import.ImportLoc);
}
}
+ PendingImportedModulesSema.clear();
}
IdentifierInfo *ASTReader::get(StringRef Name) {
Index: clang/include/clang/Serialization/ASTReader.h
===================================================================
--- clang/include/clang/Serialization/ASTReader.h
+++ clang/include/clang/Serialization/ASTReader.h
@@ -948,8 +948,14 @@
private:
/// A list of modules that were imported by precompiled headers or
- /// any other non-module AST file.
- SmallVector<ImportedSubmodule, 2> ImportedModules;
+ /// any other non-module AST file and have not yet been made visible. If a
+ /// module is made visible in the ASTReader, it will be transfered to
+ /// \c PendingImportedModulesSema.
+ SmallVector<ImportedSubmodule, 2> PendingImportedModules;
+
+ /// A list of modules that were imported by precompiled headers or
+ /// any other non-module AST file and have not yet been made visible for Sema.
+ SmallVector<ImportedSubmodule, 2> PendingImportedModulesSema;
//@}
/// The system include root to be used when loading the
_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
