bruno added inline comments.
================
Comment at: lib/Frontend/FrontendActions.cpp:227
+ IsBuiltin = true;
+ addHeaderInclude(H.NameAsWritten, Includes, LangOpts, Module->IsExternC,
+ IsBuiltin /*AlwaysInclude*/);
----------------
rsmith wrote:
> I don't understand why this would be necessary. If these headers are in fact
> textual headers, they won't be in the `HK_Normal` or `HK_Private` lists at
> all (they'll be in the `HK_Private` or `HK_PrivateTextual` lists instead), so
> your `IsBuiltin = true;` line should be unreachable. (Checking for an
> absolute path also seems suspicious here.)
>
> I suspect the problem is instead that `#import` just doesn't work properly
> with modules (specifically, either with local submodule visibility or with
> modules that do not `export *`), because it doesn't care whether the previous
> inclusion is visible or not, and as a result it assumes "I've heard about
> someone #including this ever" means the same thing as "the contents of this
> file are visible right now". I know that `#pragma once` has this issue, so
> I'd expect `#import` to also suffer from it.
In fact, I can achieve the same desired result by changing libcxx modulemap to:
@@ -161,11 +161,15 @@ module std [system] {
module cstddef {
header "cstddef"
export *
+ // FIXME: #import on Darwin requires explicit re-export
+ export Darwin.POSIX.sys.types
}
module cstdint {
header "cstdint"
export depr.stdint_h
export *
+ // FIXME: #import on Darwin requires explicit re-export
+ export Darwin.C.stdint
}
But I would like to avoid adding darwin specific stuff there.
> I don't understand why this would be necessary. If these headers are in fact
> textual headers,
> they won't be in the HK_Normal or HK_Private lists at all (they'll be in the
> HK_Private or
> HK_PrivateTextual lists instead), so your IsBuiltin = true; line should be
> unreachable.
AFAIU, builtins are added twice (1) with the full path with
`/<path_to_clang_install>/bin/../lib/clang/4.0.0/include/<builtin>.h`, as
`NormalHeader`, which maps to `HK_Normal` and (2) `<builtin>.h` as
`TextualHeader`, mapping to `HK_Textual`. This happens in the snippet
(lib/Lex/ModuleMap.cpp:1882) below:
if (BuiltinFile) {
// FIXME: Taking the name from the FileEntry is unstable and can give
// different results depending on how we've previously named that file
// in this build.
Module::Header H = { BuiltinFile->getName(), BuiltinFile };
Map.addHeader(ActiveModule, H, Role); <- (1)
// If we have both a builtin and system version of the file, the
// builtin version may want to inject macros into the system header, so
// force the system header to be treated as a textual header in this
// case.
Role = ModuleMap::ModuleHeaderRole(Role | ModuleMap::TextualHeader);
}
// Record this header.
Module::Header H = { RelativePathName.str(), File };
Map.addHeader(ActiveModule, H, Role); <- (2)
As an experiment, I changed `collectModuleHeaderIncludes` to skip
`addHeaderInclude` for builtin headers with `HK_Normal`, but it caused
regressions while compiling Darwin SDK even for non local submodule visibility.
> (Checking for an absolute path also seems suspicious here.)
Right, this was done to speedup checking for built-ins, since they are expanded
to absolute paths. But this isn't necessary for the logic to work and can be
removed.
> I suspect the problem is instead that #import just doesn't work properly with
> modules
> (specifically, either with local submodule visibility or with modules that do
> not export *),
> because it doesn't care whether the previous inclusion is visible or not, and
> as a result it assumes
> "I've heard about someone #including this ever" means the same thing as "the
> contents of this file
> are visible right now". I know that #pragma once has this issue, so I'd
> expect #import to also suffer from it.
Taking a look at this, any ideas on what's the right approach with the
#import's here? So instead of entering the header, is there a way to make the
contents for the module matching the built-in header to become
available/visible at that point?
https://reviews.llvm.org/D26267
_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
