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

[Richard Smith via cfe-commits]

rsmith added a comment.

It seems to me like this is papering over an issue rather than fixing it.

`diagnoseHeaderInclusion` calls `isHeaderInUmbrellaDirs` here, which is presumably failing to determine that `Nonmodular/A.h` is in the umbrella directory for the `Nonmodular` module. My first guess would be that this code from `ModuleMap::findHeaderInUmbrellaDirs` may be responsible:

  // Note: as an egregious but useful hack we use the real path here, because
  // frameworks moving from top-level frameworks to embedded frameworks tend
  // to be symlinked from the top-level location to the embedded location,
  // and we need to resolve lookups as if we had found the embedded location.
  StringRef DirName = SourceMgr.getFileManager().getCanonicalName(Dir);


================
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;
 
----------------
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?


https://reviews.llvm.org/D23858