[PATCH] D26267: [Modules] Include builtins with #include instead of #import for ObjC

[Richard Smith via Phabricator via cfe-commits]

rsmith added inline comments.


================
Comment at: lib/Frontend/FrontendActions.cpp:227
+        IsBuiltin = true;
+      addHeaderInclude(H.NameAsWritten, Includes, LangOpts, Module->IsExternC,
+                       IsBuiltin /*AlwaysInclude*/);
----------------
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.


https://reviews.llvm.org/D26267