From 3ec6663be0ed79325d9d865a31f514e2f4e218fe Mon Sep 17 00:00:00 2001 From: Douglas Gregor Date: Thu, 2 Feb 2012 18:42:48 +0000 Subject: [PATCH] Back out my heinous hack that tricked the module generation mechanism into using non-absolute system includes ()... ... and introduce another hack that is simultaneously more heineous and more effective. We whitelist Clang-supplied headers that augment or override system headers (such as float.h, stdarg.h, and tgmath.h). For these headers, Clang does not provide a module mapping. Instead, a system-supplied module map can refer to these headers in a system module, and Clang will look both in its own include directory and wherever the system-supplied module map suggests, then adds either or both headers. The end result is that Clang-supplied headers get merged into the system-supplied module for the C standard library. As a drive-by, fix up a few dependencies in the _Builtin_instrinsics module. llvm-svn: 149611 --- clang/include/clang/Lex/ModuleMap.h | 10 +++ clang/lib/Basic/Module.cpp | 2 + clang/lib/Frontend/FrontendActions.cpp | 65 ++++++++----------- clang/lib/Frontend/InitHeaderSearch.cpp | 8 +++ clang/lib/Headers/module.map | 53 ++------------- clang/lib/Lex/ModuleMap.cpp | 57 ++++++++++++++-- .../Inputs/System/usr/include/module.map | 21 ++++++ .../Inputs/System/usr/include/stdbool.h | 1 + .../Inputs/System/usr/include/stdint.h | 1 + .../Modules/Inputs/System/usr/include/stdio.h | 3 + clang/test/Modules/compiler_builtins.m | 16 ----- clang/test/Modules/cstd.m | 29 +++++++++ 12 files changed, 160 insertions(+), 106 deletions(-) create mode 100644 clang/test/Modules/Inputs/System/usr/include/module.map create mode 100644 clang/test/Modules/Inputs/System/usr/include/stdbool.h create mode 100644 clang/test/Modules/Inputs/System/usr/include/stdint.h create mode 100644 clang/test/Modules/Inputs/System/usr/include/stdio.h create mode 100644 clang/test/Modules/cstd.m diff --git a/clang/include/clang/Lex/ModuleMap.h b/clang/include/clang/Lex/ModuleMap.h index 6176ed8c30aa..85740816d15f 100644 --- a/clang/include/clang/Lex/ModuleMap.h +++ b/clang/include/clang/Lex/ModuleMap.h @@ -41,6 +41,10 @@ class ModuleMap { const LangOptions &LangOpts; const TargetInfo *Target; + /// \brief The directory used for Clang-supplied, builtin include headers, + /// such as "stdint.h". + const DirectoryEntry *BuiltinIncludeDir; + /// \brief Language options used to parse the module map itself. /// /// These are always simple C language options. @@ -102,6 +106,12 @@ public: /// \brief Set the target information. void setTarget(const TargetInfo &Target); + /// \brief Set the directory that contains Clang-supplied include + /// files, such as our stdarg.h or tgmath.h. + void setBuiltinIncludeDir(const DirectoryEntry *Dir) { + BuiltinIncludeDir = Dir; + } + /// \brief Retrieve the module that owns the given header file, if any. /// /// \param File The header file that is likely to be included. diff --git a/clang/lib/Basic/Module.cpp b/clang/lib/Basic/Module.cpp index c5838fb65f2b..634884074e14 100644 --- a/clang/lib/Basic/Module.cpp +++ b/clang/lib/Basic/Module.cpp @@ -32,6 +32,8 @@ Module::Module(StringRef Name, SourceLocation DefinitionLoc, Module *Parent, if (Parent) { if (!Parent->isAvailable()) IsAvailable = false; + if (Parent->IsSystem) + IsSystem = true; Parent->SubModuleIndex[Name] = Parent->SubModules.size(); Parent->SubModules.push_back(this); diff --git a/clang/lib/Frontend/FrontendActions.cpp b/clang/lib/Frontend/FrontendActions.cpp index edadf3776691..c0302329c1e2 100644 --- a/clang/lib/Frontend/FrontendActions.cpp +++ b/clang/lib/Frontend/FrontendActions.cpp @@ -126,32 +126,6 @@ ASTConsumer *GenerateModuleAction::CreateASTConsumer(CompilerInstance &CI, Sysroot, OS); } -/// \brief Add an appropriate #include/#import for the given header within -/// the current module context. -static void addHeaderInclude(StringRef Header, - bool IsBuiltinModule, - const LangOptions &LangOpts, - llvm::SmallString<256> &Includes) { - if (IsBuiltinModule) { - // Our own builtin headers play some evil tricks that depend both on - // knowing that our headers will be found first and on include_next. To - // Make sure these include_next tricks work, we include with <> and - // just the filename itself rather than using an absolutely path. - // FIXME: Is there some sensible way to generalize this? - Includes += "#include <"; - Includes += llvm::sys::path::filename(Header); - Includes += ">\n"; - return; - } - - if (LangOpts.ObjC1) - Includes += "#import \""; - else - Includes += "#include \""; - Includes += Header; - Includes += "\"\n"; -} - /// \brief Collect the set of header includes needed to construct the given /// module. /// @@ -163,21 +137,31 @@ static void collectModuleHeaderIncludes(const LangOptions &LangOpts, FileManager &FileMgr, ModuleMap &ModMap, clang::Module *Module, - bool IsBuiltinModule, llvm::SmallString<256> &Includes) { // Don't collect any headers for unavailable modules. if (!Module->isAvailable()) return; // Add includes for each of these headers. - for (unsigned I = 0, N = Module->Headers.size(); I != N; ++I) - addHeaderInclude(Module->Headers[I]->getName(), IsBuiltinModule, LangOpts, - Includes); + for (unsigned I = 0, N = Module->Headers.size(); I != N; ++I) { + if (LangOpts.ObjC1) + Includes += "#import \""; + else + Includes += "#include \""; + Includes += Module->Headers[I]->getName(); + Includes += "\"\n"; + } if (const FileEntry *UmbrellaHeader = Module->getUmbrellaHeader()) { - if (Module->Parent) - addHeaderInclude(UmbrellaHeader->getName(), IsBuiltinModule, LangOpts, - Includes); + if (Module->Parent) { + // Include the umbrella header for submodules. + if (LangOpts.ObjC1) + Includes += "#import \""; + else + Includes += "#include \""; + Includes += UmbrellaHeader->getName(); + Includes += "\"\n"; + } } else if (const DirectoryEntry *UmbrellaDir = Module->getUmbrellaDir()) { // Add all of the headers we find in this subdirectory. llvm::error_code EC; @@ -199,8 +183,13 @@ static void collectModuleHeaderIncludes(const LangOptions &LangOpts, if (ModMap.isHeaderInUnavailableModule(Header)) continue; - // Include this header. - addHeaderInclude(Dir->path(), IsBuiltinModule, LangOpts, Includes); + // Include this header umbrella header for submodules. + if (LangOpts.ObjC1) + Includes += "#import \""; + else + Includes += "#include \""; + Includes += Dir->path(); + Includes += "\"\n"; } } @@ -208,8 +197,7 @@ static void collectModuleHeaderIncludes(const LangOptions &LangOpts, for (clang::Module::submodule_iterator Sub = Module->submodule_begin(), SubEnd = Module->submodule_end(); Sub != SubEnd; ++Sub) - collectModuleHeaderIncludes(LangOpts, FileMgr, ModMap, *Sub, - IsBuiltinModule, Includes); + collectModuleHeaderIncludes(LangOpts, FileMgr, ModMap, *Sub, Includes); } bool GenerateModuleAction::BeginSourceFileAction(CompilerInstance &CI, @@ -261,11 +249,10 @@ bool GenerateModuleAction::BeginSourceFileAction(CompilerInstance &CI, const FileEntry *UmbrellaHeader = Module->getUmbrellaHeader(); // Collect the set of #includes we need to build the module. - bool IsBuiltinModule = StringRef(Module->Name).startswith("_Builtin_"); llvm::SmallString<256> HeaderContents; collectModuleHeaderIncludes(CI.getLangOpts(), CI.getFileManager(), CI.getPreprocessor().getHeaderSearchInfo().getModuleMap(), - Module, IsBuiltinModule, HeaderContents); + Module, HeaderContents); if (UmbrellaHeader && HeaderContents.empty()) { // Simple case: we have an umbrella header and there are no additional // includes, we can just parse the umbrella header directly. diff --git a/clang/lib/Frontend/InitHeaderSearch.cpp b/clang/lib/Frontend/InitHeaderSearch.cpp index ee75e6921d93..9faa12675534 100644 --- a/clang/lib/Frontend/InitHeaderSearch.cpp +++ b/clang/lib/Frontend/InitHeaderSearch.cpp @@ -666,5 +666,13 @@ void clang::ApplyHeaderSearchOptions(HeaderSearch &HS, Init.AddDefaultIncludePaths(Lang, Triple, HSOpts); + if (HSOpts.UseBuiltinIncludes) { + // Set up the builtin include directory in the module map. + llvm::sys::Path P(HSOpts.ResourceDir); + P.appendComponent("include"); + if (const DirectoryEntry *Dir = HS.getFileMgr().getDirectory(P.str())) + HS.getModuleMap().setBuiltinIncludeDir(Dir); + } + Init.Realize(Lang); } diff --git a/clang/lib/Headers/module.map b/clang/lib/Headers/module.map index e321ca3ca5ab..418ba5009094 100644 --- a/clang/lib/Headers/module.map +++ b/clang/lib/Headers/module.map @@ -1,42 +1,3 @@ -module _Builtin_stdlib [system] { - explicit module float_constants { - header "float.h" - } - - explicit module iso646 { - header "iso646.h" - } - - explicit module limits { - header "limits.h" - } - - explicit module stdalign { - header "stdalign.h" - } - - explicit module stdarg { - header "stdarg.h" - } - - explicit module stdbool { - header "stdbool.h" - } - - explicit module stddef { - header "stddef.h" - } - - explicit module stdint { - header "stdint.h" - } - - explicit module varargs { - requires unavailable - header "varargs.h" - } -} - module _Builtin_intrinsics [system] { explicit module altivec { requires altivec @@ -45,10 +6,16 @@ module _Builtin_intrinsics [system] { explicit module intel { requires x86 + export * header "immintrin.h" header "x86intrin.h" + explicit module mm_malloc { + header "mm_malloc.h" + export * // note: for dependency + } + explicit module cpuid { header "cpuid.h" } @@ -61,6 +28,7 @@ module _Builtin_intrinsics [system] { explicit module sse { requires sse export mmx + export * // note: for hackish dependency header "xmmintrin.h" } @@ -136,12 +104,5 @@ module _Builtin_intrinsics [system] { requires mm3dnow header "mm3dnow.h" } - - explicit module mm_malloc { - header "mm_malloc.h" - } } - - // FIXME: tgmath.h - // FIXME: unwind.h } diff --git a/clang/lib/Lex/ModuleMap.cpp b/clang/lib/Lex/ModuleMap.cpp index 3583b034d538..c1c50660125f 100644 --- a/clang/lib/Lex/ModuleMap.cpp +++ b/clang/lib/Lex/ModuleMap.cpp @@ -71,7 +71,7 @@ ModuleMap::resolveExport(Module *Mod, ModuleMap::ModuleMap(FileManager &FileMgr, const DiagnosticConsumer &DC, const LangOptions &LangOpts, const TargetInfo *Target) - : LangOpts(LangOpts), Target(Target) + : LangOpts(LangOpts), Target(Target), BuiltinIncludeDir(0) { llvm::IntrusiveRefCntPtr DiagIDs(new DiagnosticIDs); Diags = llvm::IntrusiveRefCntPtr( @@ -499,7 +499,10 @@ namespace clang { /// \brief The directory that this module map resides in. const DirectoryEntry *Directory; - + + /// \brief The directory containing Clang-supplied headers. + const DirectoryEntry *BuiltinIncludeDir; + /// \brief Whether an error occurred. bool HadError; @@ -540,9 +543,11 @@ namespace clang { explicit ModuleMapParser(Lexer &L, SourceManager &SourceMgr, DiagnosticsEngine &Diags, ModuleMap &Map, - const DirectoryEntry *Directory) + const DirectoryEntry *Directory, + const DirectoryEntry *BuiltinIncludeDir) : L(L), SourceMgr(SourceMgr), Diags(Diags), Map(Map), - Directory(Directory), HadError(false), ActiveModule(0) + Directory(Directory), BuiltinIncludeDir(BuiltinIncludeDir), + HadError(false), ActiveModule(0) { TargetOptions TargetOpts; TargetOpts.Triple = llvm::sys::getDefaultTargetTriple(); @@ -1028,6 +1033,24 @@ void appendSubframeworkPaths(Module *Mod, llvm::SmallVectorImpl &Path) { } } +/// \brief Determine whether the given file name is the name of a builtin +/// header, supplied by Clang to replace, override, or augment existing system +/// headers. +static bool isBuiltinHeader(StringRef FileName) { + return llvm::StringSwitch(FileName) + .Case("float.h", true) + .Case("iso646.h", true) + .Case("limits.h", true) + .Case("stdalign.h", true) + .Case("stdarg.h", true) + .Case("stdbool.h", true) + .Case("stddef.h", true) + .Case("stdint.h", true) + .Case("tgmath.h", true) + .Case("unwind.h", true) + .Default(false); +} + /// \brief Parse a header declaration. /// /// header-declaration: @@ -1058,6 +1081,7 @@ void ModuleMapParser::parseHeaderDecl(SourceLocation UmbrellaLoc) { // Look for this file. const FileEntry *File = 0; + const FileEntry *BuiltinFile = 0; llvm::SmallString<128> PathName; if (llvm::sys::path::is_absolute(FileName)) { PathName = FileName; @@ -1090,6 +1114,24 @@ void ModuleMapParser::parseHeaderDecl(SourceLocation UmbrellaLoc) { // Lookup for normal headers. llvm::sys::path::append(PathName, FileName); File = SourceMgr.getFileManager().getFile(PathName); + + // If this is a system module with a top-level header, this header + // may have a counterpart (or replacement) in the set of headers + // supplied by Clang. Find that builtin header. + if (ActiveModule->IsSystem && !Umbrella && BuiltinIncludeDir && + BuiltinIncludeDir != Directory && isBuiltinHeader(FileName)) { + llvm::SmallString<128> BuiltinPathName(BuiltinIncludeDir->getName()); + llvm::sys::path::append(BuiltinPathName, FileName); + BuiltinFile = SourceMgr.getFileManager().getFile(BuiltinPathName); + + // If Clang supplies this header but the underlying system does not, + // just silently swap in our builtin version. Otherwise, we'll end + // up adding both (later). + if (!File && BuiltinFile) { + File = BuiltinFile; + BuiltinFile = 0; + } + } } } @@ -1113,6 +1155,10 @@ void ModuleMapParser::parseHeaderDecl(SourceLocation UmbrellaLoc) { } else { // Record this header. Map.addHeader(ActiveModule, File); + + // If there is a builtin counterpart to this file, add it now. + if (BuiltinFile) + Map.addHeader(ActiveModule, BuiltinFile); } } else { Diags.Report(FileNameLoc, diag::err_mmap_header_not_found) @@ -1375,7 +1421,8 @@ bool ModuleMap::parseModuleMapFile(const FileEntry *File) { // Parse this module map file. Lexer L(ID, SourceMgr->getBuffer(ID), *SourceMgr, MMapLangOpts); Diags->getClient()->BeginSourceFile(MMapLangOpts); - ModuleMapParser Parser(L, *SourceMgr, *Diags, *this, File->getDir()); + ModuleMapParser Parser(L, *SourceMgr, *Diags, *this, File->getDir(), + BuiltinIncludeDir); bool Result = Parser.parseModuleMapFile(); Diags->getClient()->EndSourceFile(); diff --git a/clang/test/Modules/Inputs/System/usr/include/module.map b/clang/test/Modules/Inputs/System/usr/include/module.map new file mode 100644 index 000000000000..884b59c80cd0 --- /dev/null +++ b/clang/test/Modules/Inputs/System/usr/include/module.map @@ -0,0 +1,21 @@ +module cstd [system] { + // Only in compiler support directory + module float_constants { + header "float.h" + } + + // Only in system headers directory + module stdio { + header "stdio.h" + } + + // In both directories (compiler support version wins, does not forward) + module stdbool { + header "stdbool.h" + } + + // In both directories (compiler support version wins, forwards) + module stdint { + header "stdint.h" + } +} diff --git a/clang/test/Modules/Inputs/System/usr/include/stdbool.h b/clang/test/Modules/Inputs/System/usr/include/stdbool.h new file mode 100644 index 000000000000..760d7dc48efd --- /dev/null +++ b/clang/test/Modules/Inputs/System/usr/include/stdbool.h @@ -0,0 +1 @@ +// Testing hack: does not define bool/true/false. diff --git a/clang/test/Modules/Inputs/System/usr/include/stdint.h b/clang/test/Modules/Inputs/System/usr/include/stdint.h new file mode 100644 index 000000000000..e8e50f90290c --- /dev/null +++ b/clang/test/Modules/Inputs/System/usr/include/stdint.h @@ -0,0 +1 @@ +typedef int my_awesome_nonstandard_integer_type; diff --git a/clang/test/Modules/Inputs/System/usr/include/stdio.h b/clang/test/Modules/Inputs/System/usr/include/stdio.h new file mode 100644 index 000000000000..9a7b1063032c --- /dev/null +++ b/clang/test/Modules/Inputs/System/usr/include/stdio.h @@ -0,0 +1,3 @@ +typedef struct { int id; } FILE; +int fprintf(FILE*restrict, const char* restrict format, ...); + diff --git a/clang/test/Modules/compiler_builtins.m b/clang/test/Modules/compiler_builtins.m index 5ec2e978b3e0..a4f1dc21328d 100644 --- a/clang/test/Modules/compiler_builtins.m +++ b/clang/test/Modules/compiler_builtins.m @@ -1,22 +1,6 @@ // RUN: rm -rf %t // RUN: %clang -fsyntax-only -fmodules -fmodule-cache-path %t -D__need_wint_t %s -Xclang -verify -@import _Builtin_stdlib.float_constants; - -float getFltMax() { return FLT_MAX; } - -@import _Builtin_stdlib.limits; - -char getCharMax() { return CHAR_MAX; } - -// MS limits.h provides size_t. -// XFAIL: win32 -size_t size; // expected-error{{unknown type name 'size_t'}} - -@import _Builtin_stdlib.stdint; - -intmax_t value; - #ifdef __SSE__ @import _Builtin_intrinsics.intel.sse; #endif diff --git a/clang/test/Modules/cstd.m b/clang/test/Modules/cstd.m new file mode 100644 index 000000000000..8bd5c92cff14 --- /dev/null +++ b/clang/test/Modules/cstd.m @@ -0,0 +1,29 @@ +// RUN: rm -rf %t +// RUN: %clang -fsyntax-only -isysroot %S/Inputs/System -fmodules -fmodule-cache-path %t -D__need_wint_t %s -Xclang -verify + +// Supplied by compiler, but referenced from the "/usr/include" module map. +@import cstd.float_constants; + +float getFltMax() { return FLT_MAX; } + +// Supplied by the "/usr/include" module map. +@import cstd.stdio; + +void test_fprintf(FILE *file) { + fprintf(file, "Hello, modules\n"); +} + +// Supplied by compiler, which forwards to the the "/usr/include" version. +@import cstd.stdint; + +my_awesome_nonstandard_integer_type value; + +// Supplied by the compiler; that version wins. +@import cstd.stdbool; + +#ifndef bool +# error "bool was not defined!" +#endif + + +