bruno updated this revision to Diff 82067.
bruno marked 3 inline comments as done.
bruno added a comment.
Uploaded new patch with another approach to fix the issue.
https://reviews.llvm.org/D26267
Files:
include/clang/Lex/HeaderSearch.h
include/clang/Lex/ModuleMap.h
lib/Lex/HeaderSearch.cpp
lib/Lex/ModuleMap.cpp
lib/Lex/PPDirectives.cpp
test/Modules/Inputs/import-textual/M/A/A.h
test/Modules/Inputs/import-textual/M/B/B.h
test/Modules/Inputs/import-textual/M/module.modulemap
test/Modules/Inputs/import-textual/M/someheader.h
test/Modules/Inputs/import-textual/M2/A/A.h
test/Modules/Inputs/import-textual/M2/B/B.h
test/Modules/Inputs/import-textual/M2/module.modulemap
test/Modules/Inputs/import-textual/M2/someheader.h
test/Modules/Inputs/libc-libcxx/sysroot/usr/include/c++/v1/cstddef
test/Modules/Inputs/libc-libcxx/sysroot/usr/include/c++/v1/math.h
test/Modules/Inputs/libc-libcxx/sysroot/usr/include/c++/v1/module.modulemap
test/Modules/Inputs/libc-libcxx/sysroot/usr/include/c++/v1/stddef.h
test/Modules/Inputs/libc-libcxx/sysroot/usr/include/c++/v1/type_traits
test/Modules/Inputs/libc-libcxx/sysroot/usr/include/module.modulemap
test/Modules/Inputs/libc-libcxx/sysroot/usr/include/stddef.h
test/Modules/Inputs/libc-libcxx/sysroot/usr/include/sys/_types/_ptrdiff_t.h
test/Modules/Inputs/libc-libcxx/sysroot/usr/include/sys/_types/_types.h
test/Modules/builtin-import.mm
test/Modules/import-textual-noguard.mm
test/Modules/import-textual.mm
Index: test/Modules/import-textual.mm
===================================================================
--- /dev/null
+++ test/Modules/import-textual.mm
@@ -0,0 +1,10 @@
+// RUN: rm -rf %t
+// RUN: %clang_cc1 -fsyntax-only -std=c++11 -fmodules -fimplicit-module-maps -I%S/Inputs/import-textual/M -fmodules-cache-path=%t -x objective-c++ -fmodules-local-submodule-visibility %s -verify
+
+// expected-no-diagnostics
+
+#include "A/A.h"
+#include "B/B.h"
+
+typedef aint xxx;
+typedef bint yyy;
Index: test/Modules/import-textual-noguard.mm
===================================================================
--- /dev/null
+++ test/Modules/import-textual-noguard.mm
@@ -0,0 +1,8 @@
+// RUN: rm -rf %t
+// RUN: %clang_cc1 -fsyntax-only -std=c++11 -fmodules -fimplicit-module-maps -I%S/Inputs/import-textual/M2 -fmodules-cache-path=%t -x objective-c++ -fmodules-local-submodule-visibility %s -verify
+
+#include "A/A.h" // expected-error {{could not build module 'M'}}
+#include "B/B.h"
+
+typedef aint xxx;
+typedef bint yyy;
Index: test/Modules/builtin-import.mm
===================================================================
--- /dev/null
+++ test/Modules/builtin-import.mm
@@ -0,0 +1,12 @@
+// REQUIRES: system-darwin
+
+// RUN: rm -rf %t
+// RUN: %clang -cc1 -fsyntax-only -nostdinc++ -isysroot %S/Inputs/libc-libcxx/sysroot -isystem %S/Inputs/libc-libcxx/sysroot/usr/include/c++/v1 -std=c++11 -fmodules -fimplicit-module-maps -fmodules-cache-path=%t -x objective-c++ -fmodules-local-submodule-visibility %s
+
+#include <stdio.h>
+#include <stddef.h>
+#include <cstddef>
+
+typedef ptrdiff_t try1_ptrdiff_t;
+typedef my_ptrdiff_t try2_ptrdiff_t;
+
Index: test/Modules/Inputs/libc-libcxx/sysroot/usr/include/sys/_types/_types.h
===================================================================
--- /dev/null
+++ test/Modules/Inputs/libc-libcxx/sysroot/usr/include/sys/_types/_types.h
@@ -0,0 +1,6 @@
+#ifndef _SYS_TYPES_UMBRELLA
+#define _SYS_TYPES_UMBRELLA
+
+#include "_ptrdiff_t.h"
+
+#endif
Index: test/Modules/Inputs/libc-libcxx/sysroot/usr/include/sys/_types/_ptrdiff_t.h
===================================================================
--- /dev/null
+++ test/Modules/Inputs/libc-libcxx/sysroot/usr/include/sys/_types/_ptrdiff_t.h
@@ -0,0 +1,4 @@
+#ifndef _PTRDIFF_T
+#define _PTRDIFF_T
+typedef int * ptrdiff_t;
+#endif /* _PTRDIFF_T */
Index: test/Modules/Inputs/libc-libcxx/sysroot/usr/include/stddef.h
===================================================================
--- test/Modules/Inputs/libc-libcxx/sysroot/usr/include/stddef.h
+++ test/Modules/Inputs/libc-libcxx/sysroot/usr/include/stddef.h
@@ -1 +1,6 @@
-// stddef.h
+#ifndef __STDDEF_H__
+#define __STDDEF_H__
+
+#include "sys/_types/_ptrdiff_t.h"
+
+#endif
Index: test/Modules/Inputs/libc-libcxx/sysroot/usr/include/module.modulemap
===================================================================
--- test/Modules/Inputs/libc-libcxx/sysroot/usr/include/module.modulemap
+++ test/Modules/Inputs/libc-libcxx/sysroot/usr/include/module.modulemap
@@ -5,4 +5,12 @@
module stdint { header "stdint.h" export * }
module stdio { header "stdio.h" export * }
module util { header "util.h" export * }
+ module POSIX {
+ module sys {
+ module types {
+ umbrella header "sys/_types/_types.h"
+ export *
+ }
+ }
+ }
}
Index: test/Modules/Inputs/libc-libcxx/sysroot/usr/include/c++/v1/type_traits
===================================================================
--- /dev/null
+++ test/Modules/Inputs/libc-libcxx/sysroot/usr/include/c++/v1/type_traits
@@ -0,0 +1,6 @@
+#ifndef _LIBCPP_TYPE_TRAITS
+#define _LIBCPP_TYPE_TRAITS
+
+#include <cstddef>
+
+#endif
Index: test/Modules/Inputs/libc-libcxx/sysroot/usr/include/c++/v1/stddef.h
===================================================================
--- test/Modules/Inputs/libc-libcxx/sysroot/usr/include/c++/v1/stddef.h
+++ test/Modules/Inputs/libc-libcxx/sysroot/usr/include/c++/v1/stddef.h
@@ -2,5 +2,6 @@
#define LIBCXX_STDDEF_H
#include <__config>
+#include_next <stddef.h>
#endif
Index: test/Modules/Inputs/libc-libcxx/sysroot/usr/include/c++/v1/module.modulemap
===================================================================
--- test/Modules/Inputs/libc-libcxx/sysroot/usr/include/c++/v1/module.modulemap
+++ test/Modules/Inputs/libc-libcxx/sysroot/usr/include/c++/v1/module.modulemap
@@ -6,5 +6,7 @@
// FIXME: remove "textual" from stdint module below once the issue
// between umbrella headers and builtins is resolved.
module stdint { textual header "stdint.h" export * }
+ module type_traits { header "type_traits" export * }
+ module cstddef { header "cstddef" export * }
module __config { header "__config" export * }
}
Index: test/Modules/Inputs/libc-libcxx/sysroot/usr/include/c++/v1/math.h
===================================================================
--- test/Modules/Inputs/libc-libcxx/sysroot/usr/include/c++/v1/math.h
+++ test/Modules/Inputs/libc-libcxx/sysroot/usr/include/c++/v1/math.h
@@ -4,4 +4,6 @@
#include_next <math.h>
template<typename T> T abs(T t) { return (t < 0) ? -t : t; }
+#include <type_traits>
+
#endif
Index: test/Modules/Inputs/libc-libcxx/sysroot/usr/include/c++/v1/cstddef
===================================================================
--- /dev/null
+++ test/Modules/Inputs/libc-libcxx/sysroot/usr/include/c++/v1/cstddef
@@ -0,0 +1,9 @@
+#ifndef _LIBCPP_CSTDDEF
+#define _LIBCPP_CSTDDEF
+
+#include <stddef.h>
+#include <type_traits>
+
+typedef ptrdiff_t my_ptrdiff_t;
+
+#endif
Index: test/Modules/Inputs/import-textual/M2/someheader.h
===================================================================
--- /dev/null
+++ test/Modules/Inputs/import-textual/M2/someheader.h
@@ -0,0 +1 @@
+typedef int myint;
Index: test/Modules/Inputs/import-textual/M2/module.modulemap
===================================================================
--- /dev/null
+++ test/Modules/Inputs/import-textual/M2/module.modulemap
@@ -0,0 +1,17 @@
+
+module M {
+
+ module A {
+ header "A/A.h"
+ textual header "someheader.h"
+ export *
+ }
+
+ module B {
+ header "B/B.h"
+ textual header "someheader.h"
+ export *
+ }
+
+ export *
+}
Index: test/Modules/Inputs/import-textual/M2/B/B.h
===================================================================
--- /dev/null
+++ test/Modules/Inputs/import-textual/M2/B/B.h
@@ -0,0 +1,4 @@
+
+#import "someheader.h"
+
+typedef myint bint;
Index: test/Modules/Inputs/import-textual/M2/A/A.h
===================================================================
--- /dev/null
+++ test/Modules/Inputs/import-textual/M2/A/A.h
@@ -0,0 +1,4 @@
+
+#import "someheader.h"
+
+typedef myint aint;
Index: test/Modules/Inputs/import-textual/M/someheader.h
===================================================================
--- /dev/null
+++ test/Modules/Inputs/import-textual/M/someheader.h
@@ -0,0 +1,6 @@
+#ifndef C_GUARD
+#define C_GUARD
+
+typedef int myint;
+
+#endif
Index: test/Modules/Inputs/import-textual/M/module.modulemap
===================================================================
--- /dev/null
+++ test/Modules/Inputs/import-textual/M/module.modulemap
@@ -0,0 +1,17 @@
+
+module M {
+
+ module A {
+ header "A/A.h"
+ textual header "someheader.h"
+ export *
+ }
+
+ module B {
+ header "B/B.h"
+ textual header "someheader.h"
+ export *
+ }
+
+ export *
+}
Index: test/Modules/Inputs/import-textual/M/B/B.h
===================================================================
--- /dev/null
+++ test/Modules/Inputs/import-textual/M/B/B.h
@@ -0,0 +1,4 @@
+
+#import "someheader.h"
+
+typedef myint bint;
Index: test/Modules/Inputs/import-textual/M/A/A.h
===================================================================
--- /dev/null
+++ test/Modules/Inputs/import-textual/M/A/A.h
@@ -0,0 +1,4 @@
+
+#import "someheader.h"
+
+typedef myint aint;
Index: lib/Lex/PPDirectives.cpp
===================================================================
--- lib/Lex/PPDirectives.cpp
+++ lib/Lex/PPDirectives.cpp
@@ -1998,6 +1998,7 @@
// this file will have no effect.
if (ShouldEnter &&
!HeaderInfo.ShouldEnterIncludeFile(*this, File, isImport,
+ getLangOpts().Modules,
SuggestedModule.getModule())) {
ShouldEnter = false;
if (Callbacks)
Index: lib/Lex/ModuleMap.cpp
===================================================================
--- lib/Lex/ModuleMap.cpp
+++ lib/Lex/ModuleMap.cpp
@@ -144,7 +144,7 @@
/// \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) {
+bool ModuleMap::isBuiltinHeader(StringRef FileName) {
return llvm::StringSwitch<bool>(FileName)
.Case("float.h", true)
.Case("iso646.h", true)
@@ -165,7 +165,7 @@
HeadersMap::iterator Known = Headers.find(File);
if (HeaderInfo.getHeaderSearchOpts().ImplicitModuleMaps &&
Known == Headers.end() && File->getDir() == BuiltinIncludeDir &&
- isBuiltinHeader(llvm::sys::path::filename(File->getName()))) {
+ ModuleMap::isBuiltinHeader(llvm::sys::path::filename(File->getName()))) {
HeaderInfo.loadTopLevelSystemModules();
return Headers.find(File);
}
@@ -1843,7 +1843,7 @@
// supplied by Clang. Find that builtin header.
if (ActiveModule->IsSystem && LeadingToken != MMToken::UmbrellaKeyword &&
BuiltinIncludeDir && BuiltinIncludeDir != Directory &&
- isBuiltinHeader(Header.FileName)) {
+ ModuleMap::isBuiltinHeader(Header.FileName)) {
SmallString<128> BuiltinPathName(BuiltinIncludeDir->getName());
llvm::sys::path::append(BuiltinPathName, Header.FileName);
BuiltinFile = SourceMgr.getFileManager().getFile(BuiltinPathName);
Index: lib/Lex/HeaderSearch.cpp
===================================================================
--- lib/Lex/HeaderSearch.cpp
+++ lib/Lex/HeaderSearch.cpp
@@ -1072,25 +1072,62 @@
}
bool HeaderSearch::ShouldEnterIncludeFile(Preprocessor &PP,
- const FileEntry *File,
- bool isImport, Module *M) {
+ const FileEntry *File, bool isImport,
+ bool ModulesEnabled, Module *M) {
++NumIncluded; // Count # of attempted #includes.
// Get information about this file.
HeaderFileInfo &FileInfo = getFileInfo(File);
+ auto TryEnterImported = [&](void) -> bool {
+ if (!ModulesEnabled)
+ return false;
+ // Modules with builtins are special; multiple modules use builtins as
+ // modular headers, example:
+ //
+ // module stddef { header "stddef.h" export * }
+ //
+ // After module map parsing, this expands to:
+ //
+ // module stddef {
+ // header "/path_to_builtin_dirs/stddef.h"
+ // textual "stddef.h"
+ // }
+ //
+ // It's common that libc++ and system modules will both define such
+ // submodules. Make sure cached results for a builtin header won't
+ // prevent other builtin modules to potentially enter the builtin header.
+ // Note that builtins are header guarded and the decision to actually
+ // enter them is postponed to the controlling macros logic below.
+ bool TryEnterHdr = false;
+ if (FileInfo.isCompilingModuleHeader && FileInfo.isModuleHeader)
+ TryEnterHdr = File->getDir() == ModMap.getBuiltinDir() &&
+ ModuleMap::isBuiltinHeader(
+ llvm::sys::path::filename(File->getName()));
+
+ // Textual headers can be #imported from different modules. Since ObjC
+ // headers find in the wild might rely only on #import and do not contain
+ // controlling macros, be conservative and only try to enter textual headers
+ // if such macro is present.
+ if (!FileInfo.isModuleHeader &&
+ FileInfo.getControllingMacro(ExternalLookup))
+ TryEnterHdr = true;
+ return TryEnterHdr;
+ };
+
// If this is a #import directive, check that we have not already imported
// this header.
if (isImport) {
// If this has already been imported, don't import it again.
FileInfo.isImport = true;
// Has this already been #import'ed or #include'd?
- if (FileInfo.NumIncludes) return false;
+ if (FileInfo.NumIncludes && !TryEnterImported())
+ return false;
} else {
// Otherwise, if this is a #include of a file that was previously #import'd
// or if this is the second #include of a #pragma once file, ignore it.
- if (FileInfo.isImport)
+ if (FileInfo.isImport && !TryEnterImported())
return false;
}
Index: include/clang/Lex/ModuleMap.h
===================================================================
--- include/clang/Lex/ModuleMap.h
+++ include/clang/Lex/ModuleMap.h
@@ -316,6 +316,14 @@
BuiltinIncludeDir = Dir;
}
+ /// \brief Get the directory that contains Clang-supplied include files.
+ const DirectoryEntry *getBuiltinDir() const {
+ return BuiltinIncludeDir;
+ }
+
+ /// \brief Is this a compiler builtin header?
+ static bool isBuiltinHeader(StringRef FileName);
+
/// \brief Add a module map callback.
void addModuleMapCallbacks(std::unique_ptr<ModuleMapCallbacks> Callback) {
Callbacks.push_back(std::move(Callback));
Index: include/clang/Lex/HeaderSearch.h
===================================================================
--- include/clang/Lex/HeaderSearch.h
+++ include/clang/Lex/HeaderSearch.h
@@ -406,7 +406,8 @@
/// \return false if \#including the file will have no effect or true
/// if we should include it.
bool ShouldEnterIncludeFile(Preprocessor &PP, const FileEntry *File,
- bool isImport, Module *CorrespondingModule);
+ bool isImport, bool ModulesEnabled,
+ Module *CorrespondingModule);
/// \brief Return whether the specified file is a normal header,
/// a system header, or a C++ friendly system header.
_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
