[PATCH] D27852: [modules] Handle modules with nonstandard names in module.private.modulemaps
graydon created this revision. graydon added reviewers: manmanren, doug.gregor. graydon added a subscriber: cfe-commits. The module system supports accompanying a primary module (say Foo) with an auxiliary "private" module (defined in an adjacent module.private.modulemap file) that augments the primary module when associated private headers are available. The feature is intended to be used to augment the primary module with a submodule (say Foo.Private), however some users in the wild are choosing to augment the primary module with an additional top-level module with a "similar" name (in all cases so far: FooPrivate). This "works" when a user of the module initially imports a private header, such as '#import "Foo/something_private.h"' since the Foo import winds up importing FooPrivate in passing. But if the import is subsequently recorded in a PCH file, reloading the PCH will fail to validate because of a cross-check that attempts to find the module.modulemap (or module.private.modulemap) using HeaderSearch algorithm, applied to the "FooPrivate" name. Since it's stored in Foo.framework/Modules, not FooPrivate.framework/Modules, the check fails and the PCH is rejected. This patch adds a compensatory workaround in the HeaderSearch algorithm when searching (and failing to find) a module of the form FooPrivate: the name used to derive filesystem paths is decoupled from the module name being searched for, and if the initial search fails and the module is named "FooPrivate", the filesystem search name is altered to remove the "Private" suffix, and the algorithm is run a second time (still looking for a module named FooPrivate, but looking in directories derived from Foo). Accompanying this change is a new warning that triggers when a user loads a module.private.modulemap that defines a top-level module with a different name from the top-level module defined in its adjacent module.modulemap. https://reviews.llvm.org/D27852 Files: include/clang/Basic/DiagnosticLexKinds.td include/clang/Lex/HeaderSearch.h lib/Lex/HeaderSearch.cpp lib/Lex/ModuleMap.cpp test/Modules/Inputs/implicit-private-with-different-name/A.framework/Headers/a.h test/Modules/Inputs/implicit-private-with-different-name/A.framework/Headers/aprivate.h test/Modules/Inputs/implicit-private-with-different-name/A.framework/Modules/module.modulemap test/Modules/Inputs/implicit-private-with-different-name/A.framework/Modules/module.private.modulemap test/Modules/implicit-private-with-different-name.m Index: test/Modules/implicit-private-with-different-name.m === --- /dev/null +++ test/Modules/implicit-private-with-different-name.m @@ -0,0 +1,15 @@ +// RUN: rm -rf %t + +// Build PCH using A, with adjacent private module APrivate, which winds up being implicitly referenced +// RUN: %clang_cc1 -verify -fmodules -fimplicit-module-maps -fmodules-cache-path=%t -F %S/Inputs/implicit-private-with-different-name -emit-pch -o %t-A.pch %s + +// Use the PCH with no explicit way to resolve PrivateA, still pick it up through MODULE_DIRECTORY reference in PCH control block +// RUN: %clang_cc1 -verify -fmodules -fimplicit-module-maps -fmodules-cache-path=%t -F %S/Inputs/implicit-private-with-different-name -include-pch %t-A.pch %s -fsyntax-only + +// expected-warning@Inputs/implicit-private-with-different-name/A.framework/Modules/module.private.modulemap:1{{expected a submodule}} + +#ifndef HEADER +#define HEADER +#import "A/aprivate.h" +const int *y = &APRIVATE; +#endif Index: test/Modules/Inputs/implicit-private-with-different-name/A.framework/Modules/module.private.modulemap === --- /dev/null +++ test/Modules/Inputs/implicit-private-with-different-name/A.framework/Modules/module.private.modulemap @@ -0,0 +1,4 @@ +framework module APrivate { + header "aprivate.h" + export * +} Index: test/Modules/Inputs/implicit-private-with-different-name/A.framework/Modules/module.modulemap === --- /dev/null +++ test/Modules/Inputs/implicit-private-with-different-name/A.framework/Modules/module.modulemap @@ -0,0 +1,4 @@ +framework module A { + header "a.h" + export * +} Index: test/Modules/Inputs/implicit-private-with-different-name/A.framework/Headers/aprivate.h === --- /dev/null +++ test/Modules/Inputs/implicit-private-with-different-name/A.framework/Headers/aprivate.h @@ -0,0 +1 @@ +extern int APRIVATE; Index: test/Modules/Inputs/implicit-private-with-different-name/A.framework/Headers/a.h === --- /dev/null +++ test/Modules/Inputs/implicit-private-with-different-name/A.framework/Headers/a.h @@ -0,0 +1 @@ +extern int APUBLIC; Index: lib/Lex/ModuleMap.cpp === --- lib/Lex/M
[PATCH] D27852: [modules] Handle modules with nonstandard names in module.private.modulemaps
graydon marked 5 inline comments as done. graydon added a comment. Addressed review comments https://reviews.llvm.org/D27852 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D27852: [modules] Handle modules with nonstandard names in module.private.modulemaps
graydon updated this revision to Diff 82161. graydon added a comment. Updates to address review comments https://reviews.llvm.org/D27852 Files: include/clang/Basic/DiagnosticGroups.td include/clang/Basic/DiagnosticLexKinds.td include/clang/Lex/HeaderSearch.h lib/Lex/HeaderSearch.cpp lib/Lex/ModuleMap.cpp test/Modules/Inputs/implicit-private-with-different-name/A.framework/Headers/a.h test/Modules/Inputs/implicit-private-with-different-name/A.framework/Headers/aprivate.h test/Modules/Inputs/implicit-private-with-different-name/A.framework/Modules/module.modulemap test/Modules/Inputs/implicit-private-with-different-name/A.framework/Modules/module.private.modulemap test/Modules/implicit-private-with-different-name.m Index: test/Modules/implicit-private-with-different-name.m === --- /dev/null +++ test/Modules/implicit-private-with-different-name.m @@ -0,0 +1,20 @@ +// RUN: rm -rf %t + +// Build PCH using A, with adjacent private module APrivate, which winds up being implicitly referenced +// RUN: %clang_cc1 -verify -fmodules -fimplicit-module-maps -fmodules-cache-path=%t -F %S/Inputs/implicit-private-with-different-name -emit-pch -o %t-A.pch %s + +// Use the PCH with no explicit way to resolve PrivateA, still pick it up through MODULE_DIRECTORY reference in PCH control block +// RUN: %clang_cc1 -verify -fmodules -fimplicit-module-maps -fmodules-cache-path=%t -F %S/Inputs/implicit-private-with-different-name -include-pch %t-A.pch %s -fsyntax-only + +// Check the fixit +// RUN: %clang_cc1 -fmodules -fimplicit-module-maps -fmodules-cache-path=%t -F %S/Inputs/implicit-private-with-different-name -include-pch %t-A.pch %s -fsyntax-only -fdiagnostics-parseable-fixits %s 2>&1 | FileCheck %s + +// expected-warning@Inputs/implicit-private-with-different-name/A.framework/Modules/module.private.modulemap:1{{top-level module 'APrivate' in private module map, expected a submodule of 'A'}} +// expected-note@Inputs/implicit-private-with-different-name/A.framework/Modules/module.private.modulemap:1{{make 'APrivate' a submodule of 'A' to ensure it can be found by name}} +// CHECK: fix-it:"{{.*}}/Inputs/implicit-private-with-different-name/A.framework/Modules/module.private.modulemap":{1:18-1:26}:"A.Private" + +#ifndef HEADER +#define HEADER +#import "A/aprivate.h" +const int *y = &APRIVATE; +#endif Index: test/Modules/Inputs/implicit-private-with-different-name/A.framework/Modules/module.private.modulemap === --- /dev/null +++ test/Modules/Inputs/implicit-private-with-different-name/A.framework/Modules/module.private.modulemap @@ -0,0 +1,4 @@ +framework module APrivate { + header "aprivate.h" + export * +} Index: test/Modules/Inputs/implicit-private-with-different-name/A.framework/Modules/module.modulemap === --- /dev/null +++ test/Modules/Inputs/implicit-private-with-different-name/A.framework/Modules/module.modulemap @@ -0,0 +1,4 @@ +framework module A { + header "a.h" + export * +} Index: test/Modules/Inputs/implicit-private-with-different-name/A.framework/Headers/aprivate.h === --- /dev/null +++ test/Modules/Inputs/implicit-private-with-different-name/A.framework/Headers/aprivate.h @@ -0,0 +1 @@ +extern int APRIVATE; Index: test/Modules/Inputs/implicit-private-with-different-name/A.framework/Headers/a.h === --- /dev/null +++ test/Modules/Inputs/implicit-private-with-different-name/A.framework/Headers/a.h @@ -0,0 +1 @@ +extern int APUBLIC; Index: lib/Lex/ModuleMap.cpp === --- lib/Lex/ModuleMap.cpp +++ lib/Lex/ModuleMap.cpp @@ -1490,6 +1490,42 @@ ActiveModule->NoUndeclaredIncludes = true; ActiveModule->Directory = Directory; + if (!ActiveModule->Parent) { +StringRef MapFileName(ModuleMapFile->getName()); +if (MapFileName.endswith("module.private.modulemap") || +MapFileName.endswith("module_private.map")) { + // Adding a top-level module from a private modulemap is likely a + // user error; we check to see if there's another top-level module + // defined in the non-private map in the same dir, and if so emit a + // warning. + for (auto E = Map.module_begin(); E != Map.module_end(); ++E) { +auto const *M = E->getValue(); +if (!M->Parent && +M->Directory == ActiveModule->Directory && +M->Name != ActiveModule->Name) { + Diags.Report(ActiveModule->DefinitionLoc, + diag::warn_mmap_mismatched_top_level_private) +<< ActiveModule->Name << M->Name; + // The pattern we're defending against here is typically due to + // a module named FooPrivate which is supposed to be a submod
[PATCH] D27852: [modules] Handle modules with nonstandard names in module.private.modulemaps
graydon added inline comments. Comment at: test/Modules/implicit-private-with-different-name.m:13 +// expected-warning@Inputs/implicit-private-with-different-name/A.framework/Modules/module.private.modulemap:1{{top-level module 'APrivate' in private module map, expected a submodule of 'A'}} +// expected-note@Inputs/implicit-private-with-different-name/A.framework/Modules/module.private.modulemap:1{{make 'APrivate' a submodule of 'A' to ensure it can be found by name}} +// CHECK: fix-it:"{{.*}}/Inputs/implicit-private-with-different-name/A.framework/Modules/module.private.modulemap":{1:18-1:26}:"A.Private" bruno wrote: > I think we can enhance the usability a bit here, how about: > > warning: top-level module 'APrivate' in private module map, expected a > submodule of 'A' > note: make 'APrivate' a submodule of 'A' to ensure it can be found by name > framework module APrivate { >^ > A.Private > > I'm not sure what you're requesting; as far as I can see that is exactly the diagnostic I'm emitting (along with the fixit) https://reviews.llvm.org/D27852 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D27852: [modules] Handle modules with nonstandard names in module.private.modulemaps
This revision was automatically updated to reflect the committed changes. Closed by commit rL290219: [modules] Handle modules with nonstandard names in module.private.modulemaps (authored by graydon). Changed prior to commit: https://reviews.llvm.org/D27852?vs=82161&id=82177#toc Repository: rL LLVM https://reviews.llvm.org/D27852 Files: cfe/trunk/include/clang/Basic/DiagnosticGroups.td cfe/trunk/include/clang/Basic/DiagnosticLexKinds.td cfe/trunk/include/clang/Lex/HeaderSearch.h cfe/trunk/lib/Lex/HeaderSearch.cpp cfe/trunk/lib/Lex/ModuleMap.cpp cfe/trunk/test/Modules/Inputs/implicit-private-with-different-name/A.framework/Headers/a.h cfe/trunk/test/Modules/Inputs/implicit-private-with-different-name/A.framework/Headers/aprivate.h cfe/trunk/test/Modules/Inputs/implicit-private-with-different-name/A.framework/Modules/module.modulemap cfe/trunk/test/Modules/Inputs/implicit-private-with-different-name/A.framework/Modules/module.private.modulemap cfe/trunk/test/Modules/implicit-private-with-different-name.m Index: cfe/trunk/lib/Lex/ModuleMap.cpp === --- cfe/trunk/lib/Lex/ModuleMap.cpp +++ cfe/trunk/lib/Lex/ModuleMap.cpp @@ -1490,6 +1490,42 @@ ActiveModule->NoUndeclaredIncludes = true; ActiveModule->Directory = Directory; + if (!ActiveModule->Parent) { +StringRef MapFileName(ModuleMapFile->getName()); +if (MapFileName.endswith("module.private.modulemap") || +MapFileName.endswith("module_private.map")) { + // Adding a top-level module from a private modulemap is likely a + // user error; we check to see if there's another top-level module + // defined in the non-private map in the same dir, and if so emit a + // warning. + for (auto E = Map.module_begin(); E != Map.module_end(); ++E) { +auto const *M = E->getValue(); +if (!M->Parent && +M->Directory == ActiveModule->Directory && +M->Name != ActiveModule->Name) { + Diags.Report(ActiveModule->DefinitionLoc, + diag::warn_mmap_mismatched_top_level_private) +<< ActiveModule->Name << M->Name; + // The pattern we're defending against here is typically due to + // a module named FooPrivate which is supposed to be a submodule + // called Foo.Private. Emit a fixit in that case. + auto D = +Diags.Report(ActiveModule->DefinitionLoc, + diag::note_mmap_rename_top_level_private_as_submodule); + D << ActiveModule->Name << M->Name; + StringRef Bad(ActiveModule->Name); + if (Bad.consume_back("Private")) { +SmallString<128> Fixed = Bad; +Fixed.append(".Private"); +D << FixItHint::CreateReplacement(ActiveModule->DefinitionLoc, + Fixed); + } + break; +} + } +} + } + bool Done = false; do { switch (Tok.Kind) { Index: cfe/trunk/lib/Lex/HeaderSearch.cpp === --- cfe/trunk/lib/Lex/HeaderSearch.cpp +++ cfe/trunk/lib/Lex/HeaderSearch.cpp @@ -194,16 +194,36 @@ Module *Module = ModMap.findModule(ModuleName); if (Module || !AllowSearch || !HSOpts->ImplicitModuleMaps) return Module; - + + StringRef SearchName = ModuleName; + Module = lookupModule(ModuleName, SearchName); + + // The facility for "private modules" -- adjacent, optional module maps named + // module.private.modulemap that are supposed to define private submodules -- + // is sometimes misused by frameworks that name their associated private + // module FooPrivate, rather than as a submodule named Foo.Private as + // intended. Here we compensate for such cases by looking in directories named + // Foo.framework, when we previously looked and failed to find a + // FooPrivate.framework. + if (!Module && SearchName.consume_back("Private")) +Module = lookupModule(ModuleName, SearchName); + return Module; +} + +Module *HeaderSearch::lookupModule(StringRef ModuleName, StringRef SearchName) { + Module *Module = nullptr; + // Look through the various header search paths to load any available module // maps, searching for a module map that describes this module. for (unsigned Idx = 0, N = SearchDirs.size(); Idx != N; ++Idx) { if (SearchDirs[Idx].isFramework()) { - // Search for or infer a module map for a framework. + // Search for or infer a module map for a framework. Here we use + // SearchName rather than ModuleName, to permit finding private modules + // named FooPrivate in buggy frameworks named Foo. SmallString<128> FrameworkDirName; FrameworkDirName += SearchDirs[Idx].getFrameworkDir()->getName(); - llvm::sys::path::append(FrameworkDirName, ModuleName + ".framework"); - if (const DirectoryEntry *FrameworkDir + llvm::sys::path::ap
[PATCH] D31378: [PCH] Attach instance's dependency collectors to PCH external AST sources.
graydon created this revision. When a PCH is included via -include-pch, clang should treat the current TU as dependent on the sourcefile that the PCH was generated from. This is currently _partly_ accomplished by InitializePreprocessor calling AddImplicitIncludePCH to synthesize an implicit #include of the sourcefile, into the preprocessor's Predefines buffer. For FrontendActions such as PreprocessOnlyAction (which is, curiously, what the driver winds up running one of in response to a plain clang -M) this is sufficient: the preprocessor cranks over its Predefines and emits a dependency reference to the initial sourcefile. For other FrontendActions (for example -emit-obj or -fsyntax-only) the Predefines buffer is reset to the suggested predefines buffer from the PCH, so the dependency edge is lost. The result is that clang emits a .d file in those cases that lacks a reference to the .h file responsible for the input (and in Swift's case, our .swiftdeps file winds up not including a reference to the source file for a PCH bridging header.) This patch fixes the problem by taking a different tack: ignoring the Predefines buffer (which seems a bit like a hack anyways) and directly attaching the CompilerInstance's DependencyCollectors (and legacy DependencyFileGenerator) to the ASTReader for the external AST. This approach is similar to the one chosen in earlier consultation with Bruno and Ben, and I think it's the least-bad solution, given several options. https://reviews.llvm.org/D31378 Files: include/clang/Frontend/CompilerInstance.h lib/Frontend/CompilerInstance.cpp test/PCH/emit-dependencies.c Index: test/PCH/emit-dependencies.c === --- /dev/null +++ test/PCH/emit-dependencies.c @@ -0,0 +1,9 @@ +// RUN: rm -f %t.pch +// RUN: %clang_cc1 -emit-pch -o %t.pch %S/Inputs/chain-decls1.h +// RUN: %clang_cc1 -include-pch %t.pch -fsyntax-only -MT %s.o -dependency-file - %s | FileCheck %s +// CHECK: Inputs/chain-decls1.h + +int main() { + f(); + return 0; +} Index: lib/Frontend/CompilerInstance.cpp === --- lib/Frontend/CompilerInstance.cpp +++ lib/Frontend/CompilerInstance.cpp @@ -497,6 +497,8 @@ AllowPCHWithCompilerErrors, getPreprocessor(), getASTContext(), getPCHContainerReader(), getFrontendOpts().ModuleFileExtensions, + TheDependencyFileGenerator.get(), + DependencyCollectors, DeserializationListener, OwnDeserializationListener, Preamble, getFrontendOpts().UseGlobalModuleIndex); @@ -507,6 +509,8 @@ bool AllowPCHWithCompilerErrors, Preprocessor &PP, ASTContext &Context, const PCHContainerReader &PCHContainerRdr, ArrayRef> Extensions, +DependencyFileGenerator *DependencyFile, +ArrayRef> DependencyCollectors, void *DeserializationListener, bool OwnDeserializationListener, bool Preamble, bool UseGlobalModuleIndex) { HeaderSearchOptions &HSOpts = PP.getHeaderSearchInfo().getHeaderSearchOpts(); @@ -524,6 +528,12 @@ Reader->setDeserializationListener( static_cast(DeserializationListener), /*TakeOwnership=*/OwnDeserializationListener); + + if (DependencyFile) +DependencyFile->AttachToASTReader(*Reader); + for (auto &Listener : DependencyCollectors) +Listener->attachToASTReader(*Reader); + switch (Reader->ReadAST(Path, Preamble ? serialization::MK_Preamble : serialization::MK_PCH, Index: include/clang/Frontend/CompilerInstance.h === --- include/clang/Frontend/CompilerInstance.h +++ include/clang/Frontend/CompilerInstance.h @@ -662,6 +662,8 @@ bool AllowPCHWithCompilerErrors, Preprocessor &PP, ASTContext &Context, const PCHContainerReader &PCHContainerRdr, ArrayRef> Extensions, + DependencyFileGenerator *DependencyFile, + ArrayRef> DependencyCollectors, void *DeserializationListener, bool OwnDeserializationListener, bool Preamble, bool UseGlobalModuleIndex); Index: test/PCH/emit-dependencies.c === --- /dev/null +++ test/PCH/emit-dependencies.c @@ -0,0 +1,9 @@ +// RUN: rm -f %t.pch +// RUN: %clang_cc1 -emit-pch -o %t.pch %S/Inputs/chain-decls1.h +// RUN: %clang_cc1 -include-pch %t.pch -fsyntax-only -MT %s.o -dependency-file - %s | FileCheck %s +// CHECK: Inputs/chain-decls1.h + +int main() { + f(); + return 0; +} Index: lib/Frontend/CompilerInstance.cpp === --- lib/Frontend/CompilerInstance.cpp +++ lib/Frontend/CompilerInstance.cpp @@ -497,6 +497,8 @@ AllowPCHWithCompilerErrors, getPreprocessor(), getASTContext(), getPCHContainerReader(), getFrontendOpts().ModuleFileExtensions, + TheDependencyFileGenerator.get(), + DependencyCollect
[PATCH] D31378: [PCH] Attach instance's dependency collectors to PCH external AST sources.
This revision was automatically updated to reflect the committed changes. Closed by commit rL299009: [PCH] Attach instance's dependency collectors to PCH external AST sources. (authored by graydon). Changed prior to commit: https://reviews.llvm.org/D31378?vs=93089&id=93387#toc Repository: rL LLVM https://reviews.llvm.org/D31378 Files: cfe/trunk/include/clang/Frontend/CompilerInstance.h cfe/trunk/lib/Frontend/CompilerInstance.cpp cfe/trunk/test/PCH/emit-dependencies.c Index: cfe/trunk/lib/Frontend/CompilerInstance.cpp === --- cfe/trunk/lib/Frontend/CompilerInstance.cpp +++ cfe/trunk/lib/Frontend/CompilerInstance.cpp @@ -497,6 +497,8 @@ AllowPCHWithCompilerErrors, getPreprocessor(), getASTContext(), getPCHContainerReader(), getFrontendOpts().ModuleFileExtensions, + TheDependencyFileGenerator.get(), + DependencyCollectors, DeserializationListener, OwnDeserializationListener, Preamble, getFrontendOpts().UseGlobalModuleIndex); @@ -507,6 +509,8 @@ bool AllowPCHWithCompilerErrors, Preprocessor &PP, ASTContext &Context, const PCHContainerReader &PCHContainerRdr, ArrayRef> Extensions, +DependencyFileGenerator *DependencyFile, +ArrayRef> DependencyCollectors, void *DeserializationListener, bool OwnDeserializationListener, bool Preamble, bool UseGlobalModuleIndex) { HeaderSearchOptions &HSOpts = PP.getHeaderSearchInfo().getHeaderSearchOpts(); @@ -524,6 +528,12 @@ Reader->setDeserializationListener( static_cast(DeserializationListener), /*TakeOwnership=*/OwnDeserializationListener); + + if (DependencyFile) +DependencyFile->AttachToASTReader(*Reader); + for (auto &Listener : DependencyCollectors) +Listener->attachToASTReader(*Reader); + switch (Reader->ReadAST(Path, Preamble ? serialization::MK_Preamble : serialization::MK_PCH, Index: cfe/trunk/include/clang/Frontend/CompilerInstance.h === --- cfe/trunk/include/clang/Frontend/CompilerInstance.h +++ cfe/trunk/include/clang/Frontend/CompilerInstance.h @@ -662,6 +662,8 @@ bool AllowPCHWithCompilerErrors, Preprocessor &PP, ASTContext &Context, const PCHContainerReader &PCHContainerRdr, ArrayRef> Extensions, + DependencyFileGenerator *DependencyFile, + ArrayRef> DependencyCollectors, void *DeserializationListener, bool OwnDeserializationListener, bool Preamble, bool UseGlobalModuleIndex); Index: cfe/trunk/test/PCH/emit-dependencies.c === --- cfe/trunk/test/PCH/emit-dependencies.c +++ cfe/trunk/test/PCH/emit-dependencies.c @@ -0,0 +1,9 @@ +// RUN: rm -f %t.pch +// RUN: %clang_cc1 -emit-pch -o %t.pch %S/Inputs/chain-decls1.h +// RUN: %clang_cc1 -include-pch %t.pch -fsyntax-only -MT %s.o -dependency-file - %s | FileCheck %s +// CHECK: Inputs/chain-decls1.h + +int main() { + f(); + return 0; +} Index: cfe/trunk/lib/Frontend/CompilerInstance.cpp === --- cfe/trunk/lib/Frontend/CompilerInstance.cpp +++ cfe/trunk/lib/Frontend/CompilerInstance.cpp @@ -497,6 +497,8 @@ AllowPCHWithCompilerErrors, getPreprocessor(), getASTContext(), getPCHContainerReader(), getFrontendOpts().ModuleFileExtensions, + TheDependencyFileGenerator.get(), + DependencyCollectors, DeserializationListener, OwnDeserializationListener, Preamble, getFrontendOpts().UseGlobalModuleIndex); @@ -507,6 +509,8 @@ bool AllowPCHWithCompilerErrors, Preprocessor &PP, ASTContext &Context, const PCHContainerReader &PCHContainerRdr, ArrayRef> Extensions, +DependencyFileGenerator *DependencyFile, +ArrayRef> DependencyCollectors, void *DeserializationListener, bool OwnDeserializationListener, bool Preamble, bool UseGlobalModuleIndex) { HeaderSearchOptions &HSOpts = PP.getHeaderSearchInfo().getHeaderSearchOpts(); @@ -524,6 +528,12 @@ Reader->setDeserializationListener( static_cast(DeserializationListener), /*TakeOwnership=*/OwnDeserializationListener); + + if (DependencyFile) +DependencyFile->AttachToASTReader(*Reader); + for (auto &Listener : DependencyCollectors) +Listener->attachToASTReader(*Reader); + switch (Reader->ReadAST(Path, Preamble ? serialization::MK_Preamble : serialization::MK_PCH, Index: cfe/trunk/include/clang/Frontend/CompilerInstance.h === --- cfe/trunk/include/clang/Frontend/CompilerInstance.h +++ cfe/trunk/include/clang/Frontend/CompilerInstance.h @@ -662,6 +662,8 @@ bool AllowPCHWithCompilerErrors, Preprocessor &PP, ASTContext &Context, const PCHContainerReade
[PATCH] D28779: [ASTReader] Add a DeserializationListener callback for IMPORTED_MODULES
graydon created this revision. Add a callback from ASTReader to DeserializationListener when the former reads an IMPORTED_MODULES block. This supports Swift in using PCH for bridging headers. https://reviews.llvm.org/D28779 Files: include/clang/Serialization/ASTDeserializationListener.h lib/Serialization/ASTReader.cpp Index: lib/Serialization/ASTReader.cpp === --- lib/Serialization/ASTReader.cpp +++ lib/Serialization/ASTReader.cpp @@ -3245,8 +3245,11 @@ for (unsigned I = 0, N = Record.size(); I != N; /**/) { unsigned GlobalID = getGlobalSubmoduleID(F, Record[I++]); SourceLocation Loc = ReadSourceLocation(F, Record, I); - if (GlobalID) + if (GlobalID) { ImportedModules.push_back(ImportedSubmodule(GlobalID, Loc)); +if (DeserializationListener) + DeserializationListener->ModuleImportRead(GlobalID, Loc); + } } } break; Index: include/clang/Serialization/ASTDeserializationListener.h === --- include/clang/Serialization/ASTDeserializationListener.h +++ include/clang/Serialization/ASTDeserializationListener.h @@ -26,6 +26,7 @@ class MacroDefinitionRecord; class MacroInfo; class Module; +class SourceLocation; class ASTDeserializationListener { public: @@ -52,6 +53,9 @@ MacroDefinitionRecord *MD) {} /// \brief A module definition was read from the AST file. virtual void ModuleRead(serialization::SubmoduleID ID, Module *Mod) {} + /// \brief A module import was read from the AST file. + virtual void ModuleImportRead(serialization::SubmoduleID ID, +SourceLocation ImportLoc) {} }; } Index: lib/Serialization/ASTReader.cpp === --- lib/Serialization/ASTReader.cpp +++ lib/Serialization/ASTReader.cpp @@ -3245,8 +3245,11 @@ for (unsigned I = 0, N = Record.size(); I != N; /**/) { unsigned GlobalID = getGlobalSubmoduleID(F, Record[I++]); SourceLocation Loc = ReadSourceLocation(F, Record, I); - if (GlobalID) + if (GlobalID) { ImportedModules.push_back(ImportedSubmodule(GlobalID, Loc)); +if (DeserializationListener) + DeserializationListener->ModuleImportRead(GlobalID, Loc); + } } } break; Index: include/clang/Serialization/ASTDeserializationListener.h === --- include/clang/Serialization/ASTDeserializationListener.h +++ include/clang/Serialization/ASTDeserializationListener.h @@ -26,6 +26,7 @@ class MacroDefinitionRecord; class MacroInfo; class Module; +class SourceLocation; class ASTDeserializationListener { public: @@ -52,6 +53,9 @@ MacroDefinitionRecord *MD) {} /// \brief A module definition was read from the AST file. virtual void ModuleRead(serialization::SubmoduleID ID, Module *Mod) {} + /// \brief A module import was read from the AST file. + virtual void ModuleImportRead(serialization::SubmoduleID ID, +SourceLocation ImportLoc) {} }; } ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D28790: [Modules] Correct test comment from obsolete earlier version of code. NFC
graydon created this revision. Code committed in https://reviews.llvm.org/rL290219 went through a few iterations; test wound up with stale comment. https://reviews.llvm.org/D28790 Files: test/Modules/implicit-private-with-different-name.m Index: test/Modules/implicit-private-with-different-name.m === --- test/Modules/implicit-private-with-different-name.m +++ test/Modules/implicit-private-with-different-name.m @@ -3,7 +3,7 @@ // Build PCH using A, with adjacent private module APrivate, which winds up being implicitly referenced // RUN: %clang_cc1 -verify -fmodules -fimplicit-module-maps -fmodules-cache-path=%t -F %S/Inputs/implicit-private-with-different-name -emit-pch -o %t-A.pch %s -// Use the PCH with no explicit way to resolve PrivateA, still pick it up through MODULE_DIRECTORY reference in PCH control block +// Use the PCH with no explicit way to resolve APrivate, still pick it up by automatic second-chance search for "A" with "Private" appended // RUN: %clang_cc1 -verify -fmodules -fimplicit-module-maps -fmodules-cache-path=%t -F %S/Inputs/implicit-private-with-different-name -include-pch %t-A.pch %s -fsyntax-only // Check the fixit Index: test/Modules/implicit-private-with-different-name.m === --- test/Modules/implicit-private-with-different-name.m +++ test/Modules/implicit-private-with-different-name.m @@ -3,7 +3,7 @@ // Build PCH using A, with adjacent private module APrivate, which winds up being implicitly referenced // RUN: %clang_cc1 -verify -fmodules -fimplicit-module-maps -fmodules-cache-path=%t -F %S/Inputs/implicit-private-with-different-name -emit-pch -o %t-A.pch %s -// Use the PCH with no explicit way to resolve PrivateA, still pick it up through MODULE_DIRECTORY reference in PCH control block +// Use the PCH with no explicit way to resolve APrivate, still pick it up by automatic second-chance search for "A" with "Private" appended // RUN: %clang_cc1 -verify -fmodules -fimplicit-module-maps -fmodules-cache-path=%t -F %S/Inputs/implicit-private-with-different-name -include-pch %t-A.pch %s -fsyntax-only // Check the fixit ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D28790: [Modules] Correct test comment from obsolete earlier version of code. NFC
This revision was automatically updated to reflect the committed changes. Closed by commit rL292435: [Modules] Correct test comment from obsolete earlier version of code. NFC (authored by graydon). Changed prior to commit: https://reviews.llvm.org/D28790?vs=84616&id=84879#toc Repository: rL LLVM https://reviews.llvm.org/D28790 Files: cfe/trunk/test/Modules/implicit-private-with-different-name.m Index: cfe/trunk/test/Modules/implicit-private-with-different-name.m === --- cfe/trunk/test/Modules/implicit-private-with-different-name.m +++ cfe/trunk/test/Modules/implicit-private-with-different-name.m @@ -3,7 +3,7 @@ // Build PCH using A, with adjacent private module APrivate, which winds up being implicitly referenced // RUN: %clang_cc1 -verify -fmodules -fimplicit-module-maps -fmodules-cache-path=%t -F %S/Inputs/implicit-private-with-different-name -emit-pch -o %t-A.pch %s -// Use the PCH with no explicit way to resolve PrivateA, still pick it up through MODULE_DIRECTORY reference in PCH control block +// Use the PCH with no explicit way to resolve APrivate, still pick it up by automatic second-chance search for "A" with "Private" appended // RUN: %clang_cc1 -verify -fmodules -fimplicit-module-maps -fmodules-cache-path=%t -F %S/Inputs/implicit-private-with-different-name -include-pch %t-A.pch %s -fsyntax-only // Check the fixit Index: cfe/trunk/test/Modules/implicit-private-with-different-name.m === --- cfe/trunk/test/Modules/implicit-private-with-different-name.m +++ cfe/trunk/test/Modules/implicit-private-with-different-name.m @@ -3,7 +3,7 @@ // Build PCH using A, with adjacent private module APrivate, which winds up being implicitly referenced // RUN: %clang_cc1 -verify -fmodules -fimplicit-module-maps -fmodules-cache-path=%t -F %S/Inputs/implicit-private-with-different-name -emit-pch -o %t-A.pch %s -// Use the PCH with no explicit way to resolve PrivateA, still pick it up through MODULE_DIRECTORY reference in PCH control block +// Use the PCH with no explicit way to resolve APrivate, still pick it up by automatic second-chance search for "A" with "Private" appended // RUN: %clang_cc1 -verify -fmodules -fimplicit-module-maps -fmodules-cache-path=%t -F %S/Inputs/implicit-private-with-different-name -include-pch %t-A.pch %s -fsyntax-only // Check the fixit ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D28779: [ASTReader] Add a DeserializationListener callback for IMPORTED_MODULES
This revision was automatically updated to reflect the committed changes. Closed by commit rL292436: [ASTReader] Add a DeserializationListener callback for IMPORTED_MODULES (authored by graydon). Changed prior to commit: https://reviews.llvm.org/D28779?vs=84594&id=84881#toc Repository: rL LLVM https://reviews.llvm.org/D28779 Files: cfe/trunk/include/clang/Serialization/ASTDeserializationListener.h cfe/trunk/lib/Serialization/ASTReader.cpp Index: cfe/trunk/include/clang/Serialization/ASTDeserializationListener.h === --- cfe/trunk/include/clang/Serialization/ASTDeserializationListener.h +++ cfe/trunk/include/clang/Serialization/ASTDeserializationListener.h @@ -26,6 +26,7 @@ class MacroDefinitionRecord; class MacroInfo; class Module; +class SourceLocation; class ASTDeserializationListener { public: @@ -52,6 +53,9 @@ MacroDefinitionRecord *MD) {} /// \brief A module definition was read from the AST file. virtual void ModuleRead(serialization::SubmoduleID ID, Module *Mod) {} + /// \brief A module import was read from the AST file. + virtual void ModuleImportRead(serialization::SubmoduleID ID, +SourceLocation ImportLoc) {} }; } Index: cfe/trunk/lib/Serialization/ASTReader.cpp === --- cfe/trunk/lib/Serialization/ASTReader.cpp +++ cfe/trunk/lib/Serialization/ASTReader.cpp @@ -3251,8 +3251,11 @@ for (unsigned I = 0, N = Record.size(); I != N; /**/) { unsigned GlobalID = getGlobalSubmoduleID(F, Record[I++]); SourceLocation Loc = ReadSourceLocation(F, Record, I); - if (GlobalID) + if (GlobalID) { ImportedModules.push_back(ImportedSubmodule(GlobalID, Loc)); +if (DeserializationListener) + DeserializationListener->ModuleImportRead(GlobalID, Loc); + } } } break; Index: cfe/trunk/include/clang/Serialization/ASTDeserializationListener.h === --- cfe/trunk/include/clang/Serialization/ASTDeserializationListener.h +++ cfe/trunk/include/clang/Serialization/ASTDeserializationListener.h @@ -26,6 +26,7 @@ class MacroDefinitionRecord; class MacroInfo; class Module; +class SourceLocation; class ASTDeserializationListener { public: @@ -52,6 +53,9 @@ MacroDefinitionRecord *MD) {} /// \brief A module definition was read from the AST file. virtual void ModuleRead(serialization::SubmoduleID ID, Module *Mod) {} + /// \brief A module import was read from the AST file. + virtual void ModuleImportRead(serialization::SubmoduleID ID, +SourceLocation ImportLoc) {} }; } Index: cfe/trunk/lib/Serialization/ASTReader.cpp === --- cfe/trunk/lib/Serialization/ASTReader.cpp +++ cfe/trunk/lib/Serialization/ASTReader.cpp @@ -3251,8 +3251,11 @@ for (unsigned I = 0, N = Record.size(); I != N; /**/) { unsigned GlobalID = getGlobalSubmoduleID(F, Record[I++]); SourceLocation Loc = ReadSourceLocation(F, Record, I); - if (GlobalID) + if (GlobalID) { ImportedModules.push_back(ImportedSubmodule(GlobalID, Loc)); +if (DeserializationListener) + DeserializationListener->ModuleImportRead(GlobalID, Loc); + } } } break; ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D34741: [ASTReader] Treat multiple defns of ObjC protocols the same as interfaces.
graydon created this revision. In change 2ba19793512, the ASTReader logic for ObjC interfaces was modified to preserve the first definition-data read, "merging" later definitions into it rather than overwriting it (though this "merging" is, in practice, a no-op that discards the later definition-data). Unfortunately this change was only made to ObjC interfaces, not protocols; this means that when (for example) loading a protocol that references an interface, if both the protocol and interface are multiply defined (as can easily happen if the same header is read from multiple contexts), an _inconsistent_ pair of definitions is loaded: first-read for the interface and last-read for the protocol. This in turn causes very subtle downstream bugs in the Swift ClangImporter, which filters the results of name lookups based on the owning module of a definition; inconsistency between a pair of related definitions causes name lookup failures at various stages of compilation. To fix these downstream issues, this change replicates the logic applied to interfaces in change 2ba19793512, but for ObjC protocols. rdar://30851899 https://reviews.llvm.org/D34741 Files: include/clang/AST/Redeclarable.h lib/Serialization/ASTReaderDecl.cpp Index: lib/Serialization/ASTReaderDecl.cpp === --- lib/Serialization/ASTReaderDecl.cpp +++ lib/Serialization/ASTReaderDecl.cpp @@ -126,6 +126,9 @@ void ReadObjCDefinitionData(struct ObjCInterfaceDecl::DefinitionData &Data); void MergeDefinitionData(ObjCInterfaceDecl *D, struct ObjCInterfaceDecl::DefinitionData &&NewDD); +void ReadObjCDefinitionData(struct ObjCProtocolDecl::DefinitionData &Data); +void MergeDefinitionData(ObjCProtocolDecl *D, + struct ObjCProtocolDecl::DefinitionData &&NewDD); static NamedDecl *getAnonymousDeclForMerging(ASTReader &Reader, DeclContext *DC, @@ -1045,18 +1048,8 @@ IVD->setSynthesize(synth); } -void ASTDeclReader::VisitObjCProtocolDecl(ObjCProtocolDecl *PD) { - RedeclarableResult Redecl = VisitRedeclarable(PD); - VisitObjCContainerDecl(PD); - mergeRedeclarable(PD, Redecl); - - if (Record.readInt()) { -// Read the definition. -PD->allocateDefinitionData(); - -// Set the definition data of the canonical declaration, so other -// redeclarations will see it. -PD->getCanonicalDecl()->Data = PD->Data; +void ASTDeclReader::ReadObjCDefinitionData( + struct ObjCProtocolDecl::DefinitionData &Data) { unsigned NumProtoRefs = Record.readInt(); SmallVector ProtoRefs; @@ -1067,9 +1060,37 @@ ProtoLocs.reserve(NumProtoRefs); for (unsigned I = 0; I != NumProtoRefs; ++I) ProtoLocs.push_back(ReadSourceLocation()); -PD->setProtocolList(ProtoRefs.data(), NumProtoRefs, ProtoLocs.data(), -Reader.getContext()); +Data.ReferencedProtocols.set(ProtoRefs.data(), NumProtoRefs, + ProtoLocs.data(), Reader.getContext()); +} + +void ASTDeclReader::MergeDefinitionData(ObjCProtocolDecl *D, + struct ObjCProtocolDecl::DefinitionData &&NewDD) { + // FIXME: odr checking? +} +void ASTDeclReader::VisitObjCProtocolDecl(ObjCProtocolDecl *PD) { + RedeclarableResult Redecl = VisitRedeclarable(PD); + VisitObjCContainerDecl(PD); + mergeRedeclarable(PD, Redecl); + + if (Record.readInt()) { +// Read the definition. +PD->allocateDefinitionData(); + +ReadObjCDefinitionData(PD->data()); + +ObjCProtocolDecl *Canon = PD->getCanonicalDecl(); +if (Canon->Data.getPointer()) { + // If we already have a definition, keep the definition invariant and + // merge the data. + MergeDefinitionData(Canon, std::move(PD->data())); + PD->Data = Canon->Data; +} else { + // Set the definition data of the canonical declaration, so other + // redeclarations will see it. + PD->getCanonicalDecl()->Data = PD->Data; +} // Note that we have deserialized a definition. Reader.PendingDefinitions.insert(PD); } else { Index: include/clang/AST/Redeclarable.h === --- include/clang/AST/Redeclarable.h +++ include/clang/AST/Redeclarable.h @@ -21,6 +21,60 @@ namespace clang { class ASTContext; +// Some notes on redeclarables: +// +// - Every redeclarable is on a circular linked list. +// +// - Every decl has a pointer to the first element of the chain _and_ a +//DeclLink that may point to one of 3 possible states: +// - the "previous" (temporal) element in the chain +// - the "latest" (temporal) element in the chain +// - the an "uninitialized-latest" value (when newly-constructed) +// +// - The first element is also often called the canonical element. Every +//element has a pointer to it so that "getCanonical" can be fast. +// +// - Mo
[PATCH] D34741: [ASTReader] Treat multiple defns of ObjC protocols the same as interfaces.
This revision was automatically updated to reflect the committed changes. Closed by commit rL306583: [ASTReader] Treat multiple defns of ObjC protocols the same as interfaces. (authored by graydon). Repository: rL LLVM https://reviews.llvm.org/D34741 Files: cfe/trunk/include/clang/AST/Redeclarable.h cfe/trunk/lib/Serialization/ASTReaderDecl.cpp Index: cfe/trunk/lib/Serialization/ASTReaderDecl.cpp === --- cfe/trunk/lib/Serialization/ASTReaderDecl.cpp +++ cfe/trunk/lib/Serialization/ASTReaderDecl.cpp @@ -126,6 +126,9 @@ void ReadObjCDefinitionData(struct ObjCInterfaceDecl::DefinitionData &Data); void MergeDefinitionData(ObjCInterfaceDecl *D, struct ObjCInterfaceDecl::DefinitionData &&NewDD); +void ReadObjCDefinitionData(struct ObjCProtocolDecl::DefinitionData &Data); +void MergeDefinitionData(ObjCProtocolDecl *D, + struct ObjCProtocolDecl::DefinitionData &&NewDD); static NamedDecl *getAnonymousDeclForMerging(ASTReader &Reader, DeclContext *DC, @@ -1045,18 +1048,8 @@ IVD->setSynthesize(synth); } -void ASTDeclReader::VisitObjCProtocolDecl(ObjCProtocolDecl *PD) { - RedeclarableResult Redecl = VisitRedeclarable(PD); - VisitObjCContainerDecl(PD); - mergeRedeclarable(PD, Redecl); - - if (Record.readInt()) { -// Read the definition. -PD->allocateDefinitionData(); - -// Set the definition data of the canonical declaration, so other -// redeclarations will see it. -PD->getCanonicalDecl()->Data = PD->Data; +void ASTDeclReader::ReadObjCDefinitionData( + struct ObjCProtocolDecl::DefinitionData &Data) { unsigned NumProtoRefs = Record.readInt(); SmallVector ProtoRefs; @@ -1067,9 +1060,37 @@ ProtoLocs.reserve(NumProtoRefs); for (unsigned I = 0; I != NumProtoRefs; ++I) ProtoLocs.push_back(ReadSourceLocation()); -PD->setProtocolList(ProtoRefs.data(), NumProtoRefs, ProtoLocs.data(), -Reader.getContext()); +Data.ReferencedProtocols.set(ProtoRefs.data(), NumProtoRefs, + ProtoLocs.data(), Reader.getContext()); +} + +void ASTDeclReader::MergeDefinitionData(ObjCProtocolDecl *D, + struct ObjCProtocolDecl::DefinitionData &&NewDD) { + // FIXME: odr checking? +} +void ASTDeclReader::VisitObjCProtocolDecl(ObjCProtocolDecl *PD) { + RedeclarableResult Redecl = VisitRedeclarable(PD); + VisitObjCContainerDecl(PD); + mergeRedeclarable(PD, Redecl); + + if (Record.readInt()) { +// Read the definition. +PD->allocateDefinitionData(); + +ReadObjCDefinitionData(PD->data()); + +ObjCProtocolDecl *Canon = PD->getCanonicalDecl(); +if (Canon->Data.getPointer()) { + // If we already have a definition, keep the definition invariant and + // merge the data. + MergeDefinitionData(Canon, std::move(PD->data())); + PD->Data = Canon->Data; +} else { + // Set the definition data of the canonical declaration, so other + // redeclarations will see it. + PD->getCanonicalDecl()->Data = PD->Data; +} // Note that we have deserialized a definition. Reader.PendingDefinitions.insert(PD); } else { Index: cfe/trunk/include/clang/AST/Redeclarable.h === --- cfe/trunk/include/clang/AST/Redeclarable.h +++ cfe/trunk/include/clang/AST/Redeclarable.h @@ -21,6 +21,60 @@ namespace clang { class ASTContext; +// Some notes on redeclarables: +// +// - Every redeclarable is on a circular linked list. +// +// - Every decl has a pointer to the first element of the chain _and_ a +//DeclLink that may point to one of 3 possible states: +// - the "previous" (temporal) element in the chain +// - the "latest" (temporal) element in the chain +// - the an "uninitialized-latest" value (when newly-constructed) +// +// - The first element is also often called the canonical element. Every +//element has a pointer to it so that "getCanonical" can be fast. +// +// - Most links in the chain point to previous, except the link out of +//the first; it points to latest. +// +// - Elements are called "first", "previous", "latest" or +//"most-recent" when referring to temporal order: order of addition +//to the chain. +// +// - To make matters confusing, the DeclLink type uses the term "next" +//for its pointer-storage internally (thus functions like +//NextIsPrevious). It's easiest to just ignore the implementation of +//DeclLink when making sense of the redeclaration chain. +// +// - There's also a "definition" link for several types of +//redeclarable, where only one definition should exist at any given +//time (and the defn pointer is stored in the decl's "data" which +//is copied to every element on the chain when it's changed). +// +//Here is
[PATCH] D34788: [ASTReader] Add test for previous change r306583 / 145692e.
graydon created this revision. Add a test for the change to ASTReader that reproduces the logic for consolidating multiple ObjC interface definitions to the case of multiple ObjC protocol definitions. This test is a modified copy of the test that accompanied the original change to interfaces, in 2ba1979. https://reviews.llvm.org/D34788 Files: test/Modules/Inputs/lookup-assert-protocol/Base.h test/Modules/Inputs/lookup-assert-protocol/Derive.h test/Modules/Inputs/lookup-assert-protocol/H3.h test/Modules/Inputs/lookup-assert-protocol/module.map test/Modules/lookup-assert-protocol.m Index: test/Modules/lookup-assert-protocol.m === --- /dev/null +++ test/Modules/lookup-assert-protocol.m @@ -0,0 +1,17 @@ +// RUN: rm -rf %t +// RUN: %clang_cc1 -fmodules-cache-path=%t -fmodules -fimplicit-module-maps -I %S/Inputs/lookup-assert-protocol %s -verify +// expected-no-diagnostics + +#include "Derive.h" +#import + +__attribute__((objc_root_class)) +@interface Thing +@end + +@implementation Thing +- (void)test { +} +- (void)test2 { +} +@end Index: test/Modules/Inputs/lookup-assert-protocol/module.map === --- /dev/null +++ test/Modules/Inputs/lookup-assert-protocol/module.map @@ -0,0 +1,4 @@ +module X { + header "H3.h" + export * +} Index: test/Modules/Inputs/lookup-assert-protocol/H3.h === --- /dev/null +++ test/Modules/Inputs/lookup-assert-protocol/H3.h @@ -0,0 +1 @@ +#include "Base.h" Index: test/Modules/Inputs/lookup-assert-protocol/Derive.h === --- /dev/null +++ test/Modules/Inputs/lookup-assert-protocol/Derive.h @@ -0,0 +1,4 @@ +#include "Base.h" +@protocol DerivedProtocol +- (void) test2; +@end Index: test/Modules/Inputs/lookup-assert-protocol/Base.h === --- /dev/null +++ test/Modules/Inputs/lookup-assert-protocol/Base.h @@ -0,0 +1,3 @@ +@protocol BaseProtocol +- (void) test; +@end Index: test/Modules/lookup-assert-protocol.m === --- /dev/null +++ test/Modules/lookup-assert-protocol.m @@ -0,0 +1,17 @@ +// RUN: rm -rf %t +// RUN: %clang_cc1 -fmodules-cache-path=%t -fmodules -fimplicit-module-maps -I %S/Inputs/lookup-assert-protocol %s -verify +// expected-no-diagnostics + +#include "Derive.h" +#import + +__attribute__((objc_root_class)) +@interface Thing +@end + +@implementation Thing +- (void)test { +} +- (void)test2 { +} +@end Index: test/Modules/Inputs/lookup-assert-protocol/module.map === --- /dev/null +++ test/Modules/Inputs/lookup-assert-protocol/module.map @@ -0,0 +1,4 @@ +module X { + header "H3.h" + export * +} Index: test/Modules/Inputs/lookup-assert-protocol/H3.h === --- /dev/null +++ test/Modules/Inputs/lookup-assert-protocol/H3.h @@ -0,0 +1 @@ +#include "Base.h" Index: test/Modules/Inputs/lookup-assert-protocol/Derive.h === --- /dev/null +++ test/Modules/Inputs/lookup-assert-protocol/Derive.h @@ -0,0 +1,4 @@ +#include "Base.h" +@protocol DerivedProtocol +- (void) test2; +@end Index: test/Modules/Inputs/lookup-assert-protocol/Base.h === --- /dev/null +++ test/Modules/Inputs/lookup-assert-protocol/Base.h @@ -0,0 +1,3 @@ +@protocol BaseProtocol +- (void) test; +@end ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D34788: [ASTReader] Add test for previous change r306583 / 145692e.
This revision was automatically updated to reflect the committed changes. Closed by commit rL306732: [ASTReader] Add test for previous change r306583 / 145692e. (authored by graydon). Repository: rL LLVM https://reviews.llvm.org/D34788 Files: cfe/trunk/test/Modules/Inputs/lookup-assert-protocol/Base.h cfe/trunk/test/Modules/Inputs/lookup-assert-protocol/Derive.h cfe/trunk/test/Modules/Inputs/lookup-assert-protocol/H3.h cfe/trunk/test/Modules/Inputs/lookup-assert-protocol/module.map cfe/trunk/test/Modules/lookup-assert-protocol.m Index: cfe/trunk/test/Modules/Inputs/lookup-assert-protocol/module.map === --- cfe/trunk/test/Modules/Inputs/lookup-assert-protocol/module.map +++ cfe/trunk/test/Modules/Inputs/lookup-assert-protocol/module.map @@ -0,0 +1,4 @@ +module X { + header "H3.h" + export * +} Index: cfe/trunk/test/Modules/Inputs/lookup-assert-protocol/H3.h === --- cfe/trunk/test/Modules/Inputs/lookup-assert-protocol/H3.h +++ cfe/trunk/test/Modules/Inputs/lookup-assert-protocol/H3.h @@ -0,0 +1 @@ +#include "Base.h" Index: cfe/trunk/test/Modules/Inputs/lookup-assert-protocol/Base.h === --- cfe/trunk/test/Modules/Inputs/lookup-assert-protocol/Base.h +++ cfe/trunk/test/Modules/Inputs/lookup-assert-protocol/Base.h @@ -0,0 +1,3 @@ +@protocol BaseProtocol +- (void) test; +@end Index: cfe/trunk/test/Modules/Inputs/lookup-assert-protocol/Derive.h === --- cfe/trunk/test/Modules/Inputs/lookup-assert-protocol/Derive.h +++ cfe/trunk/test/Modules/Inputs/lookup-assert-protocol/Derive.h @@ -0,0 +1,4 @@ +#include "Base.h" +@protocol DerivedProtocol +- (void) test2; +@end Index: cfe/trunk/test/Modules/lookup-assert-protocol.m === --- cfe/trunk/test/Modules/lookup-assert-protocol.m +++ cfe/trunk/test/Modules/lookup-assert-protocol.m @@ -0,0 +1,17 @@ +// RUN: rm -rf %t +// RUN: %clang_cc1 -fmodules-cache-path=%t -fmodules -fimplicit-module-maps -I %S/Inputs/lookup-assert-protocol %s -verify +// expected-no-diagnostics + +#include "Derive.h" +#import + +__attribute__((objc_root_class)) +@interface Thing +@end + +@implementation Thing +- (void)test { +} +- (void)test2 { +} +@end Index: cfe/trunk/test/Modules/Inputs/lookup-assert-protocol/module.map === --- cfe/trunk/test/Modules/Inputs/lookup-assert-protocol/module.map +++ cfe/trunk/test/Modules/Inputs/lookup-assert-protocol/module.map @@ -0,0 +1,4 @@ +module X { + header "H3.h" + export * +} Index: cfe/trunk/test/Modules/Inputs/lookup-assert-protocol/H3.h === --- cfe/trunk/test/Modules/Inputs/lookup-assert-protocol/H3.h +++ cfe/trunk/test/Modules/Inputs/lookup-assert-protocol/H3.h @@ -0,0 +1 @@ +#include "Base.h" Index: cfe/trunk/test/Modules/Inputs/lookup-assert-protocol/Base.h === --- cfe/trunk/test/Modules/Inputs/lookup-assert-protocol/Base.h +++ cfe/trunk/test/Modules/Inputs/lookup-assert-protocol/Base.h @@ -0,0 +1,3 @@ +@protocol BaseProtocol +- (void) test; +@end Index: cfe/trunk/test/Modules/Inputs/lookup-assert-protocol/Derive.h === --- cfe/trunk/test/Modules/Inputs/lookup-assert-protocol/Derive.h +++ cfe/trunk/test/Modules/Inputs/lookup-assert-protocol/Derive.h @@ -0,0 +1,4 @@ +#include "Base.h" +@protocol DerivedProtocol +- (void) test2; +@end Index: cfe/trunk/test/Modules/lookup-assert-protocol.m === --- cfe/trunk/test/Modules/lookup-assert-protocol.m +++ cfe/trunk/test/Modules/lookup-assert-protocol.m @@ -0,0 +1,17 @@ +// RUN: rm -rf %t +// RUN: %clang_cc1 -fmodules-cache-path=%t -fmodules -fimplicit-module-maps -I %S/Inputs/lookup-assert-protocol %s -verify +// expected-no-diagnostics + +#include "Derive.h" +#import + +__attribute__((objc_root_class)) +@interface Thing +@end + +@implementation Thing +- (void)test { +} +- (void)test2 { +} +@end ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D27580: [modules] Add optional out-param to ASTReader::ReadAST for imported submodules.
graydon created this revision. graydon added reviewers: doug.gregor, manmanren, rsmith. graydon added a subscriber: cfe-commits. The Swift frontend is acquiring the ability to load non-module PCH files containing bridging definitions from C/ObjC. As part of this work, it needs to know which submodules were imported by a PCH in order to wrap them in local Swift modules. This information is collected by ASTReader::ReadAST in a local vector, but is currently kept private. The change here is just to make the type of the vector elements public, and provide an optional out-parameter to the ReadAST method to provide the vector's contents to a caller after a successful read. https://reviews.llvm.org/D27580 Files: include/clang/Serialization/ASTReader.h lib/Serialization/ASTReader.cpp Index: lib/Serialization/ASTReader.cpp === --- lib/Serialization/ASTReader.cpp +++ lib/Serialization/ASTReader.cpp @@ -3578,7 +3578,8 @@ ASTReader::ASTReadResult ASTReader::ReadAST(StringRef FileName, ModuleKind Type, SourceLocation ImportLoc, -unsigned ClientLoadCapabilities) { +unsigned ClientLoadCapabilities, +SmallVectorImpl *Imported) { llvm::SaveAndRestore SetCurImportLocRAII(CurrentImportLoc, ImportLoc); @@ -3744,6 +3745,10 @@ } UnresolvedModuleRefs.clear(); + if (Imported) +Imported->append(ImportedModules.begin(), + ImportedModules.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 // module itself. Index: include/clang/Serialization/ASTReader.h === --- include/clang/Serialization/ASTReader.h +++ include/clang/Serialization/ASTReader.h @@ -821,14 +821,16 @@ // \brief A list of late parsed template function data. SmallVector LateParsedTemplates; +public: struct ImportedSubmodule { serialization::SubmoduleID ID; SourceLocation ImportLoc; ImportedSubmodule(serialization::SubmoduleID ID, SourceLocation ImportLoc) : ID(ID), ImportLoc(ImportLoc) {} }; +private: /// \brief A list of modules that were imported by precompiled headers or /// any other non-module AST file. SmallVector ImportedModules; @@ -1404,9 +1406,13 @@ /// \param ClientLoadCapabilities The set of client load-failure /// capabilities, represented as a bitset of the enumerators of /// LoadFailureCapabilities. + /// + /// \param Imported optional out-parameter to append the list of modules + /// that were imported by precompiled headers or any other non-module AST file ASTReadResult ReadAST(StringRef FileName, ModuleKind Type, SourceLocation ImportLoc, -unsigned ClientLoadCapabilities); +unsigned ClientLoadCapabilities, +SmallVectorImpl *Imported = nullptr); /// \brief Make the entities in the given module and any of its (non-explicit) /// submodules visible to name lookup. Index: lib/Serialization/ASTReader.cpp === --- lib/Serialization/ASTReader.cpp +++ lib/Serialization/ASTReader.cpp @@ -3578,7 +3578,8 @@ ASTReader::ASTReadResult ASTReader::ReadAST(StringRef FileName, ModuleKind Type, SourceLocation ImportLoc, -unsigned ClientLoadCapabilities) { +unsigned ClientLoadCapabilities, +SmallVectorImpl *Imported) { llvm::SaveAndRestore SetCurImportLocRAII(CurrentImportLoc, ImportLoc); @@ -3744,6 +3745,10 @@ } UnresolvedModuleRefs.clear(); + if (Imported) +Imported->append(ImportedModules.begin(), + ImportedModules.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 // module itself. Index: include/clang/Serialization/ASTReader.h === --- include/clang/Serialization/ASTReader.h +++ include/clang/Serialization/ASTReader.h @@ -821,14 +821,16 @@ // \brief A list of late parsed template function data. SmallVector LateParsedTemplates; +public: struct ImportedSubmodule { serialization::SubmoduleID ID; SourceLocation ImportLoc; ImportedSubmodule(serialization::SubmoduleID ID, SourceLocation ImportLoc) : ID(ID), ImportLoc(ImportLoc) {} }; +private: /// \brief
[PATCH] D27580: [modules] Add optional out-param to ASTReader::ReadAST for imported submodules.
This revision was automatically updated to reflect the committed changes. Closed by commit rL289276: [modules] Add optional out-param to ASTReader::ReadAST for imported submodules. (authored by graydon). Changed prior to commit: https://reviews.llvm.org/D27580?vs=80782&id=80952#toc Repository: rL LLVM https://reviews.llvm.org/D27580 Files: cfe/trunk/include/clang/Serialization/ASTReader.h cfe/trunk/lib/Serialization/ASTReader.cpp Index: cfe/trunk/include/clang/Serialization/ASTReader.h === --- cfe/trunk/include/clang/Serialization/ASTReader.h +++ cfe/trunk/include/clang/Serialization/ASTReader.h @@ -821,14 +821,16 @@ // \brief A list of late parsed template function data. SmallVector LateParsedTemplates; +public: struct ImportedSubmodule { serialization::SubmoduleID ID; SourceLocation ImportLoc; ImportedSubmodule(serialization::SubmoduleID ID, SourceLocation ImportLoc) : ID(ID), ImportLoc(ImportLoc) {} }; +private: /// \brief A list of modules that were imported by precompiled headers or /// any other non-module AST file. SmallVector ImportedModules; @@ -1404,9 +1406,13 @@ /// \param ClientLoadCapabilities The set of client load-failure /// capabilities, represented as a bitset of the enumerators of /// LoadFailureCapabilities. + /// + /// \param Imported optional out-parameter to append the list of modules + /// that were imported by precompiled headers or any other non-module AST file ASTReadResult ReadAST(StringRef FileName, ModuleKind Type, SourceLocation ImportLoc, -unsigned ClientLoadCapabilities); +unsigned ClientLoadCapabilities, +SmallVectorImpl *Imported = nullptr); /// \brief Make the entities in the given module and any of its (non-explicit) /// submodules visible to name lookup. Index: cfe/trunk/lib/Serialization/ASTReader.cpp === --- cfe/trunk/lib/Serialization/ASTReader.cpp +++ cfe/trunk/lib/Serialization/ASTReader.cpp @@ -3578,7 +3578,8 @@ ASTReader::ASTReadResult ASTReader::ReadAST(StringRef FileName, ModuleKind Type, SourceLocation ImportLoc, -unsigned ClientLoadCapabilities) { +unsigned ClientLoadCapabilities, +SmallVectorImpl *Imported) { llvm::SaveAndRestore SetCurImportLocRAII(CurrentImportLoc, ImportLoc); @@ -3744,6 +3745,10 @@ } UnresolvedModuleRefs.clear(); + if (Imported) +Imported->append(ImportedModules.begin(), + ImportedModules.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 // module itself. Index: cfe/trunk/include/clang/Serialization/ASTReader.h === --- cfe/trunk/include/clang/Serialization/ASTReader.h +++ cfe/trunk/include/clang/Serialization/ASTReader.h @@ -821,14 +821,16 @@ // \brief A list of late parsed template function data. SmallVector LateParsedTemplates; +public: struct ImportedSubmodule { serialization::SubmoduleID ID; SourceLocation ImportLoc; ImportedSubmodule(serialization::SubmoduleID ID, SourceLocation ImportLoc) : ID(ID), ImportLoc(ImportLoc) {} }; +private: /// \brief A list of modules that were imported by precompiled headers or /// any other non-module AST file. SmallVector ImportedModules; @@ -1404,9 +1406,13 @@ /// \param ClientLoadCapabilities The set of client load-failure /// capabilities, represented as a bitset of the enumerators of /// LoadFailureCapabilities. + /// + /// \param Imported optional out-parameter to append the list of modules + /// that were imported by precompiled headers or any other non-module AST file ASTReadResult ReadAST(StringRef FileName, ModuleKind Type, SourceLocation ImportLoc, -unsigned ClientLoadCapabilities); +unsigned ClientLoadCapabilities, +SmallVectorImpl *Imported = nullptr); /// \brief Make the entities in the given module and any of its (non-explicit) /// submodules visible to name lookup. Index: cfe/trunk/lib/Serialization/ASTReader.cpp === --- cfe/trunk/lib/Serialization/ASTReader.cpp +++ cfe/trunk/lib/Serialization/ASTReader.cpp @@ -3578,7 +3578,8 @@ ASTReader::ASTReadResult ASTReader::ReadAST(StringRef FileName, ModuleKind Type, SourceLocatio