bnbarham updated this revision to Diff 366154.
bnbarham added a comment.
Forgot to add the braces back in for the case statements
Repository:
rG LLVM Github Monorepo
CHANGES SINCE LAST ACTION
https://reviews.llvm.org/D107690/new/
https://reviews.llvm.org/D107690
Files:
clang/lib/Serialization/ASTReader.cpp
clang/test/VFS/Inputs/UsesFoo.framework/Headers/UsesFoo.h
clang/test/VFS/Inputs/UsesFoo.framework/Modules/module.modulemap
clang/test/VFS/module-header-mismatches.m
clang/test/VFS/umbrella-mismatch.m
Index: clang/test/VFS/umbrella-mismatch.m
===================================================================
--- clang/test/VFS/umbrella-mismatch.m
+++ /dev/null
@@ -1,7 +0,0 @@
-// RUN: rm -rf %t
-// RUN: sed -e "s;INPUT_DIR;%/S/Inputs;g" -e "s;OUT_DIR;%/S/Inputs;g" %S/Inputs/vfsoverlay.yaml > %t.yaml
-
-// RUN: %clang_cc1 -Werror -fmodules -fimplicit-module-maps -fmodules-cache-path=%t -ivfsoverlay %t.yaml -F %S/Inputs -fsyntax-only %s -Wno-atimport-in-framework-header -verify
-// RUN: %clang_cc1 -Werror -fmodules -fimplicit-module-maps -fmodules-cache-path=%t -F %S/Inputs -fsyntax-only %s -Wno-atimport-in-framework-header -verify
-// expected-no-diagnostics
-@import UsesFoo;
Index: clang/test/VFS/module-header-mismatches.m
===================================================================
--- /dev/null
+++ clang/test/VFS/module-header-mismatches.m
@@ -0,0 +1,86 @@
+// RUN: rm -rf %t
+// RUN: split-file %s %t
+// RUN: sed -e "s;TEST_DIR;%/t;g" %t/sed-overlay.yaml > %t/overlay.yaml
+
+// These tests first build with an overlay such that the header is resolved
+// to %t/other/Mismatch.h. They then build again with the header resolved
+// to the one in their directory.
+//
+// This should cause a rebuild if the contents is different (and thus multiple
+// PCMs), but this currently isn't the case. We should at least not error,
+// since this does happen in real projects (with a different copy of the same
+// file).
+
+// RUN: %clang_cc1 -Werror -fmodules -fimplicit-module-maps -fmodules-cache-path=%t/hf-mcp -ivfsoverlay %t/overlay.yaml -F %t/header-frameworks -fsyntax-only -verify %t/use.m
+// RUN: %clang_cc1 -Werror -fmodules -fimplicit-module-maps -fmodules-cache-path=%t/hf-mcp -F %t/header-frameworks -fsyntax-only -verify %t/use.m
+// RUN: find %t/hf-mcp -name "Mismatch-*.pcm" | count 1
+
+// RUN: %clang_cc1 -Werror -fmodules -fimplicit-module-maps -fmodules-cache-path=%t/df-mcp -ivfsoverlay %t/overlay.yaml -F %t/dir-frameworks -fsyntax-only -verify %t/use.m
+// RUN: %clang_cc1 -Werror -fmodules -fimplicit-module-maps -fmodules-cache-path=%t/hf-mcp -F %t/dir-frameworks -fsyntax-only -verify %t/use.m
+// RUN: find %t/df-mcp -name "Mismatch-*.pcm" | count 1
+
+// RUN: %clang_cc1 -Werror -fmodules -fimplicit-module-maps -fmodules-cache-path=%t/nf-mcp -ivfsoverlay %t/overlay.yaml -F %t/norm-frameworks -fsyntax-only -verify %t/use.m
+// RUN: %clang_cc1 -Werror -fmodules -fimplicit-module-maps -fmodules-cache-path=%t/nf-mcp -F %t/norm-frameworks -fsyntax-only -verify %t/use.m
+// RUN: find %t/nf-mcp -name "Mismatch-*.pcm" | count 1
+
+// RUN: %clang_cc1 -Werror -fmodules -fimplicit-module-maps -fmodules-cache-path=%t/m-mcp -ivfsoverlay %t/overlay.yaml -I %t/mod -fsyntax-only -verify %t/use.m
+// RUN: %clang_cc1 -Werror -fmodules -fimplicit-module-maps -fmodules-cache-path=%t/m-mcp -I %t/mod -fsyntax-only -verify %t/use.m
+// RUN: find %t/m-mcp -name "Mismatch-*.pcm" | count 1
+
+//--- use.m
+// expected-no-diagnostics
+@import Mismatch;
+
+//--- header-frameworks/Mismatch.framework/Modules/module.modulemap
+framework module Mismatch {
+ umbrella header "Mismatch.h"
+}
+//--- header-frameworks/Mismatch.framework/Headers/Mismatch.h
+
+//--- dir-frameworks/Mismatch.framework/Modules/module.modulemap
+framework module Mismatch {
+ umbrella "someheaders"
+}
+//--- dir-frameworks/Mismatch.framework/someheaders/Mismatch.h
+
+//--- norm-frameworks/Mismatch.framework/Modules/module.modulemap
+framework module Mismatch {
+ header "Mismatch.h"
+}
+//--- norm-frameworks/Mismatch.framework/Headers/Mismatch.h
+
+//--- mod/module.modulemap
+module Mismatch {
+ umbrella header "Mismatch.h"
+}
+//--- mod/Mismatch.h
+
+//--- other/Mismatch.h
+
+//--- sed-overlay.yaml
+{
+ 'version': 0,
+ 'roots': [
+ { 'name': 'TEST_DIR', 'type': 'directory',
+ 'contents': [
+ { 'name': 'header-frameworks/Mismatch.framework/Headers/Mismatch.h',
+ 'type': 'file',
+ 'external-contents': 'TEST_DIR/other/Mismatch.h'
+ },
+ { 'name': 'dir-frameworks/Mismatch.framework/someheaders',
+ 'type': 'directory',
+ 'external-contents': 'TEST_DIR/others'
+ },
+ { 'name': 'norm-frameworks/Mismatch.framework/Headers/Mismatch.h',
+ 'type': 'file',
+ 'external-contents': 'TEST_DIR/other/Mismatch.h'
+ },
+ { 'name': 'mod/Mismatch.h',
+ 'type': 'file',
+ 'external-contents': 'TEST_DIR/other/Mismatch.h'
+ }
+ ]
+ }
+ ]
+}
+
Index: clang/test/VFS/Inputs/UsesFoo.framework/Modules/module.modulemap
===================================================================
--- clang/test/VFS/Inputs/UsesFoo.framework/Modules/module.modulemap
+++ /dev/null
@@ -1,4 +0,0 @@
-framework module UsesFoo {
- umbrella header "UsesFoo.h"
- export *
-}
Index: clang/test/VFS/Inputs/UsesFoo.framework/Headers/UsesFoo.h
===================================================================
--- clang/test/VFS/Inputs/UsesFoo.framework/Headers/UsesFoo.h
+++ /dev/null
@@ -1 +0,0 @@
-@import Foo;
Index: clang/lib/Serialization/ASTReader.cpp
===================================================================
--- clang/lib/Serialization/ASTReader.cpp
+++ clang/lib/Serialization/ASTReader.cpp
@@ -4240,8 +4240,11 @@
PreviousGeneration = incrementGeneration(*ContextObj);
unsigned NumModules = ModuleMgr.size();
- auto removeModulesAndReturn = [&](ASTReadResult ReadResult) {
- assert(ReadResult && "expected to return error");
+ SmallVector<ImportedModule, 4> Loaded;
+ if (ASTReadResult ReadResult =
+ ReadASTCore(FileName, Type, ImportLoc,
+ /*ImportedBy=*/nullptr, Loaded, 0, 0, ASTFileSignature(),
+ ClientLoadCapabilities)) {
ModuleMgr.removeModules(ModuleMgr.begin() + NumModules,
PP.getLangOpts().Modules
? &PP.getHeaderSearchInfo().getModuleMap()
@@ -4252,22 +4255,6 @@
GlobalIndex.reset();
ModuleMgr.setGlobalIndex(nullptr);
return ReadResult;
- };
-
- SmallVector<ImportedModule, 4> Loaded;
- switch (ASTReadResult ReadResult =
- ReadASTCore(FileName, Type, ImportLoc,
- /*ImportedBy=*/nullptr, Loaded, 0, 0,
- ASTFileSignature(), ClientLoadCapabilities)) {
- case Failure:
- case Missing:
- case OutOfDate:
- case VersionMismatch:
- case ConfigurationMismatch:
- case HadErrors:
- return removeModulesAndReturn(ReadResult);
- case Success:
- break;
}
// Here comes stuff that we only do once the entire chain is loaded.
@@ -4279,18 +4266,18 @@
// Read the AST block.
if (ASTReadResult Result = ReadASTBlock(F, ClientLoadCapabilities))
- return removeModulesAndReturn(Result);
+ return Failure;
// The AST block should always have a definition for the main module.
if (F.isModule() && !F.DidReadTopLevelSubmodule) {
Error(diag::err_module_file_missing_top_level_submodule, F.FileName);
- return removeModulesAndReturn(Failure);
+ return Failure;
}
// Read the extension blocks.
while (!SkipCursorToBlock(F.Stream, EXTENSION_BLOCK_ID)) {
if (ASTReadResult Result = ReadExtensionBlock(F))
- return removeModulesAndReturn(Result);
+ return Failure;
}
// Once read, set the ModuleFile bit base offset and update the size in
@@ -5605,17 +5592,20 @@
}
case SUBMODULE_UMBRELLA_HEADER: {
+ // FIXME: This doesn't work for framework modules as `Filename` is the
+ // name as written in the module file and does not include
+ // `Headers/`, so this path will never exist.
std::string Filename = std::string(Blob);
ResolveImportedPath(F, Filename);
if (auto Umbrella = PP.getFileManager().getFile(Filename)) {
if (!CurrentModule->getUmbrellaHeader())
// FIXME: NameAsWritten
ModMap.setUmbrellaHeader(CurrentModule, *Umbrella, Blob, "");
- else if (CurrentModule->getUmbrellaHeader().Entry != *Umbrella) {
- if ((ClientLoadCapabilities & ARR_OutOfDate) == 0)
- Error("mismatched umbrella headers in submodule");
- return OutOfDate;
}
+ // Note that it's too late at this point to return out of date if the
+ // name from the PCM doesn't match up with the one in the module map,
+ // but also quite unlikely since we will have already checked the
+ // modification time and size of the module map file itself.
}
break;
}
@@ -5639,16 +5629,13 @@
break;
case SUBMODULE_UMBRELLA_DIR: {
+ // See comments in SUBMODULE_UMBRELLA_HEADER
std::string Dirname = std::string(Blob);
ResolveImportedPath(F, Dirname);
if (auto Umbrella = PP.getFileManager().getDirectory(Dirname)) {
if (!CurrentModule->getUmbrellaDir())
// FIXME: NameAsWritten
ModMap.setUmbrellaDir(CurrentModule, *Umbrella, Blob, "");
- else if (CurrentModule->getUmbrellaDir().Entry != *Umbrella) {
- if ((ClientLoadCapabilities & ARR_OutOfDate) == 0)
- Error("mismatched umbrella directories in submodule");
- return OutOfDate;
}
}
break;
_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
