[PATCH] D23858: Don't diagnose non-modular includes when we are not compiling a module

[Richard Smith via cfe-commits]

rsmith accepted this revision.
rsmith added a comment.
This revision is now accepted and ready to land.

OK, if this unblocks you and you're working towards properly handling umbrella headers in the VFS, I'm happy to go ahead with this.

Please either rename the variable or move that part of the check into `diagnoseHeaderInclusion`, though -- with this change, the variable means something quite different from its name.


================
Comment at: lib/Lex/PPDirectives.cpp:749-750
@@ -748,3 +748,4 @@
   Module *RequestingModule = getModuleForLocation(FilenameLoc);
-  bool RequestingModuleIsModuleInterface = !SourceMgr.isInMainFile(FilenameLoc);
+  bool RequestingModuleIsModuleInterface =
+      !SourceMgr.isInMainFile(FilenameLoc) && getLangOpts().CompilingModule;
 
----------------
manmanren wrote:
> rsmith wrote:
> > I don't see why this should depend on whether we're compiling a module. If we're asked to diagnose non-modular #includes in modular headers, why should it matter whether we entered that header textually or by triggering precompilation of the corresponding module?
> I agree that here, we are actually including a non modular header from a modular header. But is it okay to not diagnose when we have specified fmodule-name and we are building the implementation of it? We should have diagnosed this when building the unit.
Sure, if we're running in a mode where we actually build the modules, rather than just including them textually. Under `-fmodules-local-submodule-visibility`, we support providing modules semantics without doing separate compilation for module headers.


https://reviews.llvm.org/D23858